This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] [RFC] nptl: use compare and exchange for lll_cond_lock
- From: Adhemerval Zanella <azanella at linux dot vnet dot ibm dot com>
- To: Bernie Ogden <bernie dot ogden at linaro dot org>
- Cc: "GNU C. Library" <libc-alpha at sourceware dot org>
- Date: Wed, 24 Sep 2014 11:00:26 -0300
- Subject: Re: [PATCH] [RFC] nptl: use compare and exchange for lll_cond_lock
- Authentication-results: sourceware.org; auth=none
- References: <5421B1C2 dot 9020509 at linux dot vnet dot ibm dot com> <CALE0ps308dtp7yTrrp7tCcst4RsrDgTrbS-eOqW=_+gNOZuz9A at mail dot gmail dot com>
On 24-09-2014 10:46, Bernie Ogden wrote:
> IIUC the two options are indeed equivalent - but wouldn't it be more
> efficient to synchronize on the non-comparing version?
Well, that is exactly the question I posed ;)
>
> On 23 September 2014 18:45, Adhemerval Zanella
> <azanella@linux.vnet.ibm.com> wrote:
>> While checking the generated code and macros used in generic lowlevellock.h,
>> I noted powerpc and other arch uses uses a compare and swap instead of a plain
>> exchange value on lll_cond_lock.
>>
>> I am not really sure which behavior would be desirable, since as far I could
>> they will have both the same side effects (since lll_cond_lock, different
>> from lll_lock, does not hold value of '1').
>>
>> So I am proposing this patch to sync default implementation for what mostly
>> architectures (ia64, ppc, s390, sparc, x86, hppa) uses for lll_cond_lock. I see
>> that only microblaze and sh (I am not sure about this one, I not well versed in
>> its assembly and I'm being guided by its comment) used the atomic_exchange_acq
>> instead.
>>
>> Checked on powerpc32 and powercp64 with my previous lowlevellock.h removal
>> patch.
>>
>> --
>>
>> * sysdeps/nptl/lowlevellock.h (__lll_cond_lock): Use
>> atomic_compare_and_exchange_val_acq instead of atomic_exchange_acq.
>>
>> ---
>>
>> diff --git a/sysdeps/nptl/lowlevellock.h b/sysdeps/nptl/lowlevellock.h
>> index 28f4ba3..ba22734 100644
>> --- a/sysdeps/nptl/lowlevellock.h
>> +++ b/sysdeps/nptl/lowlevellock.h
>> @@ -73,7 +73,8 @@ extern int __lll_robust_lock_wait (int *futex, int private) attribute_hidden;
>> ((void) \
>> ({ \
>> int *__futex = (futex); \
>> - if (__glibc_unlikely (atomic_exchange_acq (__futex, 2) != 0)) \
>> + if (__glibc_unlikely ( \
>> + atomic_compare_and_exchange_val_acq (__futex, 2, 0) != 0)) \
>> __lll_lock_wait (__futex, private); \
>> }))
>> #define lll_cond_lock(futex, private) __lll_cond_lock (&(futex), private)
>>