linux-kernel - Re: Folllowing up on LSF/MM RCU/idle discussion

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

Message-ID: <20220510155553.GU1790663@paulmck-ThinkPad-P17-Gen-1>
Date:   Tue, 10 May 2022 08:55:53 -0700
From:   "Paul E. McKenney" <paulmck@...nel.org>
To:     Mark Rutland <mark.rutland@....com>
Cc:     Peter Zijlstra <peterz@...radead.org>, jolsa@...hat.com,
        linux-kernel@...r.kernel.org
Subject: Re: Folllowing up on LSF/MM RCU/idle discussion

On Tue, May 10, 2022 at 10:43:51AM +0100, Mark Rutland wrote:
> On Tue, May 10, 2022 at 08:54:57AM +0200, Peter Zijlstra wrote:
> > On Mon, May 09, 2022 at 08:56:33AM -0700, Paul E. McKenney wrote:
> > > Hello, Jiri!
> > > 
> > > It was good chatting with you last week, and I hope that travels went
> > > well!
> > > 
> > > Just wanted to follow up on the non-noinstr code between the call
> > > to rcu_idle_enter() and rcu_idle_exit().  Although the most correct
> > > approach is to never have non-noinstr code in arch_cpu_idle(), for all I
> > > know there might well be architectures for which this is not feasible.
> > > If so, one workaround would be to supply a flag set by each arch (or
> > > subarch) that says that rcu_idle_enter() and rcu_idle_exit() are invoked
> > > within arch_cpu_idle().
> > > 
> > > CCing Peter, who just might have an opinion on this.  ;-)
> > 
> > Definitely have an opinion; just lack the tools to enforce these rules.
> > I cleaned up the worst of it for x86 but it's a shit-show for most
> > others. ARM in particular has some 'issues'.
> 
> Probably worth pointing out that arch_cpu_idle() is the simple case (and I
> fixed that for arm64 to be correct for RCU and noinstr). I think the same
> applies for most architectures.
> 
> The real beast is the cpuidle framework, which is what I think you're referring
> to below, and IIRC that does the rcu_idle_enter() ... rcu_idle_exit() itself?
> Maybe that was just for suspend.
> 
> I have no strong feeling about where we call rcu_idle_{enter,exit}() for
> arch_cpu_idle() specifically, but I think that case is generally simple enough
> that it doesn't really matter?
> 
> For the cpuidle framework, punting this into the driver such that it can be
> done at the last possible moment around entry/exit to a HW idle state does feel
> like it's going to be more robust, even if that means altering all those
> drivers.

There does seem to be some ability to select where the rcu_idle_enter()
and rcu_idle_exit() are executed using CPUIDLE_FLAG_RCU_IDLE.
So depending on exactly which configuration Jiri is running, maybe there
is already a straightforward fix.

However, default_idle_call() invokes rcu_idle_enter() and rcu_idle_exit()
unconditionally, perhaps because it is not considered to be part of
cpuidle.

							Thanx, Paul

> Thanks,
> Mark.
> 
> > But yeah, noinstr only when you do rcu_idle_enter.
> > 
> > The problem with validating all this is that cpuidle is a rats nest of
> > indirect calls; in order to validate the noinstr'ness of something like
> > that we need compiler support for pointer address spaces such that we
> > can stick pointers to noinstr functions in a different address space and
> > get complaints etc..