[PATCH 1/2] [COMMITTED] Optimize generic spinlock code and use C11 like atomic macros.

[Stefan Liebler]

On 05/31/2017 06:48 PM, Torvald Riegel wrote:
> On Wed, 2017-05-31 at 10:29 +0200, Stefan Liebler wrote:
>> On 05/30/2017 09:18 AM, Torvald Riegel wrote:
>>> On Wed, 2017-05-10 at 15:00 +0200, Stefan Liebler wrote:
>>>> On 05/03/2017 01:38 PM, Stefan Liebler wrote:
>>>>> On 04/25/2017 08:46 AM, Stefan Liebler wrote:
>>>>>> On 04/18/2017 03:09 PM, Stefan Liebler wrote:
>>>>>>> On 04/10/2017 01:59 PM, Stefan Liebler wrote:
>>>>>>>> On 04/09/2017 03:51 PM, Torvald Riegel wrote:
>>>>>>>>> On Fri, 2017-04-07 at 18:22 +0200, Stefan Liebler wrote:
>>>>>>>>>> @architecture maintainers:
>>>>>>>>>> I've added defines of ATOMIC_EXCHANGE_USES_CAS in the architecture
>>>>>>>>>> specific atomic-machine.h files.
>>>>>>>>>> See comment in include/atomic.h:
>>>>>>>>>> /* ATOMIC_EXCHANGE_USES_CAS is equal to 1 if atomic_exchange
>>>>>>>>>> operations
>>>>>>>>>>       are implemented based on a CAS loop; otherwise, this is 0 and we
>>>>>>>>>> assume
>>>>>>>>>>       that the atomic_exchange operations could provide better
>>>>>>>>>> performance
>>>>>>>>>>       than a CAS loop.  */
>>>>>>>>>>
>>>>>>>>>> Can review the definition to 0 or 1 in the atomic-machine.h file of
>>>>>>>>>> your
>>>>>>>>>> architecture, please?
>>>>>>>
>>>>>>>
>>>>>>> PING
>>>>>>>
>>>>>> PING
>>>>>
>>>>> PING
>>>>
>>>> PING
>>>>
>>>> @Torvald:
>>>> I'm not sure if we will get answers from everybody.
>>>> What do you propose how to proceed with the definitions of
>>>> ATOMIC_EXCHANGE_USES_CAS?
>>>
>>> Which archs are still missing?  If maintainers don't reply, I suggest we
>>> do our best to figure out what's the case for each missing arch, and add
>>> a note that it should be checked eventually (eg, /* XXX Is this actually
>>> correct?  */) to the code, and then commit.  Please CC: me on such a
>>> patch, so I can have a last look at it.
>>>
>>
>> We are sure for these archs:
>> -aarch64: #define ATOMIC_EXCHANGE_USES_CAS 0
>> -i386/x86: #define ATOMIC_EXCHANGE_USES_CAS 0
>> -mips: #define ATOMIC_EXCHANGE_USES_CAS 0/1
>> -powerpc: #define ATOMIC_EXCHANGE_USES_CAS 1
>> -s390: #define ATOMIC_EXCHANGE_USES_CAS 1
>> -tile: #define ATOMIC_EXCHANGE_USES_CAS 0
>>
>> Nobody answered for these archs:
>> -alpha: #define ATOMIC_EXCHANGE_USES_CAS 1
>> -arm: #define ATOMIC_EXCHANGE_USES_CAS 1
>> -hppa: #define ATOMIC_EXCHANGE_USES_CAS 1
>> -ia64: #define ATOMIC_EXCHANGE_USES_CAS 0
>> -m68k: #define ATOMIC_EXCHANGE_USES_CAS 1
>> -microblaze: #define ATOMIC_EXCHANGE_USES_CAS 1
>> -nios2: #define ATOMIC_EXCHANGE_USES_CAS 1
>> -sh: #define ATOMIC_EXCHANGE_USES_CAS 1
>> -sparc: #define ATOMIC_EXCHANGE_USES_CAS 1
> 
> Joseph, can you clarify for arm?
> 
removed the XXX comment for arm (see post 
https://www.sourceware.org/ml/libc-alpha/2017-06/msg00028.html)

>> In the two of three sparc files ...
>> ./sysdeps/sparc/sparc64/atomic-machine.h
>> ./sysdeps/sparc/sparc32/sparcv9/atomic-machine.h
>> ... there is the swap instruction in macro atomic_exchange_acq
>> for 4 bytes.  Other sizes are aborted or done by CAS.
>> Shall we use #define ATOMIC_EXCHANGE_USES_CAS 0 here?
> 
> I suggest we let the sparc maintainer take care of that (though we could
> set it to 0 for sparc32 I guess).
>
Set it to 0 in ./sysdeps/sparc/sparc32/sparcv9/atomic-machine.h.


>>
>> I've attached the current version with
>> /* XXX Is this actually correct?  */
>> for all architecture for which we didn't get an answer.
>>
>> The second patch "S390: Use generic spinlock code." remains the same.
> 
> I'd wait for a few more days to give Joseph time to respond, and then
> commit if there are no objections or further input.  I think maintainers
> had enough time to comment, and you pinged them often enough already.
> 
> Thanks for being patient! :)
> 
Committed.

Thanks.
Stefan