On Mon, 13 Jun 2016, Richard Cochran wrote:
> On Mon, Jun 13, 2016 at 08:41:04AM -0000, Thomas Gleixner wrote:
> > @@ -498,23 +500,27 @@ static void internal_add_timer(struct ti
> > __internal_add_timer(base, timer);
> >
> > /*
> > - * Check whether the other CPU is in dynticks mode and needs
> > - * to be triggered to reevaluate the timer wheel. We are
> > - * protected against the other CPU fiddling with the timer by
> > - * holding the timer base lock. This also makes sure that a
> > - * CPU on the way to stop its tick can not evaluate the timer
> > - * wheel.
> > - *
> > - * Spare the IPI for deferrable timers on idle targets though.
> > - * The next busy ticks will take care of it. Except full dynticks
> > - * require special care against races with idle_cpu(), lets deal
> > - * with that later.
> > - */
> > - if (IS_ENABLED(CONFIG_NO_HZ_COMMON) && base->nohz_active) {
> > - if (!(timer->flags & TIMER_DEFERRABLE) ||
> > - tick_nohz_full_cpu(base->cpu))
> > - wake_up_nohz_cpu(base->cpu);
> > - }
> > + * We might have to IPI the remote CPU if the base is idle and the
> > + * timer is not deferrable. If the other cpu is on the way to idle
> > + * then it can't set base->is_idle as we hold base lock.
> > + */
> > + if (!IS_ENABLED(CONFIG_NO_HZ_COMMON) || !base->is_idle ||
> > + (timer->flags & TIMER_DEFERRABLE))
> > + return;
>
> The tests for is_idle and TIMER_DEFERRABLE are actually checking the
> same thing. I was looking at the usage of base.is_idle, and it is
> only ever set for BASE_STD.
>
> So the TIMER_DEFERRABLE test is redundant, but maybe this is on
> purpose? Anyhow, it did leave me scratching my head.
You're right. is_idle makes the deferrable check obsolete. We can remove it,
but it needs a big fat comment ....
Thanks
tglx
