blk-throttle: Correct the placement of smp_rmb()
authorVivek Goyal <vgoyal@redhat.com>
Wed, 1 Dec 2010 18:34:52 +0000 (19:34 +0100)
committerJens Axboe <jaxboe@fusionio.com>
Wed, 1 Dec 2010 18:34:52 +0000 (19:34 +0100)
commit04a6b516cdc6efc2500b52a540cf65be8c5aaf9e
treefa5d6675308df0ff42eceeec1b406d2ce5eb2ff6
parentd1ae8ffdfaa16b2ab2e9346e81cf0ab6eaaae347
blk-throttle: Correct the placement of smp_rmb()

o I was discussing what are the variable being updated without spin lock and
  why do we need barriers and Oleg pointed out that location of smp_rmb()
  should be between read of td->limits_changed and tg->limits_changed. This
  patch fixes it.

o Following is one possible sequence of events. Say cpu0 is executing
  throtl_update_blkio_group_read_bps() and cpu1 is executing
  throtl_process_limit_change().

 cpu0                                                cpu1

 tg->limits_changed = true;
 smp_mb__before_atomic_inc();
 atomic_inc(&td->limits_changed);

                                     if (!atomic_read(&td->limits_changed))
                                             return;

                                     if (tg->limits_changed)
                                             do_something;

 If cpu0 has updated tg->limits_changed and td->limits_changed, we want to
 make sure that if update to td->limits_changed is visible on cpu1, then
 update to tg->limits_changed should also be visible.

 Oleg pointed out to ensure that we need to insert an smp_rmb() between
 td->limits_changed read and tg->limits_changed read.

o I had erroneously put smp_rmb() before atomic_read(&td->limits_changed).
  This patch fixes it.

Reported-by: Oleg Nesterov <oleg@redhat.com>
Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
Signed-off-by: Jens Axboe <jaxboe@fusionio.com>
block/blk-throttle.c