[haiku-commits] Re: BRANCH pdziepak-github.lock_elision [a18aae1] in src/system/libroot/os/arch: m68k x86 x86_64 arm ppc

  • From: Pawel Dziepak <pdziepak@xxxxxxxxxxx>
  • To: haiku-commits@xxxxxxxxxxxxx
  • Date: Tue, 23 Jul 2013 20:43:16 +0200

2013/7/23 François Revol <revol@xxxxxxx>:
> On 23/07/2013 00:30, Axel Dörfler wrote:
>> On 07/23/2013 12:03 AM, Pawel Dziepak wrote:
>>>>> would be a good idea (while atomic_set{,64}_aligned() becomes
>>>>> atomic_set{,64}().).
>>>> Can't it return the old value as well to join the club? :-)
>>> Unfortunately, no. On x86(_64) to atomically set a value all we need
>>> is a simple mov instruction since all writes and reads to naturally
>>> aligned memory locations are guaranteed to be atomic. If we want to
>>> get the old value we need to use xchg instruction, which is more
>>> expensive than a simple mov. That's why I though about having both
>>> atomic_set() and atomic_xchg().
>>
>> Ah, now I see where you're coming from.
>> Since we added those calls ourselves, we're free to define how they
>> work, at least, and having them both sounds beneficial.
>>
>> However, 'xchg' would pretty much violate our coding style guide (and
>> sounds x86 specific, too), so atomic_exchange() or even
>> atomic_get_and_set() would be preferable.
>> I think atomic_set(), and atomic_set64() should be consistent, though,
>> so whatever you end up naming that exchange function, it should probably
>> exist in 32 and 64 bit versions.
>
>
> Since atomic_set() returns previous value as per the current API, you
> might want to use another name for the non-returning version indeed, how
> about atomic_put() ?

Having both atomic_set() and atomic_put() when one of them returns the
old value and the other one doesn't probably isn't a good idea. Names
atomic_get_and_set() and atomic_set() will definitely tell much better
what these functions actually do. There won't be any problem on API
level since the compiler will notice that void return value is not
ignore. Old binaries that use atomic_set() and expect it to return a
value would get garbage data though. However, I don't think that there
is much code outside the Haiku repository that uses directly these
functions and I don't think preserving ABI compatibility here is worth
ending up with a really bad names. Everybody has been warned that
anything new that Haiku introduces to API may be changed at any time.

2013/7/23 Ingo Weinhold <ingo_weinhold@xxxxxx>:
> BTW, I believe there's quite a bit of code that assumes that aligned 32 bit
> moves are atomic and just uses volatile assignments in that case.

There is also a matter of memory ordering and volatile assignments do
not do anything about that (that may be OK if the code doesn't have
any specific requirements about in which order memory access really
happens). atomic_get() and atomic_set() on the other hand issue
appropriate barriers so they are suitable for any read-copy-update and
similar scenarios.

Paweł

Other related posts: