On Wed, Mar 22, 2017 at 11:35:49AM +0100, Peter Zijlstra wrote: > Since the futex_q can dissapear the instruction after assigning NULL, > this really should be a RELEASE barrier. That stops loads from hitting > dead memory too. > > Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
I reviewed this carefully in the previous thread, confirming that despite the move to wake queues, spurious wakeups can still lead to the situration Peter describes. As such: Reviewed-by: Darren Hart (VMware) <[email protected]> My only suggestion would be to clarify the language in the preceding comment to make that obvious, as well as clarify which plist_del it is referring to since it has been moved under the __unqueue_futex. I can do that as a follow-on though. > --- > kernel/futex.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > --- a/kernel/futex.c > +++ b/kernel/futex.c > @@ -1288,8 +1288,7 @@ static void mark_wake_futex(struct wake_ > * memory barrier is required here to prevent the following > * store to lock_ptr from getting ahead of the plist_del. > */ > - smp_wmb(); > - q->lock_ptr = NULL; > + smp_store_release(&q->lock_ptr, NULL); > } > > static int wake_futex_pi(u32 __user *uaddr, u32 uval, struct futex_q > *top_waiter, > > > -- Darren Hart VMware Open Source Technology Center
