This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH 2/2] PowerPC: libc single-thread lock optimization
- From: Adhemerval Zanella <azanella at linux dot vnet dot ibm dot com>
- To: libc-alpha at sourceware dot org
- Date: Thu, 01 May 2014 13:17:08 -0300
- Subject: Re: [PATCH 2/2] PowerPC: libc single-thread lock optimization
- Authentication-results: sourceware.org; auth=none
- References: <5361013D dot 9020005 at linux dot vnet dot ibm dot com> <CANu=DmjcqMy79AtyvOKcX5aQP=U5Rnk-qKO4ueZEeMPNFeo=sg at mail dot gmail dot com>
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.