Re: [PATCH] rcu: Reduce stack usage in show_rcu_gp_kthreads()

On Fri, Jul 24, 2026 at 02:20:48PM +0100, David Laight wrote:
> On Thu, 23 Jul 2026 18:04:30 +0800
> Zqiang <[email protected]> wrote:
> 
> > When CONFIG_KASAN=y and CONFIG_KASAN_STACK=y builds, the
> > show_rcu_gp_kthreads() exceeds the 1024-byte frame-size limit:
> > 
> ...
> >  	rcu_for_each_node_breadth_first(rnp) {
> >  		if (ULONG_CMP_GE(READ_ONCE(rcu_state.gp_seq), READ_ONCE(rnp->gp_seq_needed)) &&
> >  		    !data_race(READ_ONCE(rnp->qsmask)) && !data_race(READ_ONCE(rnp->boost_tasks)) &&
> >  		    !data_race(READ_ONCE(rnp->exp_tasks)) && !data_race(READ_ONCE(rnp->gp_tasks)))
> >  			continue;
> > -		pr_info("\trcu_node %d:%d ->gp_seq %ld ->gp_seq_needed %ld ->qsmask %#lx %c%c%c%c ->n_boosts %ld\n",
> > -			rnp->grplo, rnp->grphi,
> > -			(long)data_race(READ_ONCE(rnp->gp_seq)),
> > -			(long)data_race(READ_ONCE(rnp->gp_seq_needed)),
> > -			data_race(READ_ONCE(rnp->qsmask)),
> > -			".b"[!!data_race(READ_ONCE(rnp->boost_kthread_task))],
> > -			".B"[!!data_race(READ_ONCE(rnp->boost_tasks))],
> > -			".E"[!!data_race(READ_ONCE(rnp->exp_tasks))],
> > -			".G"[!!data_race(READ_ONCE(rnp->gp_tasks))],
> > -			data_race(READ_ONCE(rnp->n_boosts)));
> 
> Isn't that code carefully reading all the values twice?

You are quite right, it does, and it did before Zqiang's patch.  I would
welcome a patch that carefully used locals to avoid the double reads,
as that could avoid some debug confusion that might otherwise result
due to low-probability races with updates to these same variables.
However, this function is invoked quite rarely in error conditions,
so I would also avoid holding anything up waiting for such a patch.

> Also the "ab"[!!val] generates far worse code than the more obvious (val ? 'a' : 'b').
> For the former gcc indexes a constant string, the latter is done using arithmetic.

This code is invoked quite rarely, so I am not concerned about the
overhead and I like the compactness.  In the longer term, perhaps
compilers will recognize the pattern and optimize it.  Or not, who
knows?  ;-)

> I'm sure there is a good reason for data_race(READ_ONCE(xxx)) ...

And there is!!!

The data_race() says "ignore the contents from a data-race viewpoint"
and the READ_ONCE() says "carefully avoid destructive optimizations that
might otherwise be applied to this load."

For an example, suppose that we had a global variable x that was accessed
only under a lock.  Except that we have lockless access for debugging
output like that in Zqiang's patch.  We want the normal lock-protected
accesses to be plain C-language accesses so that any accidental (and thus
likely buggy) lockless accesses are spotted by KCSAN, but we very much
to *not* want to acquire the lock when doing the debug output, because
if the lock was held, we might get a debug-output-free hang just when
we are in the greatest need of that debugging output.

So we use normal C-language accesses for the non-debug uses, and we use
data_race(READ_ONCE(x)) for the debugging output.

Does that make sense?

							Thanx, Paul