Re: [RFC][BZ #13690] Always read private before lll_unlock.

[Torvald Riegel <triegel@redhat.com>]

On Fri, 2013-12-06 at 20:43 +0100, Ondřej Bílka wrote:
> On Fri, Dec 06, 2013 at 08:01:59PM +0100, Ondřej Bílka wrote:
> > Hi, a related issue to semaphore races is a race in mutex unlocking.
> >
> from bugzilla:
> >
> > On most platforms, lll_unlock() is defined as a macro like this:
> > #define lll_unlock(lock, private) \
> >   ((void) ({						      \
> >     int *__futex = &(lock);				      \
> >     int __val = atomic_exchange_rel (__futex, 0);	      \
> >     if (__builtin_expect (__val > 1, 0))		      \
> >       lll_futex_wake (__futex, 1, private);		      \
> >   }))
> > 
> Which causes this problem that could be avoided by changing macro to
> 
> #define lll_unlock(lock, private) \
>    ((void) ({                                                \
>      int *__futex = &(lock);                                 \
>      int __private = private				     \
> 
> I wrote a prelimitary patch for that, most of lll_unlock macros are
> duplicates so I added a file include/futex_unlock.h with common
> implementation. We should check these for more duplicates and if 
> other functions need similar wrapper.
> 
> Comments?

This needs clarification in POSIX; see my comment on #13690
(https://sourceware.org/bugzilla/show_bug.cgi?id=13690#c24).  Depending
on how the Austin Group decides, this is either not a bug, or we have to
fix the pending load AND investigate whether the pending futex_wake call
is harmless.  The latter might be the case for normal mutexes, but I
wouldn't be surprised to find out that PI or robust mutexes aren't as
simple and need a more complex fix.

Therefore, I think we should wait for the POSIX clarification and then
decide what the next steps should be.