On Tue, 2015-01-27 at 18:07 +0100, Peter Zijlstra wrote: > On Sun, Jan 25, 2015 at 11:36:05PM -0800, Davidlohr Bueso wrote: > > The need for the smp_mb in __rwsem_do_wake should be > > properly documented. Applies to both xadd and spinlock > > variants. > > > > Signed-off-by: Davidlohr Bueso <dbu...@suse.de> > > --- > > kernel/locking/rwsem-spinlock.c | 5 +++++ > > kernel/locking/rwsem-xadd.c | 5 +++++ > > 2 files changed, 10 insertions(+) > > > > diff --git a/kernel/locking/rwsem-spinlock.c > > b/kernel/locking/rwsem-spinlock.c > > index 2555ae1..54f7a17 100644 > > --- a/kernel/locking/rwsem-spinlock.c > > +++ b/kernel/locking/rwsem-spinlock.c > > @@ -85,6 +85,11 @@ __rwsem_do_wake(struct rw_semaphore *sem, int wakewrite) > > > > list_del(&waiter->list); > > tsk = waiter->task; > > + /* > > + * Ensure that all cores see the read before > > + * setting it to the waiter task to nil, as that > > + * grants the read lock to the next task. > > + */ > > smp_mb(); > > waiter->task = NULL; > > wake_up_process(tsk); > > Could you enhance that comment by pointing at the pairing code? Is that > the wait loop in rwsem_down_read_failed()?
Yep. > Also, the comment confuses, how can all cores observe a read into a > local variable? Yep, I'll rephrase that ;) -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/