From: Linus Torvalds <firstname.lastname@example.org> To: "Paul E. McKenney" <email@example.com> Cc: Valentin Schneider <firstname.lastname@example.org>, Joel Fernandes <email@example.com>, Thomas Gleixner <firstname.lastname@example.org>, Alan Stern <email@example.com>, Mathieu Desnoyers <firstname.lastname@example.org>, rostedt <email@example.com>, linux-kernel <firstname.lastname@example.org>, Peter Zijlstra <email@example.com>, Boqun Feng <firstname.lastname@example.org>, Will Deacon <email@example.com>, David Howells <firstname.lastname@example.org> Subject: Re: [PATCH 1/1] Fix: trace sched switch start/stop racy updates Date: Sat, 17 Aug 2019 01:28:48 -0700 [thread overview] Message-ID: <CAHk-=wiOhiAJVU71968tAND6rrEJSaYPg7DXK6Y6iiz7_RJACw@mail.gmail.com> (raw) In-Reply-To: <20190817045217.GZ28441@linux.ibm.com> On Fri, Aug 16, 2019 at 9:52 PM Paul E. McKenney <email@example.com> wrote: > > > I'd love to have a flag that says "all undefined behavior is treated > > as implementation-defined". There's a somewhat subtle - but very > > important - difference there. > > It would also be nice to downgrade some of the undefined behavior in > the standard itself. Compiler writers usually hate this because it > would require them to document what their compiler does in each case. > So they would prefer "unspecified" if the could not have "undefined". That certainly would sound very good to me. It's not the "documented behavior" that is important to me (since it is still going to be potentially different across different platforms), it's the "at least it's *some* consistent behavior for that platform". That's just _so_ much better than "undefined behavior" which basically says the compiler writer can do absolutely anything, whether it makes sense or not, and whether it might open a small bug up to absolutely catastrophic end results. > > if (a) > > global_var = 1 > > else > > global_var = 0 > > Me, I would still want to use WRITE_ONCE() in this case. I actually suspect that if we had this kind of code, and a real reason why readers would see it locklessly, then yes, WRITE_ONCE() makes sense. But most of the cases where people add WRITE_ONCE() aren't even the above kind of half-questionable cases. They are just literally WRITE_ONCE(flag, value); and since there is no real memory ordering implied by this, what's the actual value of that statement? What problem does it really solve wrt just writing it as flag = value; which is generally a lot easier to read. If the flag has semantic behavior wrt other things, and the write can race with a read (whether it's the read or the write that is unlocked is irrelevant), is still doesn't tend to make a lot of real difference. In the end, the most common reason for a WRITE_ONCE() is mostly just "to visually pair up with the non-synchronized read that uses READ_ONCE()" Put another way: a WRITE_ONCE() without a paired READ_ONCE() is almost certainly pointless. 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. 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. Linus
next prev parent reply other threads:[~2019-08-17 8: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 [this message] 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 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='CAHk-=wiOhiAJVU71968tAND6rrEJSaYPg7DXK6Y6iiz7_RJACw@mail.gmail.com' \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --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.