Re: [PATCH 5.10 020/104] ARM: OMAP2+: Fix suspcious RCU usage splats for omap_enter_idle_coupled

[Paul E. McKenney]

On Fri, Feb 19, 2021 at 10:14:27PM +0100, Pavel Machek wrote:
> Hi!
> 
> > [ Upstream commit 06862d789ddde8a99c1e579e934ca17c15a84755 ]
> > 
> > We get suspcious RCU usage splats with cpuidle in several places in
> > omap_enter_idle_coupled() with the kernel debug options enabled:
> > 
> > RCU used illegally from extended quiescent state!
> > ...
> > (_raw_spin_lock_irqsave)
> > (omap_enter_idle_coupled+0x17c/0x2d8)
> > (omap_enter_idle_coupled)
> > (cpuidle_enter_state)
> > (cpuidle_enter_state_coupled)
> > (cpuidle_enter)
> > 
> > Let's use RCU_NONIDLE to suppress these splats. Things got changed around
> > with commit 1098582a0f6c ("sched,idle,rcu: Push rcu_idle deeper into the
> > idle path") that started triggering these warnings.
> 
> I just wanted to check... AFAICT RCU_NONIDLE puts some quite heavy
> instrumentation around each statement; does it makes sense to group
> the statements into one in cases like this?

The RCU_NONIDLE() does a pair of value-returning atomic adds, but
to per-CPU variables.  My guess is that that overhead is not large
compared to the functions being called.

Nevertheless, you are right that it would be more efficient to do
something like this:

		RCU_NONIDLE(clkdm_deny_idle(cpu_clkdm[1]);
			    omap_set_pwrdm_state(cpu_pd[1], PWRDM_POWER_ON);
			    clkdm_allow_idle(cpu_clkdm[1]));

And it is the same number of lines, so why not?  ;-)

							Thanx, Paul

> Best regards,
> 								Pavel
> 
> > @@ -194,9 +194,9 @@ static int omap_enter_idle_coupled(struct cpuidle_device *dev,
> >  		    mpuss_can_lose_context)
> >  			gic_dist_disable();
> >  
> > -		clkdm_deny_idle(cpu_clkdm[1]);
> > -		omap_set_pwrdm_state(cpu_pd[1], PWRDM_POWER_ON);
> > -		clkdm_allow_idle(cpu_clkdm[1]);
> > +		RCU_NONIDLE(clkdm_deny_idle(cpu_clkdm[1]));
> > +		RCU_NONIDLE(omap_set_pwrdm_state(cpu_pd[1], PWRDM_POWER_ON));
> > +		RCU_NONIDLE(clkdm_allow_idle(cpu_clkdm[1]));
> >  
> >  		if (IS_PM44XX_ERRATUM(PM_OMAP4_ROM_SMP_BOOT_ERRATUM_GICD) &&
> >  		    mpuss_can_lose_context) {
> 
> -- 
> http://www.livejournal.com/~pavelmachek