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: Bernie Ogden <bernie dot ogden at linaro dot org>
- To: Adhemerval Zanella <azanella at linux dot vnet dot ibm dot com>
- Cc: "GNU C. Library" <libc-alpha at sourceware dot org>
- Date: Wed, 24 Sep 2014 14:46:11 +0100
- 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>
IIUC the two options are indeed equivalent - but wouldn't it be more
efficient to synchronize on the non-comparing version?
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)
>