Re: [PATCH v2] tracing/osnoise: fix potential deadlock in cpu hotplug

[Google]

On Thu, 26 Mar 2026 14:19:53 +0800 (CST)
<hu.shengming@xxxxxxxxxx> wrote:

> From: Luo Haiyang <luo.haiyang@xxxxxxxxxx>
> 
> The following sequence may leads deadlock in cpu hotplug:
> 
>     task1        task2        task3
>     -----        -----        -----
> 
>  mutex_lock(&interface_lock)
> 
>             [CPU GOING OFFLINE]
> 
>             cpus_write_lock();
>             osnoise_cpu_die();
>               kthread_stop(task3);
>                 wait_for_completion();
> 
>                       osnoise_sleep();
>                         mutex_lock(&interface_lock);
> 
>  cpus_read_lock();
> 
>  [DEAD LOCK]
> 
> Fix by swap the order of cpus_read_lock() and mutex_lock(&interface_lock).
> 
> Signed-off-by: Luo Haiyang <luo.haiyang@xxxxxxxxxx>
> 

This looks good to me.

Reviewed-by: Masami Hiramatsu (Google) <mhiramat@xxxxxxxxxx>

Thanks,


> ---
> Changes in v2:
> - update change log
> - Link to v1: https://lore.kernel.org/all/20260324150616953rMo1BWtAZ1nXTNrEFP6hr@xxxxxxxxxx/
> ---
>  kernel/trace/trace_osnoise.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/kernel/trace/trace_osnoise.c b/kernel/trace/trace_osnoise.c
> index dee610e465b9..be6cf0bb3c03 100644
> --- a/kernel/trace/trace_osnoise.c
> +++ b/kernel/trace/trace_osnoise.c
> @@ -2073,8 +2073,8 @@ static void osnoise_hotplug_workfn(struct work_struct *dummy)
>  	if (!osnoise_has_registered_instances())
>  		return;
>  
> -	guard(mutex)(&interface_lock);
>  	guard(cpus_read_lock)();
> +	guard(mutex)(&interface_lock);
>  
>  	if (!cpu_online(cpu))
>  		return;
> @@ -2237,11 +2237,11 @@ static ssize_t osnoise_options_write(struct file *filp, const char __user *ubuf,
>  	if (running)
>  		stop_per_cpu_kthreads();
>  
> -	mutex_lock(&interface_lock);
>  	/*
>  	 * avoid CPU hotplug operations that might read options.
>  	 */
>  	cpus_read_lock();
> +	mutex_lock(&interface_lock);
>  
>  	retval = cnt;
>  
> @@ -2257,8 +2257,8 @@ static ssize_t osnoise_options_write(struct file *filp, const char __user *ubuf,
>  			clear_bit(option, &osnoise_options);
>  	}
>  
> -	cpus_read_unlock();
>  	mutex_unlock(&interface_lock);
> +	cpus_read_unlock();
>  
>  	if (running)
>  		start_per_cpu_kthreads();
> @@ -2345,16 +2345,16 @@ osnoise_cpus_write(struct file *filp, const char __user *ubuf, size_t count,
>  	if (running)
>  		stop_per_cpu_kthreads();
>  
> -	mutex_lock(&interface_lock);
>  	/*
>  	 * osnoise_cpumask is read by CPU hotplug operations.
>  	 */
>  	cpus_read_lock();
> +	mutex_lock(&interface_lock);
>  
>  	cpumask_copy(&osnoise_cpumask, osnoise_cpumask_new);
>  
> -	cpus_read_unlock();
>  	mutex_unlock(&interface_lock);
> +	cpus_read_unlock();
>  
>  	if (running)
>  		start_per_cpu_kthreads();
> -- 
> 2.25.1

-- 
Masami Hiramatsu (Google) <mhiramat@xxxxxxxxxx>