Re: [RFC PATCH] entry: Account the deferred hrtimer rearm as hardirq context for lockdep

[Peter Zijlstra <peterz@infradead.org>]

On Fri, Oct 09, 2026 at 01:46:49AM +0500, Mikhail Gavrilov wrote:

> Commit 15dd3a948855
> ("hrtimer: Push reprogramming timers into the interrupt return path")
> moved the reprogramming of the clock event device out of the timer
> interrupt into the interrupt return path. On return to kernel mode
> irqentry_exit_to_kernel_mode_after_preempt() performs it after
> irq_exit_rcu() has already left lockdep's hardirq context. lockdep
> therefore records hrtimer_bases.lock and the timekeeper sequence count
> as taken by the interrupted context, on top of the locks that context
> holds, and every distinct held-lock stack an interrupt hits adds two
> lock chains. When the rearm runs from irq_enter_rcu() or
> __irq_exit_rcu() it is still inside lockdep's hardirq context, and there
> the same acquisitions reuse one pair of chains.

Urgh.

> Not touched, and a question for you: the rearm in hrtick_schedule_exit()
> also stacks chains on the preempted task's locks, under the runqueue lock.
> Chains with hrtimer_bases.lock under the runqueue lock, this rearm among
> them, were 4.9% of the pool in the boot above. Should it get the same
> treatment?

I'm thinking those would be there before the patch too. We've always
done hrtimer_start() from there. You might not have seen it because
hrtick was default disabled for most.


>  include/linux/irq-entry-common.h | 19 +++++++++++++++++--
>  1 file changed, 17 insertions(+), 2 deletions(-)
> 
> diff --git a/include/linux/irq-entry-common.h b/include/linux/irq-entry-common.h
> index 0bb6c03481fa..521cc30cef08 100644
> --- a/include/linux/irq-entry-common.h
> +++ b/include/linux/irq-entry-common.h
> @@ -468,6 +468,21 @@ static inline void irqentry_exit_to_kernel_mode_preempt(struct pt_regs *regs,
>  		irqentry_exit_cond_resched();
>  }
>  
> +/*
> + * The deferred hrtimer rearm is the tail of the timer interrupt, moved to the
> + * interrupt return path so that it runs after a possible reschedule. By now
> + * irq_exit_rcu() has left lockdep's hardirq context. Re-enter it for the
> + * rearm, so that the hrtimer base lock and the timekeeping sequence count are
> + * not chained onto the locks held by the interrupted context, which would add
> + * new lock chains for every lock stack an interrupt hits.
> + */
> +static __always_inline void irqentry_hrtimer_rearm_deferred(void)
> +{
> +	lockdep_hardirq_enter();
> +	hrtimer_rearm_deferred();
> +	lockdep_hardirq_exit();
> +}
> +
>  /**
>   * irqentry_exit_to_kernel_mode_after_preempt - Establish trace, lockdep and RCU state
>   * @regs:	Pointer to current's pt_regs
> @@ -491,7 +506,7 @@ irqentry_exit_to_kernel_mode_after_preempt(struct pt_regs *regs, irqentry_state_
>  		 */
>  		if (state.exit_rcu) {
>  			instrumentation_begin();
> -			hrtimer_rearm_deferred();
> +			irqentry_hrtimer_rearm_deferred();
>  			/* Tell the tracer that IRET will enable interrupts */
>  			trace_hardirqs_on_prepare();
>  			lockdep_hardirqs_on_prepare();
> @@ -502,7 +517,7 @@ irqentry_exit_to_kernel_mode_after_preempt(struct pt_regs *regs, irqentry_state_
>  		}
>  
>  		instrumentation_begin();
> -		hrtimer_rearm_deferred();
> +		irqentry_hrtimer_rearm_deferred();
>  		/* Covers both tracing and lockdep */
>  		trace_hardirqs_on();
>  		instrumentation_end();

The first question I had was about __exit_to_usermode_prepare(), that
also does hrtimer_rearm, but doesn't get wrapped. I'm thinking that's
not a problem because at that point the lock stack is empty and all
that. But perhaps a comment?

That said; I wonder if we want more of irqentry_exit_to_kernel_mode()
wrapped like this. The schedule_preempt_irq() thing will now also stack
rq->lock and the like on top of 'random' kernel stacks, rather than
keeping it in 'empty' hardirq contexts.

Or am I making a mess of things?