All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Paul E. McKenney" <paulmck@linux.ibm.com>
To: Valentin Schneider <valentin.schneider@arm.com>
Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	"Joel Fernandes, Google" <joel@joelfernandes.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Alan Stern <stern@rowland.harvard.edu>,
	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 16:00:34 -0700	[thread overview]
Message-ID: <20190817230034.GK28441@linux.ibm.com> (raw)
In-Reply-To: <600fd72f-11a0-ff1a-c87a-b26349f6f54a@arm.com>

On Sat, Aug 17, 2019 at 09:03:30PM +0100, Valentin Schneider wrote:
> Apologies to Steve for continuing this thread when all he wanted was moving
> an operation inside a mutex...
> 
> On 17/08/2019 16:02, Mathieu Desnoyers wrote:
> [...]
> > However, if the state of "x" can be any pointer value, or a reference
> > count value, then not using "WRITE_ONCE()" to store a constant leaves
> > the compiler free to perform that store in more than one memory access.
> > Based on [1], section "Store tearing", there are situations where this
> > happens on x86 in the wild today when storing 64-bit constants: the
> > compiler is then free to decide to use two 32-bit immediate store
> > instructions.
> > 
> 
> That's also how I understand things, and it's also one of the points raised
> in the compiler barrier section of memory-barriers.txt
> 
> Taking this store tearing, or the invented stores - e.g. the branch
> optimization pointed out by Linus:
> 
> >    if (a)
> >       global_var = 1
> >    else
> >       global_var = 0
> > 
> > then the compiler had better not turn that into
> > 
> >      global_var = 0
> >      if (a)
> >          global_var = 1
> 
> AFAICT nothing prevents this from happening inside a critical section (where
> the locking primitives provide the right barriers, but that's it). That's
> all fine when data is never accessed locklessly, but in the case of locked
> writes vs lockless reads, couldn't there be "leaks" of these transient
> states? In those cases we would want WRITE_ONCE() for the writes.
> 
> So going back to:
> 
> > 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().
> 
> AFAIU it's not always the case, since a lone READ_ONCE() could get transient
> values.

Linus noted that he believes that compilers for architectures supporting
Linux can be trusted to avoid store-to-load transformations, invented
stores, and unnecessary store tearing.  Should these appear, Linus would
report a bug against the compiler and expect it to be fixed.

> I'll be honest, it's not 100% clear to me when those optimizations can
> actually be done (maybe the branch thingy but the others are dubious), and
> it's even less clear when compilers *actually* do it - only that they have
> been reported to do it (so it's not made up).

There is significant unclarity inherent in the situation.  The standard
says one thing, different compilers do other things, and developers
often expect yet a third thing.  And sometimes things change over time,
for example, the ca. 2011 dictim against compilers inventing data races.

Hey, they didn't teach me this aspect of software development in school,
either.  ;-)

							Thanx, Paul


  reply	other threads:[~2019-08-17 23:00 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 [this message]
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=20190817230034.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 \
    --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.