Softirq handling can be preempted in RT kernels. If we use per-CPU buffers
for saving/restoring temporary KCOV contexts, various kcov-related warnings
happen due to state corruption.

This patch unconditionally moves temporary KCOV context buffers from
per-CPU to per-task. Although it is safe to continue using per-CPU buffers
in !RT kernels, this patch tries to unify the logic, for Dmitry Vyukov
commented

  Is it possible to unify the logic between RT and non-RT kernels (not
  have any ifdef's)?
  The logic is already rather complicated, having 2 work modes, and
  limited testing will be problematic in the future.

.

The local_lock_t is removed, for per-CPU buffers are removed.
But kcov_remote_lock and _irqsave/irqrestore remain in order to serialize
access to kcov_remote_areas lists, for kcov_remote_{start,stop}() are
called from both task context and softirq context.

Analyzed-by: AI Mode in Google Search (no mail address)
Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Fixes: 5ff3b30ab57d ("kcov: collect coverage from interrupts")
---
Only compile tested.

 include/linux/sched.h |   8 +++
 kernel/kcov.c         | 111 +++++++++++++++---------------------------
 lib/Kconfig.debug     |   5 +-
 3 files changed, 51 insertions(+), 73 deletions(-)

diff --git a/include/linux/sched.h b/include/linux/sched.h
index ee06cba5c6f5..eaf975138359 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -1525,6 +1525,14 @@ struct task_struct {
 
 	/* Collect coverage from softirq context: */
 	unsigned int			kcov_softirq;
+
+	/* Temporary storage for preempting remote coverage collection: */
+	unsigned int			kcov_saved_mode;
+	unsigned int			kcov_saved_size;
+	void				*kcov_saved_area;
+	struct kcov			*kcov_saved_kcov;
+	int				kcov_saved_sequence;
+
 #endif
 
 #ifdef CONFIG_MEMCG_V1
diff --git a/kernel/kcov.c b/kernel/kcov.c
index 0b369e88c7c9..b96d6ad68f92 100644
--- a/kernel/kcov.c
+++ b/kernel/kcov.c
@@ -86,21 +86,8 @@ struct kcov_remote {
 
 static DEFINE_SPINLOCK(kcov_remote_lock);
 static DEFINE_HASHTABLE(kcov_remote_map, 4);
-static struct list_head kcov_remote_areas = LIST_HEAD_INIT(kcov_remote_areas);
-
-struct kcov_percpu_data {
-	void			*irq_area;
-	local_lock_t		lock;
-
-	unsigned int		saved_mode;
-	unsigned int		saved_size;
-	void			*saved_area;
-	struct kcov		*saved_kcov;
-	int			saved_sequence;
-};
-
-static DEFINE_PER_CPU(struct kcov_percpu_data, kcov_percpu_data) = {
-	.lock = INIT_LOCAL_LOCK(lock),
+static struct list_head kcov_remote_areas[2] = {
+	LIST_HEAD_INIT(kcov_remote_areas[0]), LIST_HEAD_INIT(kcov_remote_areas[1])
 };
 
 /* Must be called with kcov_remote_lock locked. */
@@ -136,8 +123,9 @@ static struct kcov_remote_area *kcov_remote_area_get(unsigned int size)
 {
 	struct kcov_remote_area *area;
 	struct list_head *pos;
+	struct list_head *list = &kcov_remote_areas[size == CONFIG_KCOV_IRQ_AREA_SIZE];
 
-	list_for_each(pos, &kcov_remote_areas) {
+	list_for_each(pos, list) {
 		area = list_entry(pos, struct kcov_remote_area, list);
 		if (area->size == size) {
 			list_del(&area->list);
@@ -153,7 +141,7 @@ static void kcov_remote_area_put(struct kcov_remote_area *area,
 {
 	INIT_LIST_HEAD(&area->list);
 	area->size = size;
-	list_add(&area->list, &kcov_remote_areas);
+	list_add(&area->list, &kcov_remote_areas[size == CONFIG_KCOV_IRQ_AREA_SIZE]);
 	/*
 	 * KMSAN doesn't instrument this file, so it may not know area->list
 	 * is initialized. Unpoison it explicitly to avoid reports in
@@ -824,37 +812,32 @@ static inline bool kcov_mode_enabled(unsigned int mode)
 }
 
 static void kcov_remote_softirq_start(struct task_struct *t)
-	__must_hold(&kcov_percpu_data.lock)
 {
-	struct kcov_percpu_data *data = this_cpu_ptr(&kcov_percpu_data);
 	unsigned int mode;
 
 	mode = READ_ONCE(t->kcov_mode);
 	barrier();
 	if (kcov_mode_enabled(mode)) {
-		data->saved_mode = mode;
-		data->saved_size = t->kcov_size;
-		data->saved_area = t->kcov_area;
-		data->saved_sequence = t->kcov_sequence;
-		data->saved_kcov = t->kcov;
+		t->kcov_saved_mode = mode;
+		t->kcov_saved_size = t->kcov_size;
+		t->kcov_saved_area = t->kcov_area;
+		t->kcov_saved_sequence = t->kcov_sequence;
+		t->kcov_saved_kcov = t->kcov;
 		kcov_stop(t);
 	}
 }
 
 static void kcov_remote_softirq_stop(struct task_struct *t)
-	__must_hold(&kcov_percpu_data.lock)
 {
-	struct kcov_percpu_data *data = this_cpu_ptr(&kcov_percpu_data);
-
-	if (data->saved_kcov) {
-		kcov_start(t, data->saved_kcov, data->saved_size,
-				data->saved_area, data->saved_mode,
-				data->saved_sequence);
-		data->saved_mode = 0;
-		data->saved_size = 0;
-		data->saved_area = NULL;
-		data->saved_sequence = 0;
-		data->saved_kcov = NULL;
+	if (t->kcov_saved_kcov) {
+		kcov_start(t, t->kcov_saved_kcov, t->kcov_saved_size,
+			   t->kcov_saved_area, t->kcov_saved_mode,
+			   t->kcov_saved_sequence);
+		t->kcov_saved_mode = 0;
+		t->kcov_saved_size = 0;
+		t->kcov_saved_area = NULL;
+		t->kcov_saved_sequence = 0;
+		t->kcov_saved_kcov = NULL;
 	}
 }
 
@@ -874,15 +857,12 @@ void kcov_remote_start(u64 handle)
 	if (!in_task() && !in_softirq_really())
 		return;
 
-	local_lock_irqsave(&kcov_percpu_data.lock, flags);
-
 	/*
 	 * Check that kcov_remote_start() is not called twice in background
 	 * threads nor called by user tasks (with enabled kcov).
 	 */
 	mode = READ_ONCE(t->kcov_mode);
 	if (WARN_ON(in_task() && kcov_mode_enabled(mode))) {
-		local_unlock_irqrestore(&kcov_percpu_data.lock, flags);
 		return;
 	}
 	/*
@@ -891,15 +871,13 @@ void kcov_remote_start(u64 handle)
 	 * happened while collecting coverage from a background thread.
 	 */
 	if (WARN_ON(in_serving_softirq() && t->kcov_softirq)) {
-		local_unlock_irqrestore(&kcov_percpu_data.lock, flags);
 		return;
 	}
 
-	spin_lock(&kcov_remote_lock);
+	spin_lock_irqsave(&kcov_remote_lock, flags);
 	remote = kcov_remote_find(handle);
 	if (!remote) {
-		spin_unlock(&kcov_remote_lock);
-		local_unlock_irqrestore(&kcov_percpu_data.lock, flags);
+		spin_unlock_irqrestore(&kcov_remote_lock, flags);
 		return;
 	}
 	kcov_debug("handle = %llx, context: %s\n", handle,
@@ -920,19 +898,17 @@ void kcov_remote_start(u64 handle)
 		area = kcov_remote_area_get(size);
 	} else {
 		size = CONFIG_KCOV_IRQ_AREA_SIZE;
-		area = this_cpu_ptr(&kcov_percpu_data)->irq_area;
+		area = kcov_remote_area_get(size);
 	}
-	spin_unlock(&kcov_remote_lock);
+	spin_unlock_irqrestore(&kcov_remote_lock, flags);
 
-	/* Can only happen when in_task(). */
+	/* Allocate new buffer if we can sleep. */
 	if (!area) {
-		local_unlock_irqrestore(&kcov_percpu_data.lock, flags);
-		area = vmalloc(size * sizeof(unsigned long));
+		area = in_task() ? vmalloc(size * sizeof(unsigned long)) : NULL;
 		if (!area) {
 			kcov_put(kcov);
 			return;
 		}
-		local_lock_irqsave(&kcov_percpu_data.lock, flags);
 	}
 
 	/* Reset coverage size. */
@@ -943,9 +919,6 @@ void kcov_remote_start(u64 handle)
 		t->kcov_softirq = 1;
 	}
 	kcov_start(t, kcov, size, area, mode, sequence);
-
-	local_unlock_irqrestore(&kcov_percpu_data.lock, flags);
-
 }
 EXPORT_SYMBOL(kcov_remote_start);
 
@@ -1027,12 +1000,9 @@ void kcov_remote_stop(void)
 	if (!in_task() && !in_softirq_really())
 		return;
 
-	local_lock_irqsave(&kcov_percpu_data.lock, flags);
-
 	mode = READ_ONCE(t->kcov_mode);
 	barrier();
 	if (!kcov_mode_enabled(mode)) {
-		local_unlock_irqrestore(&kcov_percpu_data.lock, flags);
 		return;
 	}
 	/*
@@ -1040,12 +1010,10 @@ void kcov_remote_stop(void)
 	 * actually found the remote handle and started collecting coverage.
 	 */
 	if (in_serving_softirq() && !t->kcov_softirq) {
-		local_unlock_irqrestore(&kcov_percpu_data.lock, flags);
 		return;
 	}
 	/* Make sure that kcov_softirq is only set when in softirq. */
 	if (WARN_ON(!in_serving_softirq() && t->kcov_softirq)) {
-		local_unlock_irqrestore(&kcov_percpu_data.lock, flags);
 		return;
 	}
 
@@ -1069,13 +1037,9 @@ void kcov_remote_stop(void)
 		kcov_move_area(kcov->mode, kcov->area, kcov->size, area);
 	spin_unlock(&kcov->lock);
 
-	if (in_task()) {
-		spin_lock(&kcov_remote_lock);
-		kcov_remote_area_put(area, size);
-		spin_unlock(&kcov_remote_lock);
-	}
-
-	local_unlock_irqrestore(&kcov_percpu_data.lock, flags);
+	spin_lock_irqsave(&kcov_remote_lock, flags);
+	kcov_remote_area_put(area, size);
+	spin_unlock_irqrestore(&kcov_remote_lock, flags);
 
 	/* Get in kcov_remote_start(). */
 	kcov_put(kcov);
@@ -1119,14 +1083,19 @@ static void __init selftest(void)
 
 static int __init kcov_init(void)
 {
-	int cpu;
+	int cpu = num_possible_cpus();
 
-	for_each_possible_cpu(cpu) {
-		void *area = vmalloc_node(CONFIG_KCOV_IRQ_AREA_SIZE *
-				sizeof(unsigned long), cpu_to_node(cpu));
-		if (!area)
-			return -ENOMEM;
-		per_cpu_ptr(&kcov_percpu_data, cpu)->irq_area = area;
+#ifdef CONFIG_PREEMPT_RT
+	/* Allocate some extra buffers in order to prepare for softirq preemption. */
+	cpu = cpu >= 4 ? cpu * 2 : cpu + 4;
+#endif
+	while (cpu--) {
+		void *area = vmalloc(CONFIG_KCOV_IRQ_AREA_SIZE * sizeof(unsigned long));
+		unsigned long flags;
+
+		spin_lock_irqsave(&kcov_remote_lock, flags);
+		kcov_remote_area_put(area, CONFIG_KCOV_IRQ_AREA_SIZE);
+		spin_unlock_irqrestore(&kcov_remote_lock, flags);
 	}
 
 	/*
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 8ff5adcfe1e0..b9e6798dc66d 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -2247,10 +2247,11 @@ config KCOV_INSTRUMENT_ALL
 config KCOV_IRQ_AREA_SIZE
 	hex "Size of interrupt coverage collection area in words"
 	depends on KCOV
+	range 0x80 0x1000000
 	default 0x40000
 	help
-	  KCOV uses preallocated per-cpu areas to collect coverage from
-	  soft interrupts. This specifies the size of those areas in the
+	  KCOV uses preallocated areas to collect coverage from soft
+	  interrupts. This specifies the size of those areas in the
 	  number of unsigned long words.
 
 config KCOV_SELFTEST
-- 
2.54.0

On 2026-05-22 20:10:47 [+0900], Tetsuo Handa wrote:
> --- a/kernel/kcov.c
> +++ b/kernel/kcov.c
> @@ -86,21 +86,8 @@ struct kcov_remote {
>  
>  static DEFINE_SPINLOCK(kcov_remote_lock);
>  static DEFINE_HASHTABLE(kcov_remote_map, 4);
> -static struct list_head kcov_remote_areas = LIST_HEAD_INIT(kcov_remote_areas);
> -
> -struct kcov_percpu_data {
> -	void			*irq_area;
> -	local_lock_t		lock;
> -
> -	unsigned int		saved_mode;
> -	unsigned int		saved_size;
> -	void			*saved_area;
> -	struct kcov		*saved_kcov;
> -	int			saved_sequence;
> -};
> -
> -static DEFINE_PER_CPU(struct kcov_percpu_data, kcov_percpu_data) = {
> -	.lock = INIT_LOCAL_LOCK(lock),
> +static struct list_head kcov_remote_areas[2] = {
> +	LIST_HEAD_INIT(kcov_remote_areas[0]), LIST_HEAD_INIT(kcov_remote_areas[1])
>  };
>  
>  /* Must be called with kcov_remote_lock locked. */
> @@ -136,8 +123,9 @@ static struct kcov_remote_area *kcov_remote_area_get(unsigned int size)
>  {
>  	struct kcov_remote_area *area;
>  	struct list_head *pos;
> +	struct list_head *list = &kcov_remote_areas[size == CONFIG_KCOV_IRQ_AREA_SIZE];

This gets probably the job done but the two lists and check condition
looks a bit hackish. The size is not really important, it is user-sized
buffer vs softirq-buffer. Having an explicit might not be bad.

> @@ -1119,14 +1083,19 @@ static void __init selftest(void)
>  
>  static int __init kcov_init(void)
>  {
> -	int cpu;
> +	int cpu = num_possible_cpus();
>  
> -	for_each_possible_cpu(cpu) {
> -		void *area = vmalloc_node(CONFIG_KCOV_IRQ_AREA_SIZE *
> -				sizeof(unsigned long), cpu_to_node(cpu));
> -		if (!area)
> -			return -ENOMEM;
> -		per_cpu_ptr(&kcov_percpu_data, cpu)->irq_area = area;
> +#ifdef CONFIG_PREEMPT_RT
> +	/* Allocate some extra buffers in order to prepare for softirq preemption. */
> +	cpu = cpu >= 4 ? cpu * 2 : cpu + 4;
> +#endif

Maybe IS_ENABLED(). This assumes that we have twice as many tasks in
softirq context as we have CPUs. Is probably good enough.

Are the sizes somehow important? Any reason why there should not be a
"sane" lower size and then fallback to the user-buffer if it runs out of
softirqs buffers?

The user may request multiple sized buffers via KCOV_REMOTE_ENABLE, are
they cleaned up? The question would could the softirq use the user sized
buffers if any.

> +	while (cpu--) {
> +		void *area = vmalloc(CONFIG_KCOV_IRQ_AREA_SIZE * sizeof(unsigned long));
> +		unsigned long flags;
> +
> +		spin_lock_irqsave(&kcov_remote_lock, flags);
> +		kcov_remote_area_put(area, CONFIG_KCOV_IRQ_AREA_SIZE);
> +		spin_unlock_irqrestore(&kcov_remote_lock, flags);
>  	}
>  
>  	/*

Sebastian