Hi, On 1/12/2023 11:58 AM, Jingbo Xu wrote: > > On 1/12/23 12:06 AM, David Howells wrote: >> Hou Tao <houtao@xxxxxxxxxxxxxxx> wrote: >> >>> clear_bit(FSCACHE_VOLUME_ACQUIRE_PENDING, &cursor->flags); >>> + /* >>> + * Paired with barrier in wait_on_bit(). Check >>> + * wake_up_bit() and waitqueue_active() for details. >>> + */ >>> + smp_mb__after_atomic(); >>> wake_up_bit(&cursor->flags, FSCACHE_VOLUME_ACQUIRE_PENDING); >> What two values are you applying a partial ordering to? > Yeah Hou Tao has explained that a full barrier is needed here to avoid > the potential reordering at the waker side. > > As I was also researching on this these days, I'd like to share my > thought on this, hopefully if it could give some insight :) > > Without the barrier at the waker side, it may suffer from the following > race: > > ``` > CPU0 - waker CPU1 - waiter > > if (waitqueue_active(wq_head)) <-- find no wq_entry in wq_head list > wake_up(wq_head); > > for (;;) { > prepare_to_wait(...); > # add wq_entry into wq_head list > > if (@cond) <-- @cond is false > break; > schedule(); <-- wq_entry still in > wq_head list, > wait for next wakeup > } > finish_wait(&wq_head, &wait); > > @cond = true; > ``` > > in which case the waiter misses the wakeup for one time. Thanks for the details annotation. It is exactly what I tried to say but failed to. > -- Linux-cachefs mailing list Linux-cachefs@xxxxxxxxxx https://listman.redhat.com/mailman/listinfo/linux-cachefs