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 <pre...@linux.vnet.ibm.com>
> >
> > [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 <pre...@linux.vnet.ibm.com>
>
> 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 <pre...@linux.vnet.ibm.com>
Reviewed-by: Paul E. McKenney <paul...@linux.vnet.ibm.com>
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.
Thanx, Paul
> ---
> kernel/time/tick-broadcast-hrtimer.c | 11 +++++++++--
> 1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/time/tick-broadcast-hrtimer.c
> b/kernel/time/tick-broadcast-hrtimer.c
> index eb682d5..d3dd564 100644
> --- a/kernel/time/tick-broadcast-hrtimer.c
> +++ b/kernel/time/tick-broadcast-hrtimer.c
> @@ -49,6 +49,7 @@ static void bc_set_mode(enum clock_event_mode mode,
> */
> static int bc_set_next(ktime_t expires, struct clock_event_device *bc)
> {
> + int bc_moved;
> /*
> * We try to cancel the timer first. If the callback is on
> * flight on some other cpu then we let it handle it. If we
> @@ -60,9 +61,15 @@ static int bc_set_next(ktime_t expires, struct
> clock_event_device *bc)
> * restart the timer because we are in the callback, but we
> * can set the expiry time and let the callback return
> * HRTIMER_RESTART.
> + *
> + * Since we are in the idle loop at this point and because
> + * hrtimer_{start/cancel} functions call into tracing,
> + * calls to these functions must be bound within RCU_NONIDLE.
> */
> - if (hrtimer_try_to_cancel(&bctimer) >= 0) {
> - hrtimer_start(&bctimer, expires, HRTIMER_MODE_ABS_PINNED);
> + RCU_NONIDLE(bc_moved = (hrtimer_try_to_cancel(&bctimer) >= 0) ?
> + !hrtimer_start(&bctimer, expires, HRTIMER_MODE_ABS_PINNED) :
> + 0);
> + if (bc_moved) {
> /* Bind the "device" to the cpu */
> bc->bound_on = smp_processor_id();
> } else if (bc->bound_on == smp_processor_id()) {
>
>
> Thanks
>
> Regards
> Preeti U Murthy
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
