From: "Paul E. McKenney" <paulmck@linux.ibm.com> To: Linus Torvalds <torvalds@linux-foundation.org> Cc: Valentin Schneider <valentin.schneider@arm.com>, Joel Fernandes <joel@joelfernandes.org>, Thomas Gleixner <tglx@linutronix.de>, Alan Stern <stern@rowland.harvard.edu>, Mathieu Desnoyers <mathieu.desnoyers@efficios.com>, rostedt <rostedt@goodmis.org>, linux-kernel <linux-kernel@vger.kernel.org>, Peter Zijlstra <peterz@infradead.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: Sat, 17 Aug 2019 15:28:34 -0700 [thread overview] Message-ID: <20190817222834.GJ28441@linux.ibm.com> (raw) In-Reply-To: <CAHk-=wiOhiAJVU71968tAND6rrEJSaYPg7DXK6Y6iiz7_RJACw@mail.gmail.com> On Sat, Aug 17, 2019 at 01:28:48AM -0700, Linus Torvalds wrote: [ . . . ] > Put another way: a WRITE_ONCE() without a paired READ_ONCE() is almost > certainly pointless. "Your honor, I have no further questions at this time, but I reserve the right to recall this witness." Outside of things like MMIO (where one could argue that the corresponding READ_ONCE() is in the device firmware), the use cases I can imagine for WRITE_ONCE() with no READ_ONCE() are quite strange. For example, doing the WRITE_ONCE()s while read-holding a given lock and doing plain reads while write-holding that same lock. While at the same time being worried about store tearing and similar. Perhaps I am suffering a failure of imagination, but I am not seeing a reasonable use for such things at the moment. > But the reverse is not really true. All a READ_ONCE() says is "I want > either the old or the new value", and it can get that _without_ being > paired with a WRITE_ONCE(). > > See? They just aren't equally important. > > > > And yes, reads are different from writes. Reads don't have the same > > > kind of "other threads of execution can see them" effects, so a > > > compiler turning a single read into multiple reads is much more > > > realistic and not the same kind of "we need to expect a certain kind > > > of sanity from the compiler" issue. > > > > Though each of those compiler-generated multiple reads might return a > > different value, right? > > Right. See the examples I have in the email to Mathieu: > > unsigned int bits = some_global_value; > ...test different bits in in 'bits' ... > > can easily cause multiple reads (particularly on a CPU that has a > "test bits in memory" instruction and a lack of registers. > > So then doing it as > > unsigned int bits = READ_ONCE(some_global_value); > .. test different bits in 'bits'... > > makes a real and obvious semantic difference. In ways that changing a one-time > > ptr->flag = true; > > to > > WRITE_ONCE(ptr->flag, true); > > does _not_ make any obvious semantic difference what-so-ever. Agreed, especially given that only one bit of ->flag is most likely ever changing. > Caching reads is also something that makes sense and is common, in > ways that caching writes does not. So doing > > while (in_progress_global) /* twiddle your thumbs */; > > obviously trivially translates to an infinite loop with a single > conditional in front of it, in ways that > > while (READ_ONCE(in_progress_global)) /* twiddle */; > > does not. > > So there are often _obvious_ reasons to use READ_ONCE(). > > I really do not find the same to be true of WRITE_ONCE(). There are > valid uses, but they are definitely much less common, and much less > obvious. Agreed, and I expect READ_ONCE() to continue to be used more heavily than is WRITE_ONCE(), even including the documentation-only WRITE_ONCE() usage. Thanx, Paul
next prev parent reply other threads:[~2019-08-17 22:29 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 [this message] 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 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=20190817222834.GJ28441@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 \ --subject='Re: [PATCH 1/1] Fix: trace sched switch start/stop racy updates' \ /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
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.