From: Herbert Xu <herbert@gondor.apana.org.au> To: Linus Torvalds <torvalds@linux-foundation.org> Cc: Alan Stern <stern@rowland.harvard.edu>, "Paul E. McKenney" <paulmck@linux.ibm.com>, Boqun Feng <boqun.feng@gmail.com>, 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: inet: frags: Turn fqdir->dead into an int for old Alphas Date: Fri, 7 Jun 2019 22:09:49 +0800 [thread overview] Message-ID: <20190607140949.tzwyprrhmqdx33iu@gondor.apana.org.au> (raw) In-Reply-To: <CAHk-=wgGnCw==uY8radrB+Tg_CEmzOtwzyjfMkuh7JmqFh+jzQ@mail.gmail.com> On Tue, Jun 04, 2019 at 09:04:55AM -0700, Linus Torvalds wrote: > > 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.). This is in fact a real bug in the code in question that no amount of READ_ONCE/WRITE_ONCE would have caught. The field fqdir->dead is declared as boolean so writing to it is not atomic (on old Alphas). I don't think it currently matters because padding would ensure that it is in fact 64 bits long. However, should someone add another char/bool/bitfield in this struct in future it could become an issue. So let's fix it. ---8<-- The field fqdir->dead is meant to be written (and read) atomically. As old Alpha CPUs can't write a single byte atomically, we need at least an int for it to work. Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au> diff --git a/include/net/inet_frag.h b/include/net/inet_frag.h index e91b79ad4e4a..8c458fba74ad 100644 --- a/include/net/inet_frag.h +++ b/include/net/inet_frag.h @@ -14,7 +14,9 @@ struct fqdir { int max_dist; struct inet_frags *f; struct net *net; - bool dead; + + /* We can't use boolean because this needs atomic writes. */ + int dead; struct rhashtable rhashtable ____cacheline_aligned_in_smp; diff --git a/net/ipv4/inet_fragment.c b/net/ipv4/inet_fragment.c index 35e9784fab4e..05aa7c145817 100644 --- a/net/ipv4/inet_fragment.c +++ b/net/ipv4/inet_fragment.c @@ -193,7 +193,7 @@ void fqdir_exit(struct fqdir *fqdir) { fqdir->high_thresh = 0; /* prevent creation of new frags */ - fqdir->dead = true; + fqdir->dead = 1; /* call_rcu is supposed to provide memory barrier semantics, * separating the setting of fqdir->dead with the destruction -- Email: Herbert Xu <herbert@gondor.apana.org.au> Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
WARNING: multiple messages have this Message-ID (diff)
From: Herbert Xu <herbert@gondor.apana.org.au> To: lkp@lists.01.org Subject: inet: frags: Turn fqdir->dead into an int for old Alphas Date: Fri, 07 Jun 2019 22:09:49 +0800 [thread overview] Message-ID: <20190607140949.tzwyprrhmqdx33iu@gondor.apana.org.au> (raw) In-Reply-To: <CAHk-=wgGnCw==uY8radrB+Tg_CEmzOtwzyjfMkuh7JmqFh+jzQ@mail.gmail.com> [-- Attachment #1: Type: text/plain, Size: 2141 bytes --] On Tue, Jun 04, 2019 at 09:04:55AM -0700, Linus Torvalds wrote: > > 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.). This is in fact a real bug in the code in question that no amount of READ_ONCE/WRITE_ONCE would have caught. The field fqdir->dead is declared as boolean so writing to it is not atomic (on old Alphas). I don't think it currently matters because padding would ensure that it is in fact 64 bits long. However, should someone add another char/bool/bitfield in this struct in future it could become an issue. So let's fix it. ---8<-- The field fqdir->dead is meant to be written (and read) atomically. As old Alpha CPUs can't write a single byte atomically, we need at least an int for it to work. Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au> diff --git a/include/net/inet_frag.h b/include/net/inet_frag.h index e91b79ad4e4a..8c458fba74ad 100644 --- a/include/net/inet_frag.h +++ b/include/net/inet_frag.h @@ -14,7 +14,9 @@ struct fqdir { int max_dist; struct inet_frags *f; struct net *net; - bool dead; + + /* We can't use boolean because this needs atomic writes. */ + int dead; struct rhashtable rhashtable ____cacheline_aligned_in_smp; diff --git a/net/ipv4/inet_fragment.c b/net/ipv4/inet_fragment.c index 35e9784fab4e..05aa7c145817 100644 --- a/net/ipv4/inet_fragment.c +++ b/net/ipv4/inet_fragment.c @@ -193,7 +193,7 @@ void fqdir_exit(struct fqdir *fqdir) { fqdir->high_thresh = 0; /* prevent creation of new frags */ - fqdir->dead = true; + fqdir->dead = 1; /* call_rcu is supposed to provide memory barrier semantics, * separating the setting of fqdir->dead with the destruction -- Email: Herbert Xu <herbert@gondor.apana.org.au> Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
next prev parent reply other threads:[~2019-06-07 14:10 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 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 ` Herbert Xu [this message] 2019-06-07 14:09 ` inet: frags: Turn fqdir->dead into an int for old Alphas 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=20190607140949.tzwyprrhmqdx33iu@gondor.apana.org.au \ --to=herbert@gondor.apana.org.au \ --cc=andrea.parri@amarulasolutions.com \ --cc=boqun.feng@gmail.com \ --cc=davem@davemloft.net \ --cc=fengguang.wu@intel.com \ --cc=fweisbec@gmail.com \ --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 \ --cc=torvalds@linux-foundation.org \ /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.