From: Dmitry Vyukov <dvyukov@google.com>
To: Peter Zijlstra <peterz@infradead.org>
Cc: Ingo Molnar <mingo@kernel.org>,
Mark Rutland <mark.rutland@arm.com>,
Andrey Ryabinin <aryabinin@virtuozzo.com>,
Ingo Molnar <mingo@redhat.com>, Will Deacon <will.deacon@arm.com>,
Andrew Morton <akpm@linux-foundation.org>,
kasan-dev <kasan-dev@googlegroups.com>,
"linux-mm@kvack.org" <linux-mm@kvack.org>,
"x86@kernel.org" <x86@kernel.org>,
LKML <linux-kernel@vger.kernel.org>,
Linus Torvalds <torvalds@linux-foundation.org>,
Thomas Gleixner <tglx@linutronix.de>,
"H. Peter Anvin" <hpa@zytor.com>
Subject: Re: [PATCH 2/3] asm-generic, x86: wrap atomic operations
Date: Tue, 28 Mar 2017 11:46:06 +0200 [thread overview]
Message-ID: <CACT4Y+Yww1C03nEHj3UNGaffSYDhWo1b8VjDcCP88YOQdZv+=w@mail.gmail.com> (raw)
In-Reply-To: <20170328092712.bk32k5iteqqm6pgh@hirez.programming.kicks-ass.net>
On Tue, Mar 28, 2017 at 11:27 AM, Peter Zijlstra <peterz@infradead.org> 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'.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
next prev parent reply other threads:[~2017-03-28 9:46 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-03-14 19:24 [PATCH 0/3] x86, kasan: add KASAN checks to " Dmitry Vyukov
2017-03-14 19:24 ` [PATCH 1/3] kasan: allow kasan_check_read/write() to accept pointers to volatiles Dmitry Vyukov
2017-03-14 19:24 ` [PATCH 2/3] asm-generic, x86: wrap atomic operations Dmitry Vyukov
2017-03-20 16:41 ` Andrey Ryabinin
2017-03-20 17:17 ` Mark Rutland
2017-03-21 9:25 ` Andrey Ryabinin
2017-03-21 10:41 ` Mark Rutland
2017-03-21 18:06 ` Dmitry Vyukov
2017-03-21 21:20 ` Arnd Bergmann
2017-03-22 10:42 ` Dmitry Vyukov
2017-03-22 11:30 ` Arnd Bergmann
2017-03-22 12:14 ` Dmitry Vyukov
2017-03-22 12:48 ` Arnd Bergmann
2017-03-24 6:52 ` Ingo Molnar
2017-03-24 7:14 ` Dmitry Vyukov
2017-03-24 8:39 ` Dmitry Vyukov
2017-03-24 10:57 ` Ingo Molnar
2017-03-24 12:46 ` Dmitry Vyukov
2017-03-28 7:52 ` Ingo Molnar
2017-03-28 9:27 ` Peter Zijlstra
2017-03-28 9:46 ` Dmitry Vyukov [this message]
2017-03-28 9:51 ` Ingo Molnar
2017-03-28 9:56 ` Dmitry Vyukov
2017-03-28 10:15 ` Ingo Molnar
2017-03-28 16:29 ` Dmitry Vyukov
2017-03-14 19:24 ` [PATCH 3/3] asm-generic: add KASAN instrumentation to " Dmitry Vyukov
2017-03-30 22:30 ` [PATCH 0/3] x86, kasan: add KASAN checks " Andrew Morton
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CACT4Y+Yww1C03nEHj3UNGaffSYDhWo1b8VjDcCP88YOQdZv+=w@mail.gmail.com' \
--to=dvyukov@google.com \
--cc=akpm@linux-foundation.org \
--cc=aryabinin@virtuozzo.com \
--cc=hpa@zytor.com \
--cc=kasan-dev@googlegroups.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=mark.rutland@arm.com \
--cc=mingo@kernel.org \
--cc=mingo@redhat.com \
--cc=peterz@infradead.org \
--cc=tglx@linutronix.de \
--cc=torvalds@linux-foundation.org \
--cc=will.deacon@arm.com \
--cc=x86@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox