This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH 2/2] PowerPC: libc single-thread lock optimization


Thanks for the review Will.


On 01-05-2014 06:11, Will Newton wrote:
> On 30 April 2014 14:57, Adhemerval Zanella <azanella@linux.vnet.ibm.com> wrote:
>> diff --git a/nptl/sysdeps/unix/sysv/linux/powerpc/lowlevellock.h b/nptl/sysdeps/unix/sysv/linux/powerpc/lowlevellock.h
>> index ab92c3f..38529a4 100644
>> --- a/nptl/sysdeps/unix/sysv/linux/powerpc/lowlevellock.h
>> +++ b/nptl/sysdeps/unix/sysv/linux/powerpc/lowlevellock.h
>> @@ -76,6 +76,16 @@
>>  # endif
>>  #endif
>>
>> +/* For internal libc.so lock calls in single-thread process we use normal
>> +   load/stores.  */
>> +#if !defined NOT_IN_libc || defined UP
> Where does this UP define come from? It seems to date back to the
> early days of NPTL but I don't know when it gets set.

In fact I didn't git into too much to find, I added just to keep more close to x86
implementation.  In fact I think it is safe to just drop its use.


>> +# define __lll_is_single_thread                                                      \
>> +  __glibc_likely (THREAD_GETMEM (THREAD_SELF, header.multiple_threads) == 0)
>> +#else
>> +# define __lll_is_single_thread (0)
>> +#endif
>> +
>> +
>>  #define lll_futex_wait(futexp, val, private) \
>>    lll_futex_timed_wait (futexp, val, NULL, private)
>>
>> @@ -205,7 +215,9 @@
>>  /* Set *futex to ID if it is 0, atomically.  Returns the old value */
>>  #define __lll_robust_trylock(futex, id) \
>>    ({ int __val;                                                                      \
>> -     __asm __volatile ("1:     lwarx   %0,0,%2" MUTEX_HINT_ACQ "\n"          \
>> +     if (!__lll_is_single_thread)                                            \
>> +       __asm __volatile (                                                    \
>> +                      "1:      lwarx   %0,0,%2" MUTEX_HINT_ACQ "\n"          \
>>                        "        cmpwi   0,%0,0\n"                             \
>>                        "        bne     2f\n"                                 \
>>                        "        stwcx.  %3,0,%2\n"                            \
>> @@ -214,6 +226,12 @@
>>                        : "=&r" (__val), "=m" (*futex)                         \
>>                        : "r" (futex), "r" (id), "m" (*futex)                  \
>>                        : "cr0", "memory");                                    \
>> +     else                                                                    \
>> +       {                                                                     \
>> +        __val = *futex;                                                      \
>> +        if (__val == 0)                                                      \
>> +          *futex = id;                                                       \
>> +       }                                                                     \
>>       __val;                                                                  \
>>    })
>>
>> @@ -237,8 +255,16 @@ extern int __lll_robust_lock_wait (int *futex, int private) attribute_hidden;
>>  #define lll_lock(lock, private) \
>>    (void) ({                                                                  \
>>      int *__futex = &(lock);                                                  \
>> -    if (__builtin_expect (atomic_compare_and_exchange_val_acq (__futex, 1, 0),\
>> -                         0) != 0)                                            \
>> +    int __tmp;                                                               \
>> +    if (!__lll_is_single_thread)                                             \
>> +      __tmp = atomic_compare_and_exchange_val_acq (__futex, 1, 0);           \
>> +    else                                                                     \
>> +      {                                                                              \
>> +       __tmp = *__futex;                                                     \
>> +       if (__tmp == 0)                                                       \
>> +         *__futex = 1;                                                       \
>> +      }                                                                              \
>> +    if (__builtin_expect (__tmp, 0) != 0)                                    \
> Should this be __glibc_unlikely? e.g.:
>
> __glibc_unlikely(__tmp != 0)

I will fix it, thanks.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]