On Mon, Feb 16, 2015 at 08:49:12AM +0530, Preeti U Murthy wrote:
> On 02/13/2015 07:56 PM, Paul E. McKenney wrote:
> > On Fri, Feb 13, 2015 at 12:52:45PM +0530, Preeti U Murthy wrote:
> >> On 02/13/2015 10:57 AM, Preeti U Murthy wrote:
> >>> On 02/13/2015 06:27 AM, Sam Bobroff wrote:
> >>>> Hello,
> >>>>
> >>>> I'm receiving this while booting a vanilla 3.19 kernel on a Power 8
> >>>> machine:
> >>>
> >>> Does the below patch fix the issue ?
> >>>
> >>> From: Preeti U Murthy <[email protected]>
> >>>
> >>> [PATCH] tick/hrtimer-broadcast: Fix a suspicious RCU usage in the tick
> >>> broadcast path
> >>>
> >>> ---
> >>> kernel/time/tick-broadcast-hrtimer.c | 2 +-
> >>> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>>
> >>> diff --git a/kernel/time/tick-broadcast-hrtimer.c
> >>> b/kernel/time/tick-broadcast-hrtimer.c
> >>> index eb682d5..57b8e32 100644
> >>> --- a/kernel/time/tick-broadcast-hrtimer.c
> >>> +++ b/kernel/time/tick-broadcast-hrtimer.c
> >>> @@ -62,7 +62,7 @@ static int bc_set_next(ktime_t expires, struct
> >>> clock_event_device *bc)
> >>> * HRTIMER_RESTART.
> >>> */
> >>> if (hrtimer_try_to_cancel(&bctimer) >= 0) {
> >>> - hrtimer_start(&bctimer, expires, HRTIMER_MODE_ABS_PINNED);
> >>> + RCU_NONIDLE(hrtimer_start(&bctimer, expires,
> >>> HRTIMER_MODE_ABS_PINNED));
> >>> /* Bind the "device" to the cpu */
> >>> bc->bound_on = smp_processor_id();
> >>> } else if (bc->bound_on == smp_processor_id()) {
> >>>
> >> Actually the below patch is the complete fix. Paul can you please
> >> review this ? As an alternate solution I checked to see if its
> >> possible to move rcu_idle_enter()/exit() closer to the
> >> cpuidle_enter() call, but that won't work as you may have already
> >> tried earlier.
> >>
> >> -----------------------------------------------------------------
> >>
> >> tick/broadcast-hrtimer : Fix suspicious RCU usage in idle loop
> >>
> >> From: Preeti U Murthy <[email protected]>
> >>
> >> The hrtimer mode of broadcast queues hrtimers in the idle entry
> >> path so as to wakeup cpus in deep idle states. hrtimer_{start/cancel}
> >> functions call into tracing which uses RCU. But it is not legal to call
> >> into RCU in cpuidle because it is one of the quiescent states. Hence
> >> protect this region with RCU_NONIDLE which informs RCU that the cpu
> >> is momentarily non-idle.
> >>
> >> Signed-off-by: Preeti U Murthy <[email protected]>
> >
> > Reviewed-by: Paul E. McKenney <[email protected]>
> >
> > Another alternative would be to change the hrtimer_{start/cancel}()
> > functions' tracepoints to the _rcuidle form. The advantage of this
> > approach is less RCU-notification overhead when tracing is enabled.
>
> But since the hrtimer_{start/cancel} functions' tracepoints are more
> often called from paths which are in the non-quiescent states, wouldn't
> we be doing an rcu_irq_enter/exit() redundantly far too often in that case ?
And the other advantage of doing it the way you did (and I -did- give
you a Reviewed-by!) is that you are incurring the extra overhead from
the idle loop, where that extra overhead is less likely to be holding
something else up. So, yes, I do agree with your patch.
Thanx, Paul
_______________________________________________
Linuxppc-dev mailing list
[email protected]
https://lists.ozlabs.org/listinfo/linuxppc-dev
