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

Karl Mehltretter <[email protected]>
Newsgroups dev.linux.lists.linux-rt-devel,org.kernel.vger.linux-kernel
Message-ID <[email protected]>
On Thu, Aug 13, 2026 at 03:08:26PM +0100, Peter Zijlstra wrote:
> 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
> 

Thanks, I tested this.

The idea helps, but the ksirqd check is not enough.

__do_softirq() also runs from task context and ktimerd with ksirqd=false.
Subtracting HARDIRQ_OFFSET there corrupts preempt_count.

In the direct IRQ-exit path, __local_bh_disable_ip() also warns because
HARDIRQ_OFFSET is still set.

My version below checks in_hardirq() in handle_softirqs(). For direct
IRQ-exit dispatch it replaces HARDIRQ_OFFSET with SOFTIRQ_OFFSET, then
restores it.

Other callers keep the normal local-BH accounting. tick_irq_exit() still
runs after HARDIRQ_OFFSET is dropped.

In a 400-round KCOV test, flaky PCs fell from 140 to 129. The average
trace size fell from 147.7 to 141.6 PCs. But measured syzcaller coverage
did not improve significantly.

I also ran a targeted KCSAN test that intentionally produced 58
reports on each kernel. Baseline classified all IRQ-exit accesses as
"by task". With this patch, all were "by interrupt". Both passed the
KCSAN selftest. Lockdep and IRQ-tracing boots also passed.

This patch does not replace the KCOV pause series. It only covers work
dispatched directly from IRQ exit. Deferred rearm outside this path
and task-context scheduler leaks remain.

Karl

---
diff --git a/kernel/softirq.c b/kernel/softirq.c
index 7980a4a232f..d6be8ca2793 100644
--- a/kernel/softirq.c
+++ b/kernel/softirq.c
@@ -350,8 +350,8 @@ static inline void ksoftirqd_run_end(void)
 	local_irq_enable();
 }
 
-static inline void softirq_handle_begin(void) { }
-static inline void softirq_handle_end(void) { }
+static inline bool softirq_handle_begin(void) { return false; }
+static inline void softirq_handle_end(bool from_hardirq) { }
 
 static inline bool should_wake_ksoftirqd(void)
 {
@@ -481,15 +481,35 @@ 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 bool softirq_handle_begin(void)
 {
-	__local_bh_disable_ip(_RET_IP_, SOFTIRQ_OFFSET);
+	bool from_hardirq = in_hardirq();
+
+	if (!from_hardirq) {
+		__local_bh_disable_ip(_RET_IP_, SOFTIRQ_OFFSET);
+		return false;
+	}
+
+	/* Replace the retained hardirq context with normal softirq context. */
+	__preempt_count_add((int)SOFTIRQ_OFFSET - (int)HARDIRQ_OFFSET);
+	if (softirq_count() == SOFTIRQ_OFFSET)
+		lockdep_softirqs_off(_RET_IP_);
+
+	return true;
 }
 
-static inline void softirq_handle_end(void)
+static inline void softirq_handle_end(bool from_hardirq)
 {
-	__local_bh_enable(SOFTIRQ_OFFSET);
-	WARN_ON_ONCE(in_interrupt());
+	if (!from_hardirq) {
+		__local_bh_enable(SOFTIRQ_OFFSET);
+		WARN_ON_ONCE(in_interrupt());
+		return;
+	}
+
+	if (softirq_count() == SOFTIRQ_OFFSET)
+		lockdep_softirqs_on(_RET_IP_);
+	__preempt_count_sub((int)SOFTIRQ_OFFSET - (int)HARDIRQ_OFFSET);
+	WARN_ON_ONCE(!in_hardirq());
 }
 
 static inline void ksoftirqd_run_begin(void)
@@ -605,6 +625,7 @@ static void handle_softirqs(bool ksirqd)
 	unsigned long old_flags = current->flags;
 	int max_restart = MAX_SOFTIRQ_RESTART;
 	struct softirq_action *h;
+	bool from_hardirq;
 	bool in_hardirq;
 	__u32 pending;
 	int softirq_bit;
@@ -618,7 +639,7 @@ static void handle_softirqs(bool ksirqd)
 
 	pending = local_softirq_pending();
 
-	softirq_handle_begin();
+	from_hardirq = softirq_handle_begin();
 	in_hardirq = lockdep_softirq_start();
 	account_softirq_enter(current);
 
@@ -670,7 +691,7 @@ static void handle_softirqs(bool ksirqd)
 
 	account_softirq_exit(current);
 	lockdep_softirq_end(in_hardirq);
-	softirq_handle_end();
+	softirq_handle_end(from_hardirq);
 	current_restore_flags(old_flags, PF_MEMALLOC);
 }
 
@@ -740,6 +761,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 +772,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 +780,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 +791,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();
 }
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.