Re: [PATCH v2 5/6] entry,hrtimer: Push reprogramming timers into the interrupt return path

[Thomas Gleixner]

On Wed, Jan 21 2026 at 17:20, Peter Zijlstra wrote:
>  	while (ti_work & EXIT_TO_USER_MODE_WORK_LOOP) {
>  
> +		/*
> +		 * If hrtimer need re-arming, do so before enabling IRQs,
> +		 * except when a reschedule is needed, in that case schedule()
> +		 * will do this.
> +		 */
> +		if ((ti_work & (_TIF_NEED_RESCHED |
> +				_TIF_NEED_RESCHED_LAZY |
> +				_TIF_HRTIMER_REARM)) == _TIF_HRTIMER_REARM)
> +			hrtimer_rearm();

Two things I'm not convinced that they are handled correctly:

    1) Interrupts

       After reenabling interrupts and before reaching schedule() an
       interrupt comes in and runs soft interrupt processing for a while
       on the way back, which delays the update until that processing
       completes.

    2) Time slice extension

       When the time slice is granted this will not rearm the clockevent
       device unless the slice hrtimer becomes the first expiring timer
       on that CPU, but even then that misses the full reevaluation of
       the next timer event.

> -static void __hrtimer_rearm(struct hrtimer_cpu_base *cpu_base, ktime_t now)
> +static void __hrtimer_rearm(struct hrtimer_cpu_base *cpu_base,
> +			    ktime_t now, ktime_t expires_next)
>  {
> -	ktime_t expires_next = hrtimer_update_next_event(cpu_base);
> -
>  	cpu_base->expires_next = expires_next;
>  	cpu_base->in_hrtirq = 0;
>  
> @@ -1970,9 +1969,30 @@ void hrtimer_interrupt(struct clock_even
>  		cpu_base->hang_detected = 1;
>  	}
>  
> -	__hrtimer_rearm(cpu_base, now);
> +#ifdef TIF_HRTIMER_REARM
> +	set_thread_flag(TIF_HRTIMER_REARM);
> +#else
> +	__hrtimer_rearm(cpu_base, now, expires_next);
> +#endif

in hrtimer.h where you already have the #ifdef TIF_HRTIMER_REARM
section:

static inline bool hrtimer_set_rearm_delayed()
{
	set_thread_flag(TIF_HRTIMER_REARM);
        return true;
}

and a empty stub returning false for the other case then this becomes:

        if (!hrtimer_set_rearm_delayed())
        	hrtimer_rearm(...);

and the ugly ifdef in the code goes away.

>  	raw_spin_unlock_irqrestore(&cpu_base->lock, flags);
>  }
> +
> +#ifdef TIF_HRTIMER_REARM
> +void _hrtimer_rearm(void)

Grr. I had to read this five times to figure out that we now have

hrtimer_rearm()
 _hrtimer_rearm()
  __hrtimer_rearm()

You clearly ran out of characters to make that obvious:

hrtimer_rearm_delayed()
  hrtimer_rearm()
    hrtimer_do_rearm()

or something like that.

Thanks,

        tglx