Re: [patch 18/20] timer: Forward wheel clock whenever possible

From: Richard Cochran
Date: Mon Jun 13 2016 - 11:17:56 EST


Thomas,

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.

Thanks,
Richard