Re: [PATCH v4] kcov: move kcov_remote_data to task_struct and remove local_lock

[Sebastian Andrzej Siewior]

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