Re: [linux-next:master] [rcu] c9b55f9da0: WARNING:at_kernel/rcu/rcutorture.c:#rcutorture_one_extend_check[rcutorture]

[Boqun Feng <boqun.feng@gmail.com>]

On Sun, Feb 23, 2025 at 07:21:25PM -0800, Boqun Feng wrote:
> I finally find why I cannot reproduce this, I accidentally used
> next.2025.02.10a to build the kernel first, which has commit
> ("rcutorture: Move RCU_TORTURE_TEST_{CHK_RDR_STATE,LOG_CPU} to bool"),
> which changes Kconfig RCU_TORTURE_TEST_CHK_DRD_STATE into a bool and
> that disabled the test... (because config from you has it as =m).
> 
> On Mon, Feb 24, 2025 at 10:22:02AM +0800, Oliver Sang wrote:
> > hi, Paul,
> > 
> > On Fri, Feb 21, 2025 at 05:02:51PM -0800, Paul E. McKenney wrote:
> > 
> > [...]
> > 
> > > > > 
> > > > > And rcutorture's WARN_ON() has a bug that is exposed by that change
> > > > > in Kconfig option.  Does the patch shown below help?
> > > > 
> > > > the patch does not fix the WARNING in our tests. attached one dmesg FYI.
> > > 
> > > Just to make sure that I understand, this patch was applied against this
> > > commit, correct?
> > > 
> > > c9b55f9da0d2 ("rcu: limit PREEMPT_RCU configurations")
> > > 
> > > I am guessing this based on this dmesg line:
> > > 
> > >  [  109.553307][  T781] CPU: 1 UID: 0 PID: 781 Comm: rcu_torture_rea Tainted: G                T  6.14.0-rc1-00007-gc9b55f9da0d2 #1
> > 
> > above line is not from the dmesg I attached in last mail. it's from
> > https://download.01.org/0day-ci/archive/20250217/202502171415.8ec87c87-lkp@intel.com/dmesg.xz
> > which is for our original report.
> > 
> > > 
> > > Is this really the case, or am I confused?
> > 
> > we applied your patch as:
> > 
> > 89519085afdf2 fix for c9b55f9da0 from Paul
> > c9b55f9da0d2c rcu: limit PREEMPT_RCU configurations
> > f001b7165def8 osnoise: provide quiescent states
> > 
> > so in the dmesg I attached in last mail (I attached it again in this mail):
> > 
> > [    0.000000][    T0] Linux version 6.14.0-rc1-00008-g89519085afdf (kbuild@9871be4fdbcc) (gcc-12 (Debian 12.2.0-14) 12.2.0, GNU ld (GNU Binutils for Debian) 2.40) #1 SMP PREEMPT Fri Feb 21 00:34:02 CST 2025
> > ...
> > [  117.463907][  T812] CPU: 1 UID: 0 PID: 812 Comm: rcu_torture_rea Tainted: G                T  6.14.0-rc1-00008-g89519085afdf #1
> > 
> > the change of this 89519085afdf2 is as [1]
> > 
> > I'm not sure if it's better to upload dmesg for fix patch to
> > https://download.01.org/0day-ci/archive/20250217/202502171415.8ec87c87-lkp@intel.com
> > again, so I did not do that. sorry if this causes confusion.
> > 
> > not sure if this is the correct applyment? thanks
> > 
> > [1]
> > diff --git a/kernel/rcu/rcutorture.c b/kernel/rcu/rcutorture.c
> > index d26fb1d33ed9a..de85a88810cf6 100644
> > --- a/kernel/rcu/rcutorture.c
> > +++ b/kernel/rcu/rcutorture.c
> > @@ -1873,6 +1873,8 @@ static void rcu_torture_reader_do_mbchk(long myid, struct rcu_torture *rtp,
> >  #define ROEC_ARGS "%s %s: Current %#x  To add %#x  To remove %#x  preempt_count() %#x\n", __func__, s, curstate, new, old, preempt_count()
> >  static void rcutorture_one_extend_check(char *s, int curstate, int new, int old, bool insoftirq)
> >  {
> > +       int mask;
> > +
> >         if (!IS_ENABLED(CONFIG_RCU_TORTURE_TEST_CHK_RDR_STATE))
> >                 return;
> > 
> > @@ -1902,8 +1904,10 @@ static void rcutorture_one_extend_check(char *s, int curstate, int new, int old,
> >         WARN_ONCE(cur_ops->extendables &&
> >                   !(curstate & (RCUTORTURE_RDR_PREEMPT | RCUTORTURE_RDR_SCHED)) &&
> >                   (preempt_count() & PREEMPT_MASK), ROEC_ARGS);

And we need another fix for the WARN_ONCE() above because in
non-preemptible RCU on a preemptible kernel, rcu_read_lock() is just
preempt_disable(). Sending both really quick.

Regards,
Boqun

> > -       WARN_ONCE(cur_ops->readlock_nesting &&
> > -                 !(curstate & (RCUTORTURE_RDR_RCU_1 | RCUTORTURE_RDR_RCU_2)) &&
> > +       mask = RCUTORTURE_RDR_RCU_1 | RCUTORTURE_RDR_RCU_2;
> > +       if (IS_ENABLED(CONFIG_PREEMPT_RCU))
> 
> Now look into this, I think this should be:
> 
> 	if (!IS_ENABLED(CONFIG_PREEMPT_RCU))
> 
> because:
> 
> *	For preemptible RCU, ->readlock_nesting() will return 
> 	rcu_preempt_depth()
> 
> *	For non-preemptible RCU, ->readlock_nesting() will return
> 	preempt count.
> 
> , which means if RCUTORTURE_RDR_PREEMPT or RCUTORTURE_RDR_SCHED is in
> the curstate for *non-preemption RCU*, ->readlock_nesting() will be >0.
> That is, the "mask" needs to consider _PREEMPT and _SCHED for
> *non-preemption RCU*, not preemptible RCU.
> 
> Paul? Did I get it right?
> 
> Regards,
> Boqun
> 
> > +               mask |= RCUTORTURE_RDR_PREEMPT | RCUTORTURE_RDR_SCHED;
> > +       WARN_ONCE(cur_ops->readlock_nesting && !(curstate & mask) &&
> >                   cur_ops->readlock_nesting() > 0, ROEC_ARGS);
> >  }
> > 
[...]