Re: [PATCH 2/3] asm-generic, x86: wrap atomic operations

[Dmitry Vyukov]

On Tue, Mar 28, 2017 at 11:27 AM, Peter Zijlstra <peterz@xxxxxxxxxxxxx> wrote:
> On Tue, Mar 28, 2017 at 09:52:32AM +0200, Ingo Molnar wrote:
>
>> No, regular C code.
>>
>> I don't see the point of generating all this code via CPP - it's certainly not
>> making it more readable to me. I.e. this patch I commented on is a step backwards
>> for readability.
>
> Note that much of the atomic stuff we have today is all CPP already.
>
> x86 is the exception because its 'weird', but most other archs are
> almost pure CPP -- check Alpha for example, or asm-generic/atomic.h.
>
> Also, look at linux/atomic.h, its a giant maze of CPP.
>
> The CPP help us generate functions, reduces endless copy/paste (which
> induces random differences -- read bugs) and construct variants
> depending on the architecture input.
>
> Yes, the CPP is a pain, but writing all that out explicitly is more of a
> pain.
>
>
>
> I've not yet looked too hard at these patches under consideration; and I
> really wish we could get the compiler to do the right thing here, but
> reducing the endless copy/paste that's otherwise the result of this, is
> something I've found to be very valuable.
>
> Not to mention that adding additional atomic ops got trivial (the set is
> now near complete, so that's not much of an argument anymore -- but it
> was, its what kept me sane sanitizing the atomic ops across all our 25+
> architectures).


I am almost done with Ingo's proposal, including de-macro-ifying x86
atomic ops code.
I am ready to do either of them, I think both have pros and cons and
there is no perfect solution. But please agree on something.


While we are here, one thing that I noticed is that 32-bit atomic code
uses 'long long' for 64-bit operands, while 64-bit code uses 'long'
for 64-bit operands. This sorta worked more of less before, but
ultimately it makes it impossible to write any portable code (e.g. you
don't know what format specifier to use to print return value of
atomic64_read, nor what local variable type to use to avoid compiler
warnings). With the try_cmpxchg it become worse, because 'long*' is
not convertible to 'long long*' so it is not possible to write any
portable code that uses it. If you declare 'old' variable as 'long'
32-bit code won't compile, if you declare it as 'long long' 64-bit
code won't compiler. I think we need to switch to a single type for
64-bit operands/return values, e.g. 'long long'.