From: Linus Torvalds <torvalds@linux-foundation.org> To: Alan Stern <stern@rowland.harvard.edu> Cc: "Paul E. McKenney" <paulmck@linux.ibm.com>, Boqun Feng <boqun.feng@gmail.com>, Herbert Xu <herbert@gondor.apana.org.au>, Frederic Weisbecker <fweisbec@gmail.com>, Fengguang Wu <fengguang.wu@intel.com>, LKP <lkp@01.org>, LKML <linux-kernel@vger.kernel.org>, Netdev <netdev@vger.kernel.org>, "David S. Miller" <davem@davemloft.net>, Andrea Parri <andrea.parri@amarulasolutions.com>, Luc Maranget <luc.maranget@inria.fr>, Jade Alglave <j.alglave@ucl.ac.uk> Subject: Re: rcu_read_lock lost its compiler barrier Date: Tue, 4 Jun 2019 09:04:55 -0700 [thread overview] Message-ID: <CAHk-=wgGnCw==uY8radrB+Tg_CEmzOtwzyjfMkuh7JmqFh+jzQ@mail.gmail.com> (raw) In-Reply-To: <Pine.LNX.4.44L0.1906041026570.1731-100000@iolanthe.rowland.org> On Tue, Jun 4, 2019 at 7:44 AM Alan Stern <stern@rowland.harvard.edu> wrote: > > Even if you don't think the compiler will ever do this, the C standard > gives compilers the right to invent read accesses if a plain (i.e., > non-atomic and non-volatile) write is present. Note that for the kernel, it's not like we go strictly by what the standard says anyway. The reason for the "read for write" thing is that obviously you sometimes have broken architectures (*cough*alpha*cough*) that need to do some common writes are read-maskmodify-write accesses, and for bitfields you obviously *always* have that issue. In general, a sane compiler (as opposed to "we just read the standard") had better not add random reads, because people might have some mappings be write-only when the hardware supports it. And we do generally expect sanity from our compilers, and will add compiler flags to disable bad behavior if required - even if said behavior would be "technically allowed by the standard". > (Incidentally, regardless of whether the compiler will ever do this, I > have seen examples in the kernel where people did exactly this > manually, in order to avoid dirtying a cache line unnecessarily.) I really hope and expect that this is not something that the compiler ever does. If a compiler turns "x = y" into if (x != y) x = y" (like we do manually at times, as you say), that compiler is garbage that we cannot use for the kernel. It would break things like "smp_wmb()" ordering guarantees, I'm pretty sure. And as always, if we're doing actively stupid things, and the compiler then turns our stupid code into something we don't expect, the corollary is that then it's on _us_. IOW, if we do if (x != 1) { ... } x = 1; and the compiler goes "oh, we already checked that 'x == 1'" and moves that "unconditional" 'x = 1' into the conditional section like if (x != 1) { .. x = 1; } then that's not something we can then complain about. So our expectation is that the compiler is _sane_, not that it's some "C as assembler". Adding spurious reads is not ok. But if we already had reads in the code and the compiler combines them with other ops, that's on us. End result: in general, we do expect that the compiler turns a regular assignment into a single plain write when that's what the code does, and does not add extra logic over and beyond that. In fact, the alpha port was always subtly buggy exactly because of the "byte write turns into a read-and-masked-write", even if I don't think anybody ever noticed (we did fix cases where people _did_ notice, though, and we might still have some cases where we use 'int' for booleans because of alpha issues.). So I don't technically disagree with anything you say, I just wanted to point out that as far as the kernel is concerned, we do have higher quality expectations from the compiler than just "technically valid according to the C standard". Linus
WARNING: multiple messages have this Message-ID (diff)
From: Linus Torvalds <torvalds@linux-foundation.org> To: lkp@lists.01.org Subject: Re: rcu_read_lock lost its compiler barrier Date: Tue, 04 Jun 2019 09:04:55 -0700 [thread overview] Message-ID: <CAHk-=wgGnCw==uY8radrB+Tg_CEmzOtwzyjfMkuh7JmqFh+jzQ@mail.gmail.com> (raw) In-Reply-To: <Pine.LNX.4.44L0.1906041026570.1731-100000@iolanthe.rowland.org> [-- Attachment #1: Type: text/plain, Size: 3059 bytes --] On Tue, Jun 4, 2019 at 7:44 AM Alan Stern <stern@rowland.harvard.edu> wrote: > > Even if you don't think the compiler will ever do this, the C standard > gives compilers the right to invent read accesses if a plain (i.e., > non-atomic and non-volatile) write is present. Note that for the kernel, it's not like we go strictly by what the standard says anyway. The reason for the "read for write" thing is that obviously you sometimes have broken architectures (*cough*alpha*cough*) that need to do some common writes are read-maskmodify-write accesses, and for bitfields you obviously *always* have that issue. In general, a sane compiler (as opposed to "we just read the standard") had better not add random reads, because people might have some mappings be write-only when the hardware supports it. And we do generally expect sanity from our compilers, and will add compiler flags to disable bad behavior if required - even if said behavior would be "technically allowed by the standard". > (Incidentally, regardless of whether the compiler will ever do this, I > have seen examples in the kernel where people did exactly this > manually, in order to avoid dirtying a cache line unnecessarily.) I really hope and expect that this is not something that the compiler ever does. If a compiler turns "x = y" into if (x != y) x = y" (like we do manually at times, as you say), that compiler is garbage that we cannot use for the kernel. It would break things like "smp_wmb()" ordering guarantees, I'm pretty sure. And as always, if we're doing actively stupid things, and the compiler then turns our stupid code into something we don't expect, the corollary is that then it's on _us_. IOW, if we do if (x != 1) { ... } x = 1; and the compiler goes "oh, we already checked that 'x == 1'" and moves that "unconditional" 'x = 1' into the conditional section like if (x != 1) { .. x = 1; } then that's not something we can then complain about. So our expectation is that the compiler is _sane_, not that it's some "C as assembler". Adding spurious reads is not ok. But if we already had reads in the code and the compiler combines them with other ops, that's on us. End result: in general, we do expect that the compiler turns a regular assignment into a single plain write when that's what the code does, and does not add extra logic over and beyond that. In fact, the alpha port was always subtly buggy exactly because of the "byte write turns into a read-and-masked-write", even if I don't think anybody ever noticed (we did fix cases where people _did_ notice, though, and we might still have some cases where we use 'int' for booleans because of alpha issues.). So I don't technically disagree with anything you say, I just wanted to point out that as far as the kernel is concerned, we do have higher quality expectations from the compiler than just "technically valid according to the C standard". Linus
next prev parent reply other threads:[~2019-06-04 16:05 UTC|newest] Thread overview: 99+ messages / expand[flat|nested] mbox.gz Atom feed top 2015-09-10 0:57 [rcu] kernel BUG at include/linux/pagemap.h:149! Fengguang Wu 2015-09-10 0:57 ` Fengguang Wu 2015-09-10 10:25 ` Boqun Feng 2015-09-10 17:16 ` Paul E. McKenney 2015-09-10 17:16 ` Paul E. McKenney 2015-09-11 2:19 ` Boqun Feng [not found] ` <CAJzB8QG=1iZW3dQEie6ZSTLv8GZ3YSut0aL1VU7LLmiHQ1B1DQ@mail.gmail.com> 2015-09-11 21:59 ` Paul E. McKenney 2015-09-11 21:59 ` Paul E. McKenney 2015-09-12 5:46 ` Boqun Feng 2015-09-21 19:30 ` Frederic Weisbecker 2015-09-21 19:30 ` Frederic Weisbecker 2015-09-21 20:43 ` Paul E. McKenney 2015-09-21 20:43 ` Paul E. McKenney 2019-06-02 5:56 ` rcu_read_lock lost its compiler barrier Herbert Xu 2019-06-02 5:56 ` Herbert Xu 2019-06-02 20:54 ` Linus Torvalds 2019-06-02 20:54 ` Linus Torvalds 2019-06-03 2:46 ` Herbert Xu 2019-06-03 2:46 ` Herbert Xu 2019-06-03 3:47 ` Paul E. McKenney 2019-06-03 4:01 ` Herbert Xu 2019-06-03 4:01 ` Herbert Xu 2019-06-03 4:17 ` Herbert Xu 2019-06-03 4:17 ` Herbert Xu 2019-06-03 7:23 ` Paul E. McKenney 2019-06-03 8:42 ` Paul E. McKenney 2019-06-03 15:26 ` David Laight 2019-06-03 15:40 ` Linus Torvalds 2019-06-03 15:40 ` Linus Torvalds 2019-06-03 5:26 ` Herbert Xu 2019-06-03 5:26 ` Herbert Xu 2019-06-03 6:42 ` Boqun Feng 2019-06-03 6:42 ` Boqun Feng 2019-06-03 20:03 ` Paul E. McKenney 2019-06-04 14:44 ` Alan Stern 2019-06-04 14:44 ` Alan Stern 2019-06-04 16:04 ` Linus Torvalds [this message] 2019-06-04 16:04 ` Linus Torvalds 2019-06-04 17:00 ` Alan Stern 2019-06-04 17:00 ` Alan Stern 2019-06-04 17:29 ` Linus Torvalds 2019-06-04 17:29 ` Linus Torvalds 2019-06-07 14:09 ` inet: frags: Turn fqdir->dead into an int for old Alphas Herbert Xu 2019-06-07 14:09 ` Herbert Xu 2019-06-07 15:26 ` Eric Dumazet 2019-06-07 15:26 ` Eric Dumazet 2019-06-07 15:32 ` Herbert Xu 2019-06-07 15:32 ` Herbert Xu 2019-06-07 16:13 ` Eric Dumazet 2019-06-07 16:13 ` Eric Dumazet 2019-06-07 16:19 ` Linus Torvalds 2019-06-07 16:19 ` Linus Torvalds 2019-06-08 15:27 ` Paul E. McKenney 2019-06-08 17:42 ` Linus Torvalds 2019-06-08 17:42 ` Linus Torvalds 2019-06-08 17:50 ` Linus Torvalds 2019-06-08 17:50 ` Linus Torvalds 2019-06-08 18:50 ` Paul E. McKenney 2019-06-08 18:14 ` Paul E. McKenney 2019-06-06 4:51 ` rcu_read_lock lost its compiler barrier Herbert Xu 2019-06-06 4:51 ` Herbert Xu 2019-06-06 6:05 ` Paul E. McKenney 2019-06-06 6:14 ` Herbert Xu 2019-06-06 6:14 ` Herbert Xu 2019-06-06 9:06 ` Paul E. McKenney 2019-06-06 9:28 ` Herbert Xu 2019-06-06 9:28 ` Herbert Xu 2019-06-06 10:58 ` Paul E. McKenney 2019-06-06 13:38 ` Herbert Xu 2019-06-06 13:38 ` Herbert Xu 2019-06-06 13:48 ` Paul E. McKenney 2019-06-06 8:16 ` Andrea Parri 2019-06-06 14:19 ` Alan Stern 2019-06-06 14:19 ` Alan Stern 2019-06-08 15:19 ` Paul E. McKenney 2019-06-08 15:56 ` Alan Stern 2019-06-08 15:56 ` Alan Stern 2019-06-08 16:31 ` Paul E. McKenney 2019-06-03 9:35 ` Paul E. McKenney 2019-06-06 8:38 ` Andrea Parri 2019-06-06 9:32 ` Herbert Xu 2019-06-06 9:32 ` Herbert Xu 2019-06-03 0:06 ` Paul E. McKenney 2019-06-03 3:03 ` Herbert Xu 2019-06-03 3:03 ` Herbert Xu 2019-06-03 9:27 ` Paul E. McKenney 2019-06-03 15:55 ` Linus Torvalds 2019-06-03 15:55 ` Linus Torvalds 2019-06-03 16:07 ` Linus Torvalds 2019-06-03 16:07 ` Linus Torvalds 2019-06-03 19:53 ` Paul E. McKenney 2019-06-03 20:24 ` Linus Torvalds 2019-06-03 20:24 ` Linus Torvalds 2019-06-04 21:14 ` Paul E. McKenney 2019-06-05 2:21 ` Herbert Xu 2019-06-05 2:21 ` Herbert Xu 2019-06-05 3:30 ` Paul E. McKenney 2019-06-06 4:37 ` Herbert Xu 2019-06-06 4:37 ` Herbert Xu
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-=wgGnCw==uY8radrB+Tg_CEmzOtwzyjfMkuh7JmqFh+jzQ@mail.gmail.com' \ --to=torvalds@linux-foundation.org \ --cc=andrea.parri@amarulasolutions.com \ --cc=boqun.feng@gmail.com \ --cc=davem@davemloft.net \ --cc=fengguang.wu@intel.com \ --cc=fweisbec@gmail.com \ --cc=herbert@gondor.apana.org.au \ --cc=j.alglave@ucl.ac.uk \ --cc=linux-kernel@vger.kernel.org \ --cc=lkp@01.org \ --cc=luc.maranget@inria.fr \ --cc=netdev@vger.kernel.org \ --cc=paulmck@linux.ibm.com \ --cc=stern@rowland.harvard.edu \ /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: linkBe 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.