All of lore.kernel.org
 help / color / mirror / Atom feed
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

  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.