All of lore.kernel.org
 help / color / mirror / Atom feed
From: Karl Mehltretter <kmehltretter@gmail.com>
To: Peter Zijlstra <peterz@infradead.org>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	 Andrey Konovalov <andreyknvl@gmail.com>,
	Alexander Potapenko <glider@google.com>,
	 Dmitry Vyukov <dvyukov@google.com>,
	Marco Elver <elver@google.com>,
	 Bradley Morgan <include@grrlz.net>,
	Anna-Maria Behnsen <anna-maria@linutronix.de>,
	 Frederic Weisbecker <frederic@kernel.org>,
	Thomas Gleixner <tglx@kernel.org>,
	 Ingo Molnar <mingo@redhat.com>,
	Juri Lelli <juri.lelli@redhat.com>,
	 Vincent Guittot <vincent.guittot@linaro.org>,
	Dietmar Eggemann <dietmar.eggemann@arm.com>,
	 Steven Rostedt <rostedt@goodmis.org>,
	Ben Segall <bsegall@google.com>, Mel Gorman <mgorman@suse.de>,
	 Valentin Schneider <vschneid@redhat.com>,
	K Prateek Nayak <kprateek.nayak@amd.com>,
	 Sebastian Andrzej Siewior <bigeasy@linutronix.de>,
	Clark Williams <clrkwllms@kernel.org>,
	 linux-rt-devel@lists.linux.dev, kasan-dev@googlegroups.com,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 3/6] hrtimer: Pause KCOV during deferred rearm
Date: Mon, 17 Aug 2026 03:43:39 +0200	[thread overview]
Message-ID: <aoJltY3whwRjbmFW@gmail.com> (raw)
In-Reply-To: <20260813130826.GW687043@noisy.programming.kicks-ass.net>

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();
 }
 

  reply	other threads:[~2026-08-17  1:43 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-11 15:41 [PATCH v2 0/6] kcov: Suppress timer and scheduler coverage leaks Karl Mehltretter
2026-08-11 15:41 ` [PATCH v2 1/6] kcov: Use unsigned int for kcov_start() mode parameter Karl Mehltretter
2026-08-11 15:41 ` [PATCH v2 2/6] kcov: Add a kcov_pause guard Karl Mehltretter
2026-08-11 15:41 ` [PATCH v2 3/6] hrtimer: Pause KCOV during deferred rearm Karl Mehltretter
2026-08-12 10:21   ` Peter Zijlstra
2026-08-13  5:59     ` Karl Mehltretter
2026-08-13 13:08       ` Peter Zijlstra
2026-08-17  1:43         ` Karl Mehltretter [this message]
2026-08-11 15:41 ` [PATCH v2 4/6] sched/core: Pause KCOV in __schedule() Karl Mehltretter
2026-08-12 10:35   ` Peter Zijlstra
2026-08-11 15:41 ` [PATCH v2 5/6] sched/core: Pause KCOV in try_to_wake_up() Karl Mehltretter
2026-08-12 10:35   ` Peter Zijlstra
2026-08-11 15:41 ` [PATCH v2 6/6] sched/core: Pause KCOV in wake_up_new_task() Karl Mehltretter
2026-08-12 10:36   ` Peter Zijlstra
2026-08-11 19:59 ` [PATCH v2 0/6] kcov: Suppress timer and scheduler coverage leaks Bradley Morgan
2026-08-12 10:36   ` Peter Zijlstra

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aoJltY3whwRjbmFW@gmail.com \
    --to=kmehltretter@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=andreyknvl@gmail.com \
    --cc=anna-maria@linutronix.de \
    --cc=bigeasy@linutronix.de \
    --cc=bsegall@google.com \
    --cc=clrkwllms@kernel.org \
    --cc=dietmar.eggemann@arm.com \
    --cc=dvyukov@google.com \
    --cc=elver@google.com \
    --cc=frederic@kernel.org \
    --cc=glider@google.com \
    --cc=include@grrlz.net \
    --cc=juri.lelli@redhat.com \
    --cc=kasan-dev@googlegroups.com \
    --cc=kprateek.nayak@amd.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-rt-devel@lists.linux.dev \
    --cc=mgorman@suse.de \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=rostedt@goodmis.org \
    --cc=tglx@kernel.org \
    --cc=vincent.guittot@linaro.org \
    --cc=vschneid@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.