linux-kernel - Re: [PATCH 5/6] rcu: Remove full memory barrier on RCU stall printout

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Tue, 4 Jun 2024 13:13:25 +0200
From: Frederic Weisbecker <frederic@...nel.org>
To: "Paul E. McKenney" <paulmck@...nel.org>
Cc: LKML <linux-kernel@...r.kernel.org>,
	Valentin Schneider <vschneid@...hat.com>,
	Boqun Feng <boqun.feng@...il.com>,
	Joel Fernandes <joel@...lfernandes.org>,
	Neeraj Upadhyay <neeraj.upadhyay@....com>,
	Uladzislau Rezki <urezki@...il.com>,
	Zqiang <qiang.zhang1211@...il.com>, rcu <rcu@...r.kernel.org>
Subject: Re: [PATCH 5/6] rcu: Remove full memory barrier on RCU stall printout

Le Mon, Jun 03, 2024 at 05:10:54PM -0700, Paul E. McKenney a écrit :
> On Wed, May 15, 2024 at 02:53:31PM +0200, Frederic Weisbecker wrote:
> > RCU stall printout fetches the EQS state of a CPU with a preceding full
> > memory barrier. However there is nothing to order this read against at
> > this debugging stage. It is inherently racy when performed remotely.
> > 
> > Do a plain read instead.
> > 
> > This was the last user of rcu_dynticks_snap().
> > 
> > Signed-off-by: Frederic Weisbecker <frederic@...nel.org>
> 
> I went through all of these, and the look good.  Though I am a bit
> nervous about this one.  The RCU CPU stall warning code used to be
> completely unordered, but the hardware taught me better.  I did not
> add these in response to a problem (just lazily used the existing fully
> ordered primitive), but you never know.

At least I haven't found against what it is ordering the dynticks counter here.

> Me, I would have kept the extra
> memory barriers in all six patches because they are not on a fastpath,

It is still time to discard the patches :-)

> but you are quite correct that they are redundant.

Yes and it's not so much for optimization purpose, like you said it's
not a fast-path, although in the case of fqs round scan it _might_ be
debatable in the presence of hurry callbacks, but I use those changes
more for documentation purpose. My opinion on that being that having
memory barriers when they are not necessary doesn't help reviewers and
doesn't bring the incentive to actually verify that the ordering is
correct when it is really required, since there is so much of it
everywhere anyway. I'd rather have a clear, well visible and precise
picture. But that's just personal belief.

> 
> So I have queued these, and intend to send them into the next merge
> window.  However, you now own vanilla RCU grace-period memory ordering,
> both normal and expedited.  As in if someone else breaks it, you already
> bought it.  ;-)

Sure, but it's a bet. That one day a younger person will buy it from me
double the price ;-)

Thanks.