[PATCH] drivers/perf: arm-pmu: fix RCU usage on resume from idle states

[Paul E. McKenney]

On Wed, Apr 20, 2016 at 05:52:05PM +0100, Lorenzo Pieralisi wrote:
> On Wed, Apr 20, 2016 at 09:23:54AM -0700, Paul E. McKenney wrote:
> 
> [...]
> 
> > > > Maybe RCU_NONIDLE() will help here?
> > > 
> > > Thanks for chiming in.
> > > 
> > > CPU_PM notifiers are called from process context (which is not necessarily
> > > the idle thread) with IRQs disabled from:
> > > 
> > > - CPUidle drivers state enter calls
> > > - syscore callbacks (ie suspend2RAM - suspend thread)
> > > - bL switcher
> > > - MCPM loopback
> > > 
> > > The questions I have are:
> > > 
> > > - Is it safe to wrap a call (in this case armpmu_start()) with RCU_NONIDLE
> > >   if the core is not actually executing the idle thread ? The function
> > >   requiring rcu locks/dereferences is perf_event_update_userpage().
> > 
> > Yes it is.
> > 
> > > - What are RCU_NONIDLE side-effects (ie what can be actually called from
> > >   within an RCU_NONIDLE wrapper ?)
> > 
> > There are a few restrictions:
> > 
> > 1.	Code within RCU_NONIDLE() cannot block.  Then again, neither
> > 	can the idle task.  ;-)
> > 
> > 2.	RCU_NONIDLE() can be nested, but not indefinitely.  Then again,
> > 	given that the limit even on a 32-bit system is something like
> > 	a million, I bet you hit compiler or stack-size limits long
> > 	before you overflow RCU_NONIDLE()'s counter.
> > 
> > 3.	You can neither branch into the middle of RCU_NONIDLE()'s code
> > 	nor branch out from the middle of RCU_NONIDLE()'s code.
> > 	Calling functions is just fine, but things like this are not:
> > 
> > 		RCU_NONIDLE({
> > 			do_something();
> > 			goto bad_idea;	/* BUG!!! */
> > 			do_something_else();});
> > 		do_yet_a_third_thing();
> > 	bad_idea:
> > 
> > 	Branching -within- the RCU_NONIDLE() code is just fine.
> > 
> > Yes, and I am adding this information to RCU_NONIDLE()'s header
> > comment, apologies for its not being there to begin with!
> > (See below for patches.)
> 
> Thank you for the explanation, that's now clear. For my own understanding:
> RCU_NONIDLE() is a way to inform the RCU subsystem that the CPU in question
> should be temporarily *watched* (ie it is not idle from an RCU standpoint),
> correct ?

Exactly!

> > > It would be nice if we can use it instead of merging this patch, I need
> > > more insights into RCU_NONIDLE usage though before proceeding.
> > 
> > Please let me know if any of the above restrictions cause you a problem.
> 
> I can't think of any, perf_event_update_userpage(), that I will call
> indirectly through:
> 
> RCU_NONIDLE(armpmu_start());
> 
> is not allowed to block anyway, so I think we have a much better
> solution than this one, new patch coming, Catalin please drop this one.
> 
> Thank you all !

Please let me know how it goes!

							Thanx, Paul