linux-kernel - Re: [PATCH 5.10 020/104] ARM: OMAP2+: Fix suspcious RCU usage splats for omap_enter_idle

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] [day] [month] [year] [list]

Message-ID: <20210219212622.GD2743@paulmck-ThinkPad-P72>
Date:   Fri, 19 Feb 2021 13:26:22 -0800
From:   "Paul E. McKenney" <paulmck@...nel.org>
To:     Pavel Machek <pavel@....cz>
Cc:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        linux-kernel@...r.kernel.org, stable@...r.kernel.org,
        Daniel Lezcano <daniel.lezcano@...aro.org>,
        Russell King <rmk+kernel@...linux.org.uk>,
        Tony Lindgren <tony@...mide.com>,
        Sasha Levin <sashal@...nel.org>
Subject: Re: [PATCH 5.10 020/104] ARM: OMAP2+: Fix suspcious RCU usage splats
 for omap_enter_idle_coupled

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