Re: [PATCH] [BZ #20985] robust mutexes: Fix broken x86 assembly by removing it

[Florian Weimer <fweimer at redhat dot com>]

On 01/13/2017 12:20 PM, Torvald Riegel wrote:
> On Fri, 2017-01-13 at 11:01 +0100, Florian Weimer wrote:
> > On 12/22/2016 12:23 PM, Torvald Riegel wrote:
> > > On Thu, 2016-12-22 at 12:06 +0100, Florian Weimer wrote:
> > > > On 12/22/2016 11:22 AM, Torvald Riegel wrote:
> > > > >     	(LLL_ROBUST_MUTEX_LOCK_MODIFIER): New.
> > > >
> > > > This needs to be documented in the code.  It seems the code always
> > > > defines it as 0, so it's not clear to me why it makes sense to define it
> > > > at all (twice even).
> > >
> > > This should be defined differently when pthread_mutex_cond_lock.c
> > > includes the file; I just noticed that it doesn't do that.
> > >
> > > Is the patch OK besides that (ie, so after fixing that oversight and
> > > adding a suitable comment to the definition of
> > > LLL_ROBUST_MUTEX_LOCK_MODIFIER?).
> >
> > Do we already have a test case which detects the bug you introduced?
>
> The bug I introduced, meaning the oversight in the previous definition?
> I don't think we need tests for mistakes that never made it into master.

My worry is if the existing test suite does not catch this, we have 
*substantial* amount of object code which is completely untested.

> Additionally, I intend to get rid of the special handling of cond_lock
> eventually; it's not necessary anymore because the new condvar doesn't
> use futex-requeue.  But that's a change for the next release.

Ahh, well, that might also explain why the test suite doesn't detect it. :)

> > Would you please post a consolidated patch rebased to current master?
>
> Attached.

Can you mention in the commit message that this patch does not use 
C11-style atomics?  In case we revisit this later and wonder what was 
the reason for not using relaxed MO loads for the futex word values etc.

The substance of the patch looks okay to me, and it passes testing on 
x86_64 for me, too.

Thanks,
Florian