Re: [PATCH v3 03/17] timer: Move store of next event into __next_timer_interrupt()

Frederic Weisbecker
2022-10-26T09:33:13Z

On Tue, Oct 25, 2022 at 03:58:36PM +0200, Anna-Maria Behnsen wrote:
> Both call sides of __next_timer_interrupt() store the return value directly
> in base->next_expiry. Move the store into __next_timer_interrupt().
>
> No functional change.
>
> Signed-off-by: Anna-Maria Behnsen <anna-maria@linutronix.de>
> Reviewed-by: Thomas Gleixner <tglx@linutronix.de>
> ---
> kernel/time/timer.c | 11 +++++++----
> 1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/kernel/time/timer.c b/kernel/time/timer.c
> index 717fcb9fb14a..7695c733dfa5 100644
> --- a/kernel/time/timer.c
> +++ b/kernel/time/timer.c
> @@ -1571,8 +1571,10 @@ static int next_pending_bucket(struct timer_base *base, unsigned offset,
> /*
> * Search the first expiring timer in the various clock levels. Caller must
> * hold base->lock.
> + *
> + * Store next expiry time in base->next_expiry.
> */
> -static unsigned long __next_timer_interrupt(struct timer_base *base)
> +static void __next_timer_interrupt(struct timer_base *base)
> {
> unsigned long clk, next, adj;
> unsigned lvl, offset = 0;
> @@ -1638,10 +1640,11 @@ static unsigned long __next_timer_interrupt(struct timer_base *base)
> clk += adj;
> }
>
> + base->next_expiry = next;
> base->next_expiry_recalc = false;

In that case, maybe rename that function as next_expiry_recalc() to make its
purpose clearer?

Thanks!

> base->timers_pending = !(next == base->clk + NEXT_TIMER_MAX_DELTA);
>
> - return next;
> + return;
> }
>
> #ifdef CONFIG_NO_HZ_COMMON
> @@ -1701,7 +1704,7 @@ u64 get_next_timer_interrupt(unsigned long basej, u64 basem)
>
> raw_spin_lock(&base->lock);
> if (base->next_expiry_recalc)
> - base->next_expiry = __next_timer_interrupt(base);
> + __next_timer_interrupt(base);
> nextevt = base->next_expiry;
>
> /*
> @@ -1784,7 +1787,7 @@ static inline void __run_timers(struct timer_base *base)
> WARN_ON_ONCE(!levels && !base->next_expiry_recalc
> && base->timers_pending);
> base->clk++;
> - base->next_expiry = __next_timer_interrupt(base);
> + __next_timer_interrupt(base);
>
> while (levels--)
> expire_timers(base, heads + levels);
> --
> 2.30.2
>