On Fri, Dec 16, 2016 at 04:50:45PM -0800, Darren Hart wrote: > On Tue, Dec 13, 2016 at 09:36:42AM +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. > > > > +Paul McKenney > > Per the introduction of the comment below from: > > f1a11e0 futex: remove the wait queue > > I believe the intent was to ensure the plist_del in ... the previous > __unqueue_futex(q) ... from getting ahead of the smp_store_release added here, > which could result in q being destroyed by the waking task before plist_del > can > act on it. Is that > right? > > The comment below predates the refactoring which hid plist_del under the > __unqueue_futex() making it a bit less clear as to the associated plist_del: > > However, since this comment, we have moved the wake-up out of wake_futex > through > the use of wake queues (wake_up_q) which now happens after the hb lock is > released (see futex_wake, futex_wake_op, and futex_requeue). Is this race > still > a valid concern?
Yes I think so, since __unqueue_futex() dereferences lock_ptr and does stores in the memory it points to, those stores must not happen _after_ we NULL lock_ptr itself. futex_wait(), which calls unqueue_me() could have had a spurious wakeup and observe our NULL store and 'free' the futex_q.
