Re: [PATCH 4/9] timers: Always keep track of next expiry

Anna-Maria Behnsen
2020-07-14T08:49:37Z

Hi Frederic,

On Tue, 7 Jul 2020, Frederic Weisbecker wrote:

> So far next expiry was only tracked while the CPU was in nohz_idle mode
> in order to cope with missing ticks that can't increment the base->clk
> periodically anymore.
>
> We are going to expand that logic beyond nohz in order to spare timers
> softirqs so do it unconditionally.
>
> Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Anna-Maria Gleixner <anna-maria@linutronix.de>
> Cc: Juri Lelli <juri.lelli@redhat.com>
> ---
> kernel/time/timer.c | 36 +++++++++++++++++++-----------------
> 1 file changed, 19 insertions(+), 17 deletions(-)
>
> diff --git a/kernel/time/timer.c b/kernel/time/timer.c
> index acf7cb8c09f8..8a4138e47aa4 100644
> --- a/kernel/time/timer.c
> +++ b/kernel/time/timer.c
> @@ -558,8 +558,22 @@ trigger_dyntick_cpu(struct timer_base *base, struct timer_list *timer)
> * 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 the base lock:
> */
> - if (!base->is_idle)
> - return;
> + if (base->is_idle)
> + wake_up_nohz_cpu(base->cpu);
> +}
> +
> +/*
> + * Enqueue the timer into the hash bucket, mark it pending in
> + * the bitmap and store the index in the timer flags.
> + */
> +static void enqueue_timer(struct timer_base *base, struct timer_list *timer,
> + unsigned int idx)
> +{
> + hlist_add_head(&timer->entry, base->vectors + idx);
> + __set_bit(idx, base->pending_map);
> + timer_set_idx(timer, idx);
> +
> + trace_timer_start(timer, timer->expires, timer->flags);
>
> /* Check whether this is the new first expiring timer: */
> if (time_after_eq(timer->expires, base->next_expiry))
> @@ -578,21 +592,7 @@ trigger_dyntick_cpu(struct timer_base *base, struct timer_list *timer)
> } else {
> base->next_expiry = timer->expires;
> }
> - wake_up_nohz_cpu(base->cpu);
> -}
>
> -/*
> - * Enqueue the timer into the hash bucket, mark it pending in
> - * the bitmap and store the index in the timer flags.
> - */
> -static void enqueue_timer(struct timer_base *base, struct timer_list *timer,
> - unsigned int idx)
> -{
> - hlist_add_head(&timer->entry, base->vectors + idx);
> - __set_bit(idx, base->pending_map);
> - timer_set_idx(timer, idx);
> -
> - trace_timer_start(timer, timer->expires, timer->flags);
> trigger_dyntick_cpu(base, timer);
> }
>

Could you please split those two hunks which do only a restructuring into a
separate patch?

Thanks,

Anna-Maria