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

[lorenzo.pieralisi@arm.com (Lorenzo Pieralisi)]

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 ?

> > 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 !
Lorenzo