From: Joel Fernandes <joel@joelfernandes.org>
To: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Cc: Peter Zijlstra <peterz@infradead.org>,
paulmck <paulmck@linux.ibm.com>, Will Deacon <will@kernel.org>,
Linus Torvalds <torvalds@linux-foundation.org>,
Thomas Gleixner <tglx@linutronix.de>,
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 15:03:31 -0400 [thread overview]
Message-ID: <20190821190331.GA113711@google.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
Fwiw, and, interestingly, on clang v7.0.1-8 (aarch64), I get a proper 64-bit
str with the above example (even when not using volatile):
0000000000000000 <nonvol>:
0: d28acf08 mov x8, #0x5678 // #22136
4: f2a24688 movk x8, #0x1234, lsl #16
8: f2cacf08 movk x8, #0x5678, lsl #32
c: f2e24688 movk x8, #0x1234, lsl #48
10: 90000009 adrp x9, 8 <nonvol+0x8>
14: 91000129 add x9, x9, #0x0
18: f9000128 str x8, [x9]
1c: d65f03c0 ret
test1.o: file format elf64-littleaarch64
And even with -O2 it is a single store:
Disassembly of section .text:
0000000000000000 <nonvol>:
0: d28acf09 mov x9, #0x5678 // #22136
4: f2a24689 movk x9, #0x1234, lsl #16
8: f2cacf09 movk x9, #0x5678, lsl #32
c: 90000008 adrp x8, 8 <nonvol+0x8>
10: f2e24689 movk x9, #0x1234, lsl #48
14: f9000109 str x9, [x8]
18: d65f03c0 ret
thanks,
- Joel
[...]
next prev parent reply other threads:[~2019-08-21 19:03 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
2019-08-21 19:03 ` Joel Fernandes [this message]
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=20190821190331.GA113711@google.com \
--to=joel@joelfernandes.org \
--cc=boqun.feng@gmail.com \
--cc=dhowells@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mathieu.desnoyers@efficios.com \
--cc=paulmck@linux.ibm.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.