* [RFC PATCH] entry: Account the deferred hrtimer rearm as hardirq context for lockdep
@ 2026-10-08 20:46 Mikhail Gavrilov
2026-10-12 15:21 ` Peter Zijlstra
0 siblings, 1 reply; 4+ messages in thread
From: Mikhail Gavrilov @ 2026-10-08 20:46 UTC (permalink / raw)
To: Thomas Gleixner, Peter Zijlstra
Cc: Andy Lutomirski, Ingo Molnar, Will Deacon, Boqun Feng,
Waiman Long, Sebastian Andrzej Siewior, Clark Williams,
Steven Rostedt, Karl Mehltretter, linux-kernel, linux-rt-devel,
Mikhail Gavrilov
On a desktop debug kernel (PROVE_LOCKING, KASAN, LOCKDEP_CHAINS_BITS=19)
lockdep switches itself off after about two days of uptime with
"BUG: MAX_LOCKDEP_CHAIN_HLOCKS too low!", and locking is not validated
until the next reboot. In the last such boot the pool ran out after 46.6
hours, and a fifth of it, 533509 of 2621431 chain hlocks in 63838 of
344717 chains, had been created by the deferred hrtimer rearm on return
from interrupt.
Make lockdep account that rearm as part of the timer interrupt it
belongs to.
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.
The dependencies recorded on the return path are "lock held with
interrupts enabled -> hrtimer_bases.lock". Such a lock is hardirq-unsafe
and hrtimer_bases.lock is hardirq-safe, so a path in the reverse
direction is already reported by the irq-safety checks without these
edges.
Re-enter lockdep's hardirq context around the rearm on the kernel return
path. The return to user mode path holds no locks and is left alone, as
is the rearm in __schedule(), which runs under the runqueue lock.
With the same kernel tree, configuration and workload, one hour of
uptime including a full kernel build, the chains of this shape, with
hrtimer_bases.lock and optionally the timekeeper sequence count on top
of a held-lock stack outside the scheduler, go from 13266 (96187 chain
hlocks, 15.9% of those in use) to 4. Those 4 come from clock_was_set(),
which takes cpus_read_lock() and each CPU's hrtimer_bases.lock and reads
the clock under it.
Fixes: 15dd3a948855 ("hrtimer: Push reprogramming timers into the interrupt return path")
Signed-off-by: Mikhail Gavrilov <mikhail.v.gavrilov@gmail.com>
---
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?
Tested on x86_64 only (Ryzen 9 7950X), PREEMPT_DYNAMIC. Not tested on
PREEMPT_RT or on the other architectures that use generic entry with
HRTIMER_REARM_DEFERRED.
Method: tree 0c2669a9f4a1 plus local fixes, the same .config
(LOCKDEP_CHAINS_BITS=19, LOCKDEP_BITS=16) and gcc 16.2.1 for both kernels,
built with and without this patch. Each kernel booted fresh and sat at the
login screen with sleep inhibited; a full kernel build (make clean,
make -j32) started at 600 s of uptime, and /proc/lockdep_chains and
/proc/lockdep_stats were read at 3600 s. One run per kernel.
without with
lock chains 97283 81257
chain hlocks used 603921 485100
hrtimer_bases.lock on held-lock stacks outside the
scheduler (the shape above) 13266 4
their chain hlocks 96187 12
hrtimer_bases.lock under the runqueue lock (untouched) 5208 4665
hardirq chains starting with hrtimer_bases.lock 11 11
direct dependencies 36118 34332
deferred rearms in 2 s (hrtimer_rearm with deferred=1) 534 578
A chain is counted in the third row when it is in process or softirq
context, hrtimer_bases.lock sits on a non-empty held-lock stack with no
scheduler lock in it, nothing or one sequence count follows it, and
debugobjects' obj_hash lock never follows hrtimer_bases.lock on that same
stack (that would be the stack's own hrtimer_start()/hrtimer_cancel()).
The four left with the patch:
cpu_hotplug_lock -> hrtimer_bases.lock [-> &____s->seqcount#2]
&dev->mutex -> cpu_hotplug_lock -> hrtimer_bases.lock [-> &____s->seqcount#2]
Beyond the removed chains, the total dropped by another 2764 chains
(2.8%); one pair of runs does not separate that from run-to-run spread,
the untouched runqueue-lock row moved by 10% between the two runs.
Peter suggested looking for such patterns in lockdep_chains here:
https://lore.kernel.org/all/20250310112315.GS16878@noisy.programming.kicks-ass.net/
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();
--
2.56.0
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [RFC PATCH] entry: Account the deferred hrtimer rearm as hardirq context for lockdep
2026-10-08 20:46 [RFC PATCH] entry: Account the deferred hrtimer rearm as hardirq context for lockdep Mikhail Gavrilov
@ 2026-10-12 15:21 ` Peter Zijlstra
2026-10-13 6:04 ` Mikhail Gavrilov
0 siblings, 1 reply; 4+ messages in thread
From: Peter Zijlstra @ 2026-10-12 15:21 UTC (permalink / raw)
To: Mikhail Gavrilov
Cc: Thomas Gleixner, Andy Lutomirski, Ingo Molnar, Will Deacon,
Boqun Feng, Waiman Long, Sebastian Andrzej Siewior,
Clark Williams, Steven Rostedt, Karl Mehltretter, linux-kernel,
linux-rt-devel
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?
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [RFC PATCH] entry: Account the deferred hrtimer rearm as hardirq context for lockdep
2026-10-12 15:21 ` Peter Zijlstra
@ 2026-10-13 6:04 ` Mikhail Gavrilov
2026-10-13 6:38 ` Peter Zijlstra
0 siblings, 1 reply; 4+ messages in thread
From: Mikhail Gavrilov @ 2026-10-13 6:04 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Thomas Gleixner, Andy Lutomirski, Ingo Molnar, Will Deacon,
Boqun Feng, Waiman Long, Sebastian Andrzej Siewior,
Clark Williams, Steven Rostedt, Karl Mehltretter, linux-kernel,
linux-rt-devel
On Mon, Oct 12, 2026 at 05:21:20PM +0200, Peter Zijlstra wrote:
> On Fri, Oct 09, 2026 at 01:46:49AM +0500, Mikhail Gavrilov wrote:
> > 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.
Right, hrtick_start() has always put hrtimer_bases.lock under the
runqueue lock, and 9213aa4784cf ("sched: Default enable HRTICK when
deferred rearming is enabled") is what made it common. I'll leave that
one alone.
> 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?
Yes, no locks can be held there. And since the chain key only covers
the locks taken since the last change of irq context, a rearm on an
empty lock stack lands in the same chains as the rearm in hardirq
context. In the 46.6 hour boot, the 16 chains starting with
hrtimer_bases.lock were all created in hardirq context, and no process
or softirq context chain starts with it.
For v2 I'd add to the comment above the helper:
* The rearm on return to user mode needs no such annotation: no locks
* are held there, so it uses the same lock chains as the rearm in
* hardirq context.
> 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?
Apart from the rearm, the preemption is the only thing in there that
takes locks, and I don't see a way to run it in lockdep's hardirq
context: preempt_schedule_irq() enables interrupts before calling
__schedule(), which trips the lockdep_hardirq_context() check in
lockdep_hardirqs_on_prepare(), and hardirq_context is per CPU, so it
would stay set for whatever task __schedule() switches to.
Nor is the shape specific to the interrupt return: schedule() under a
held sleeping lock stacks rq->lock on that lock the same way. In the
46.6 hour boot, chains with rq->__lock on top of other held locks were
18.1% of the pool, and lockdep_chains does not tell which of them came
from preemption.
--
Thanks,
Mikhail
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [RFC PATCH] entry: Account the deferred hrtimer rearm as hardirq context for lockdep
2026-10-13 6:04 ` Mikhail Gavrilov
@ 2026-10-13 6:38 ` Peter Zijlstra
0 siblings, 0 replies; 4+ messages in thread
From: Peter Zijlstra @ 2026-10-13 6:38 UTC (permalink / raw)
To: Mikhail Gavrilov
Cc: Thomas Gleixner, Andy Lutomirski, Ingo Molnar, Will Deacon,
Boqun Feng, Waiman Long, Sebastian Andrzej Siewior,
Clark Williams, Steven Rostedt, Karl Mehltretter, linux-kernel,
linux-rt-devel
On Tue, Oct 13, 2026 at 11:04:48AM +0500, Mikhail Gavrilov wrote:
> On Mon, Oct 12, 2026 at 05:21:20PM +0200, Peter Zijlstra wrote:
> > 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?
>
> Apart from the rearm, the preemption is the only thing in there that
> takes locks, and I don't see a way to run it in lockdep's hardirq
> context: preempt_schedule_irq() enables interrupts before calling
> __schedule(), which trips the lockdep_hardirq_context() check in
> lockdep_hardirqs_on_prepare(), and hardirq_context is per CPU, so it
> would stay set for whatever task __schedule() switches to.
Right; that won't do.
> Nor is the shape specific to the interrupt return: schedule() under a
> held sleeping lock stacks rq->lock on that lock the same way. In the
> 46.6 hour boot, chains with rq->__lock on top of other held locks were
> 18.1% of the pool, and lockdep_chains does not tell which of them came
> from preemption.
That's roughly 2 out of 11. Not sure that's worth it. If I get another
crazy idea I'll let you know ;-)
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-10-13 6:38 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-08 20:46 [RFC PATCH] entry: Account the deferred hrtimer rearm as hardirq context for lockdep Mikhail Gavrilov
2026-10-12 15:21 ` Peter Zijlstra
2026-10-13 6:04 ` Mikhail Gavrilov
2026-10-13 6:38 ` Peter Zijlstra
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®