This is the mail archive of the libc-ports@sources.redhat.com mailing list for the libc-ports 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] Add explicit acquire/release semantics to atomic_exchange_and_add.


On 7/11/2012 5:50 AM, Maxim Kuvyrkov wrote:
> On 29/06/2012, at 11:00 AM, Joseph S. Myers wrote:
> 
>> On Thu, 28 Jun 2012, Maxim Kuvyrkov wrote:
>>
>>> +/* ??? Barrier semantics for atomic_exchange_and_add appear to be
>>> +   undefined.  Use full barrier for now, as that's safe.  */
>>
>> Please file a bug to clarify these semantics, if not already filed, and 
>> reference it in the comment.  (Clarifying the semantics will I suppose 
>> involve examining both direct and indirect users of 
>> atomic_exchange_and_add to work out what they need and whether it should 
>> be split into multiple macros with different barrier semantics.)
> 
> This is now http://sourceware.org/bugzilla/show_bug.cgi?id=14350 .
> 
> Current generic implementation in include/atomic.h is based on atomic_compare_and_exchange_acq, so it may be that atomic_exchange_and_add implies acquire, but not release semantics.  However, I doubt that the generic implementation was exhaustively tested on multi-processor systems, so we should not blindly depend on this.

I've not seen any *exhaustive* testing of anything :-)

> As a first step here are patches to add atomic_exchange_and_add_{acq,rel} variants, which then will be used in upcoming optimizations to __libc_lock_lock/__libc_lock_trylock macros and pthread_spin_lock/pthread_spin_trylock implementations.
> 
> Tested on mips-linux-gnu.
> 
> OK to apply?
> 
> --
> Maxim Kuvyrkov
> CodeSourcery / Mentor Graphics
> 
> Add explicit acquire/release semantics to atomic_exchange_and_add.
> 
> 	2012-07-11  Maxim Kuvyrkov  <maxim@codesourcery.com>
> 
> 	* include/atomic.h (atomic_exchange_and_add): Split into ...
> 	(atomic_exchange_and_add_acq, atomic_exchange_and_add_rel): ... these.
> 	New atomic macros.

This looks good to me.

We should have been explicit about the semantics in the first place.

Cheers,
Carlos.
-- 
Carlos O'Donell
Mentor Graphics / CodeSourcery
carlos_odonell@mentor.com
carlos@codesourcery.com
+1 (613) 963 1026


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