From: "Paul E. McKenney" <paulmck@linux.ibm.com>
To: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Cc: Peter Zijlstra <peterz@infradead.org>,
Will Deacon <will@kernel.org>,
Linus Torvalds <torvalds@linux-foundation.org>,
Thomas Gleixner <tglx@linutronix.de>,
"Joel Fernandes, Google" <joel@joelfernandes.org>,
Alan Stern <stern@rowland.harvard.edu>,
rostedt <rostedt@goodmis.org>,
Valentin Schneider <valentin.schneider@arm.com>,
linux-kernel <linux-kernel@vger.kernel.org>,
Boqun Feng <boqun.feng@gmail.com>,
Will Deacon <will.deacon@arm.com>,
David Howells <dhowells@redhat.com>
Subject: Re: [PATCH 1/1] Fix: trace sched switch start/stop racy updates
Date: Wed, 21 Aug 2019 09:14:26 -0700 [thread overview]
Message-ID: <20190821161426.GK28441@linux.ibm.com> (raw)
In-Reply-To: <52600272.1909.1566402523680.JavaMail.zimbra@efficios.com>
On Wed, Aug 21, 2019 at 11:48:43AM -0400, Mathieu Desnoyers wrote:
> ----- On Aug 21, 2019, at 8:33 AM, Peter Zijlstra peterz@infradead.org wrote:
>
> > On Wed, Aug 21, 2019 at 06:23:10AM -0700, Paul E. McKenney wrote:
> >> On Wed, Aug 21, 2019 at 11:32:01AM +0100, Will Deacon wrote:
> >
> >> > and so it is using a store-pair instruction to reduce the complexity in
> >> > the immediate generation. Thus, the 64-bit store will only have 32-bit
> >> > atomicity. In fact, this is scary because if I change bar to:
> >> >
> >> > void bar(u64 *x)
> >> > {
> >> > *(volatile u64 *)x = 0xabcdef10abcdef10;
> >> > }
> >> >
> >> > then I get:
> >> >
> >> > bar:
> >> > mov w1, 61200
> >> > movk w1, 0xabcd, lsl 16
> >> > str w1, [x0]
> >> > str w1, [x0, 4]
> >> > ret
> >> >
> >> > so I'm not sure that WRITE_ONCE would even help :/
> >>
> >> Well, I can have the LWN article cite your email, then. So thank you
> >> very much!
> >>
> >> Is generation of this code for a 64-bit volatile store considered a bug?
> >> Or does ARMv8 exclude the possibility of 64-bit MMIO registers? And I
> >> would guess that Thomas and Linus would ask a similar bugginess question
> >> for normal stores. ;-)
> >
> > I'm calling this a compiler bug; the way I understand volatile this is
> > very much against the intentended use case. That is, this is buggy even
> > on UP vs signals or MMIO.
>
> And here is a simpler reproducer on my gcc-8.3.0 (aarch64) compiled with O2:
>
> volatile unsigned long a;
>
> void fct(void)
> {
> a = 0x1234567812345678ULL;
> }
>
> void fct(void)
> {
> a = 0x1234567812345678ULL;
> 0: 90000000 adrp x0, 8 <fct+0x8>
> 4: 528acf01 mov w1, #0x5678 // #22136
> 8: 72a24681 movk w1, #0x1234, lsl #16
> c: f9400000 ldr x0, [x0]
> 10: b9000001 str w1, [x0]
> 14: b9000401 str w1, [x0, #4]
> }
> 18: d65f03c0 ret
>
> And the non-volatile case uses stp (is it a single store to memory ?):
>
> unsigned long a;
>
> void fct(void)
> {
> a = 0x1234567812345678ULL;
> }
>
> void fct(void)
> {
> a = 0x1234567812345678ULL;
> 0: 90000000 adrp x0, 8 <fct+0x8>
> 4: 528acf01 mov w1, #0x5678 // #22136
> 8: 72a24681 movk w1, #0x1234, lsl #16
> c: f9400000 ldr x0, [x0]
> 10: 29000401 stp w1, w1, [x0]
> }
> 14: d65f03c0 ret
>
> It would probably be a good idea to audit other architectures, since this
> is done by the compiler backend.
That does seem like a very good idea!
Thanx, Paul
next prev parent reply other threads:[~2019-08-21 16:14 UTC|newest]
Thread overview: 60+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-05-18 10:29 WARNING in tracepoint_probe_register_prio (3) syzbot
2019-08-16 0:11 ` syzbot
2019-08-16 14:26 ` [PATCH 1/1] Fix: trace sched switch start/stop racy updates Mathieu Desnoyers
2019-08-16 16:25 ` Steven Rostedt
2019-08-16 16:48 ` Valentin Schneider
2019-08-16 17:04 ` Steven Rostedt
2019-08-16 17:41 ` Mathieu Desnoyers
2019-08-16 19:18 ` Steven Rostedt
2019-08-16 19:19 ` Alan Stern
2019-08-16 20:44 ` Joel Fernandes
2019-08-16 20:49 ` Thomas Gleixner
2019-08-16 20:57 ` Joel Fernandes
2019-08-16 22:27 ` Valentin Schneider
2019-08-16 22:57 ` Linus Torvalds
2019-08-17 1:41 ` Mathieu Desnoyers
2019-08-17 4:52 ` Paul E. McKenney
2019-08-17 8:28 ` Linus Torvalds
2019-08-17 8:44 ` Linus Torvalds
2019-08-17 15:02 ` Mathieu Desnoyers
2019-08-17 20:03 ` Valentin Schneider
2019-08-17 23:00 ` Paul E. McKenney
2019-08-19 10:34 ` Valentin Schneider
2019-08-17 22:28 ` Paul E. McKenney
2019-08-20 14:01 ` Peter Zijlstra
2019-08-20 20:31 ` Paul E. McKenney
2019-08-20 20:39 ` Peter Zijlstra
2019-08-20 20:52 ` Paul E. McKenney
2019-08-16 21:04 ` Linus Torvalds
2019-08-17 1:36 ` Mathieu Desnoyers
2019-08-17 2:13 ` Steven Rostedt
2019-08-17 14:40 ` Mathieu Desnoyers
2019-08-17 15:26 ` Steven Rostedt
2019-08-17 15:55 ` Mathieu Desnoyers
2019-08-17 16:40 ` Steven Rostedt
2019-08-17 22:06 ` Paul E. McKenney
2019-08-17 8:08 ` Linus Torvalds
2019-08-20 13:56 ` Peter Zijlstra
2019-08-20 20:29 ` Paul E. McKenney
2019-08-21 10:32 ` Will Deacon
2019-08-21 13:23 ` Paul E. McKenney
2019-08-21 13:32 ` Will Deacon
2019-08-21 13:56 ` Paul E. McKenney
2019-08-21 16:22 ` Will Deacon
2019-08-21 15:33 ` Peter Zijlstra
2019-08-21 15:48 ` Mathieu Desnoyers
2019-08-21 16:14 ` Paul E. McKenney [this message]
2019-08-21 19:03 ` Joel Fernandes
2019-09-09 6:21 ` Herbert Xu
2019-08-16 20:49 ` Steven Rostedt
2019-08-16 20:59 ` Joel Fernandes
2019-08-17 1:25 ` Mathieu Desnoyers
2019-08-18 9:15 ` stable markup was " Pavel Machek
2019-08-16 17:19 ` Mathieu Desnoyers
2019-08-16 19:15 ` Steven Rostedt
2019-08-17 14:27 ` Mathieu Desnoyers
2019-08-17 15:42 ` Steven Rostedt
2019-08-17 15:53 ` Mathieu Desnoyers
2019-08-17 16:43 ` Steven Rostedt
2019-08-16 12:32 ` WARNING in tracepoint_probe_register_prio (3) syzbot
2019-08-16 12:41 ` Sebastian Andrzej Siewior
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=20190821161426.GK28441@linux.ibm.com \
--to=paulmck@linux.ibm.com \
--cc=boqun.feng@gmail.com \
--cc=dhowells@redhat.com \
--cc=joel@joelfernandes.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mathieu.desnoyers@efficios.com \
--cc=peterz@infradead.org \
--cc=rostedt@goodmis.org \
--cc=stern@rowland.harvard.edu \
--cc=tglx@linutronix.de \
--cc=torvalds@linux-foundation.org \
--cc=valentin.schneider@arm.com \
--cc=will.deacon@arm.com \
--cc=will@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.