Re: [PATCH] Have x86 and x86_64 lll_futex_wake return the syscall error.

["Carlos O'Donell" <carlos at redhat dot com>]

On 06/09/2013 03:25 PM, Andreas Jaeger wrote:
> On 05/24/2013 11:53 PM, Carlos O'Donell wrote:
>> It is very very possible that the futex syscall returns an
>> error and that the caller of lll_futex_wake may want to
>> look at that error and propagate the failure.
>>
>> In the current i386 and x86-64 implementations of pthread_once
>> we don't currently check for a failure in lll_futex_wake
>> but we *should* for the sake of correctness and robustness.
>> It's this kind of problem which eventually results in
>> hard to track down bugs in the nptl code.
>>
>> I first noticed this when I was testing removing the pthread_once
>> assembly implementation for x86 and x86_64 and looking at the
>> performance difference. Regardless of the results of that analysis,
>> which isn't done, and which Torvald is now initially tackling as the
>> pthread_once unification work[1], this is a good step forward.
>>
>> This patch is a first step and ensures that a status is returned
>> for the lll_futex_wake call so we can detect failures.
>>
>> No regressions in either x86 or x86-64.
>>
>> OK to commit?
> 
> Yes, looks fine to me,

Thanks, I'd forgotten about this patch :-)

I also verified that this change does not impact any of the
generated code for x86 and x86-64, principally because it's
not used. The x86* implementations use mostly hand-written
assembly (which I question the usefulness and correctness
of).

Checked in.

commit be11d713940133fd4200fb96db8200092b05325e
Author: Carlos O'Donell <carlos@redhat.com>
Date:   Mon Jun 10 12:05:11 2013 -0400

    x86*: Return syscall error for lll_futex_wake.
    
    It is very very possible that the futex syscall returns an
    error and that the caller of lll_futex_wake may want to
    look at that error and propagate the failure.
    
    This patch allows a caller to see the syscall error.
    
    There are no users of the syscall error at present, but
    future cleanups are now be able to check for the error.
    
    --
    
    nplt/
    
    2013-06-10  Carlos O'Donell  <carlos@redhat.com>
    
        * sysdeps/unix/sysv/linux/i386/lowlevellock.h
        (lll_futex_wake): Return syscall error.
        * sysdeps/unix/sysv/linux/x86_64/lowlevellock.h
        (lll_futex_wake): Return syscall error.

Cheers,
Carlos.