Re: [PATCH v2 3/6] hrtimer: Pause KCOV during deferred rearm

On Thu, Aug 13, 2026 at 07:59:57AM +0200, Karl Mehltretter wrote:
> On Wed, Aug 12, 2026 at 12:21:00PM +0100, Peter Zijlstra wrote:
> > > Deferred hrtimer rearm can run after HARDIRQ_OFFSET is dropped. in_task()
> > > is then true, so KCOV attributes the instrumented timer-reprogramming
> > > subtree to current.
> > 
> > But that is clearly noinstr code; there should be no kcov calls in
> > there.
> > 
> > If kcov is emitted inside noinstr, then kcov is a broken piece of crap
> > and needs to die.
> > 
> > NAK
> 
> Thanks for the review!
> 
> By "instrumented" I meant KCOV-instrumented. The selftest callback comes
> from __hrtimer_rearm_deferred() in ordinary .text, not .noinstr.text.
> 
> On x86, irq_exit_rcu() runs in an instrumentable IDT-entry region.
> __irq_exit_rcu() subtracts hardirq offset before calling

DEFINE_IDTENTRY_IRQ(func)
__visible noinstr void func(regs, error_code)
  run_irq_on_irqstack_cond(__func, regs, vector)
    irq_enter_rcu()
    func
    irq_exit_rcu()

Gah, that is the softirq thing and is indeed just inside the
instrumented code :-( My memory had all the preempt_count fiddling in
the noinst code.

> hrtimer_rearm_deferred(), so check_kcov_mode() sees in_task() and records
> callee coverage for current.
> 
> This is the same class of failure as 477d81a1c47a ("x86/entry: Remove
> unwanted instrumentation in common_interrupt()"). There the hardirq offset
> had not yet been added, here it has already been removed. Its callee
> could be inlined. 

That one was a lot simpler, it really wanted to be noinstr.

> Deferred rearm instead reaches shared hrtimer, tick,
> clockevent and architecture code. Statically excluding the graph
> would be pervasive and also lose coverage from ordinary task context.
> 
> Do you want deferred rearm and its complete call graph converted to
> noinstr, or merely built without KCOV instrumentation?

Bah, so the only reason this one pops is because it is outside of the
softirq code, same for those two wakeups I suppose.

Would something crazy like this work? That closes the holes in the
preempt_count munging around there.

*completely* untested and all that

---
diff --git a/kernel/softirq.c b/kernel/softirq.c
index 7980a4a232f9..42a1b24c4a0c 100644
--- a/kernel/softirq.c
+++ b/kernel/softirq.c
@@ -481,14 +481,14 @@ void __local_bh_enable_ip(unsigned long ip, unsigned int cnt)
 }
 EXPORT_SYMBOL(__local_bh_enable_ip);
 
-static inline void softirq_handle_begin(void)
+static inline void softirq_handle_begin(bool ksirqd)
 {
-	__local_bh_disable_ip(_RET_IP_, SOFTIRQ_OFFSET);
+	__local_bh_disable_ip(_RET_IP_, SOFTIRQ_OFFSET - (!ksirqd)*HARDIRQ_OFFSET);
 }
 
-static inline void softirq_handle_end(void)
+static inline void softirq_handle_end(bool ksirqd)
 {
-	__local_bh_enable(SOFTIRQ_OFFSET);
+	__local_bh_enable(SOFTIRQ_OFFSET - (!ksirqd)*HARDIRQ_OFFSET);
 	WARN_ON_ONCE(in_interrupt());
 }
 
@@ -618,7 +618,7 @@ static void handle_softirqs(bool ksirqd)
 
 	pending = local_softirq_pending();
 
-	softirq_handle_begin();
+	softirq_handle_begin(ksirqd);
 	in_hardirq = lockdep_softirq_start();
 	account_softirq_enter(current);
 
@@ -670,7 +670,7 @@ static void handle_softirqs(bool ksirqd)
 
 	account_softirq_exit(current);
 	lockdep_softirq_end(in_hardirq);
-	softirq_handle_end();
+	softirq_handle_end(ksirqd);
 	current_restore_flags(old_flags, PF_MEMALLOC);
 }
 
@@ -740,6 +740,9 @@ static inline void wake_timersd(void) { }
 
 #endif
 
+#define IRQ_EXIT_TIMERS  (NMI_MASK | HARDIRQ_MASK)
+#define IRQ_EXIT_SOFTIRQ (IRQ_EXIT_TIMERS | HARDIRQ_DISABLE_MASK | SOFTIRQ_MASK)
+
 static inline void __irq_exit_rcu(void)
 {
 #ifndef __ARCH_IRQ_EXIT_IRQS_DISABLED
@@ -748,7 +751,6 @@ static inline void __irq_exit_rcu(void)
 	lockdep_assert_irqs_disabled();
 #endif
 	account_hardirq_exit(current);
-	preempt_count_sub(HARDIRQ_OFFSET);
 	/*
 	 * Interrupts may happen between hardirq_disable_enter() and
 	 * local_irq_save() in local_interrupt_disable(), if irq_exit() invokes
@@ -757,7 +759,7 @@ static inline void __irq_exit_rcu(void)
 	 * hardirq disabling count is already 1, hence we need to prevent
 	 * invoking softirq when a local_interrupt_disable() is ongoing.
 	 */
-	if (!in_interrupt() && !hardirq_disable_count() &&
+	if ((preempt_count() & IRQ_EXIT_SOFTIRQ) == HARDIRQ_OFFSET &&
 	    local_softirq_pending()) {
 		/*
 		 * If we left hrtimers unarmed, make sure to arm them now,
@@ -768,9 +770,11 @@ static inline void __irq_exit_rcu(void)
 	}
 
 	if (IS_ENABLED(CONFIG_IRQ_FORCED_THREADING) && force_irqthreads() &&
-	    local_timers_pending_force_th() && !(in_nmi() | in_hardirq()))
+	    local_timers_pending_force_th() &&
+	    (preempt_count() & IRQ_EXIT_TIMERS) == HARDIRQ_OFFSET)
 		wake_timersd();
 
+	preempt_count_sub(HARDIRQ_OFFSET);
 	tick_irq_exit();
 }