Re: next: suspicious RCU usage message since commit 'rcu: Remove superfluous versions of rcu_read_lock_sched_held()'

[Guenter Roeck]

On 04/24/2016 10:28 PM, Paul E. McKenney wrote:
> On Sun, Apr 24, 2016 at 04:56:38PM -0700, Guenter Roeck wrote:
> > Hi Paul,
> >
> > On 04/24/2016 02:31 PM, Paul E. McKenney wrote:
> > > On Sun, Apr 24, 2016 at 02:14:24PM -0700, Guenter Roeck wrote:
> > > > Hi,
> > > >
> > > > I see the following log message when running a qemu test for 'beagle'
> > > > with omap2plus_defconfig.
> > > >
> > > > ===============================
> > > > [ INFO: suspicious RCU usage. ]
> > > > 4.6.0-rc4-next-20160422 #1 Not tainted
> > > > -------------------------------
> > > > include/trace/events/power.h:328 suspicious rcu_dereference_check() usage!
> > > >
> > > > other info that might help us debug this:
> > > >
> > > > RCU used illegally from idle CPU!
> > > > rcu_scheduler_active = 1, debug_locks = 0
> > > > RCU used illegally from extended quiescent state!
> > > > no locks held by swapper/0/0.
> > > >
> > > > stack backtrace:
> > > > CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.6.0-rc4-next-20160422 #1
> > > > Hardware name: Generic OMAP3-GP (Flattened Device Tree)
> > > > [<c010f55c>] (unwind_backtrace) from [<c010b64c>] (show_stack+0x10/0x14)
> > > > [<c010b64c>] (show_stack) from [<c047acbc>] (dump_stack+0xa8/0xe0)
> > > > [<c047acbc>] (dump_stack) from [<c012bc10>] (pwrdm_set_next_pwrst+0xf8/0x1cc)
> > > > [<c012bc10>] (pwrdm_set_next_pwrst) from [<c01269fc>] (omap3_enter_idle_bm+0x1b8/0x1e8)
> > > > [<c01269fc>] (omap3_enter_idle_bm) from [<c05fa0b8>] (cpuidle_enter_state+0x84/0x408)
> > > > [<c05fa0b8>] (cpuidle_enter_state) from [<c0182c1c>] (cpu_startup_entry+0x1c8/0x3f0)
> > > > [<c0182c1c>] (cpu_startup_entry) from [<c0b00c20>] (start_kernel+0x354/0x3cc)
> > > >
> > > > bisect points to commit 'rcu: Remove superfluous versions of
> > > > rcu_read_lock_sched_held()'. Bisect log is attached.
> > >
> > > I believe that the real fix is not a revert of that commit, but rather
> > > that some of the tracing statements need an "_rcuidle" suffix.
> > >
> > > Something like the following (untested, probably does not build) patch.
> > >
> > > 							Thanx, Paul
> > >
> > > ------------------------------------------------------------------------
> > >
> > > commit ca91304178e1cf53ee391236a0ac3969cc814e5f
> > > Author: Paul E. McKenney <paulmck@xxxxxxxxxxxxxxxxxx>
> > > Date:   Sun Apr 24 14:30:16 2016 -0700
> > >
> > >      arm: Use _rcuidle tracepoint to allow use from idle
> > >
> > >      Signed-off-by: Paul E. McKenney <paulmck@xxxxxxxxxxxxxxxxxx>
> > >
> > > diff --git a/arch/arm/mach-omap2/powerdomain.c b/arch/arm/mach-omap2/powerdomain.c
> > > index 78af6d8cf2e2..12b66b5bcc55 100644
> > > --- a/arch/arm/mach-omap2/powerdomain.c
> > > +++ b/arch/arm/mach-omap2/powerdomain.c
> > > @@ -523,8 +523,8 @@ int pwrdm_set_next_pwrst(struct powerdomain *pwrdm, u8 pwrst)
> > >
> > >   	if (arch_pwrdm && arch_pwrdm->pwrdm_set_next_pwrst) {
> > >   		/* Trace the pwrdm desired target state */
> > > -		trace_power_domain_target(pwrdm->name, pwrst,
> > > -					  smp_processor_id());
> > > +		trace_power_domain_target_rcuidle(pwrdm->name, pwrst,
> > > +						  smp_processor_id());
> > >   		/* Program the pwrdm desired target state */
> > >   		ret = arch_pwrdm->pwrdm_set_next_pwrst(pwrdm, pwrst);
> > >   	}
> >
> > It does build. After applying it, I get a different traceback.
> >
> > [<c010f55c>] (unwind_backtrace) from [<c010b64c>] (show_stack+0x10/0x14)
> > [<c010b64c>] (show_stack) from [<c047ac3c>] (dump_stack+0xa8/0xe0)
> > [<c047ac3c>] (dump_stack) from [<c012c340>] (_pwrdm_state_switch+0x188/0x32c)
> > [<c012c340>] (_pwrdm_state_switch) from [<c012c4f0>] (_pwrdm_post_transition_cb+0xc/0x14)
> > [<c012c4f0>] (_pwrdm_post_transition_cb) from [<c012ba74>] (pwrdm_for_each+0x30/0x5c)
> > [<c012ba74>] (pwrdm_for_each) from [<c012c72c>] (pwrdm_post_transition+0x24/0x30)
> > [<c012c72c>] (pwrdm_post_transition) from [<c012548c>] (omap_sram_idle+0xfc/0x240)
> > [<c012548c>] (omap_sram_idle) from [<c0126934>] (omap3_enter_idle_bm+0xf0/0x1e8)
> > [<c0126934>] (omap3_enter_idle_bm) from [<c05fa038>] (cpuidle_enter_state+0x84/0x408)
> > [<c05fa038>] (cpuidle_enter_state) from [<c0182b90>] (cpu_startup_entry+0x1c8/0x3f0)
> > [<c0182b90>] (cpu_startup_entry) from [<c0b00c20>] (start_kernel+0x354/0x3cc)
> >
> > After making the same change in _pwrdm_state_switch(), the traceback is gone
> > from my tests (beagle, beagle-xm, and overo-tobi).
>
> Very good!
>
> (And yes, you normally find these one at a time...)
Are you going to submit a formal patch ?

Thanks,
Guenter