diff mbox series

tracing: Fix the unusable problem caused by non-empty pipe_cpumask

Message ID 20230901072626.278880-1-nashuiliang@gmail.com (mailing list archive)
State Superseded
Headers show
Series tracing: Fix the unusable problem caused by non-empty pipe_cpumask | expand

Commit Message

Chuang Wang Sept. 1, 2023, 7:26 a.m. UTC
There is a problem in the linux-6.5 when I execute the following
command:

 $ perf ftrace -t irqsoff
 failed to open trace_pipe

At the same time, when I open this file, the same error occurs.

Therefore, after carefully looking at the code, the open function of
'trace_pipe' returns -EBUSY in open_pipe_on_cpu() because no clearing
operation was performed when 'trace_array->pipe_cpumask' was allocated.

With this patch, Use zalloc_cpumask_var() to ensure that
'trace_array->pipe_cpumask' is reset to 0 when allocated.

Cc: stable@vger.kernel.org
Fixes: c2489bb7e6be ("tracing: Introduce pipe_cpumask to avoid race on trace_pipes")
Signed-off-by: Chuang Wang <nashuiliang@gmail.com>
---
 kernel/trace/trace.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Masami Hiramatsu (Google) Sept. 1, 2023, 2:02 p.m. UTC | #1
On Fri,  1 Sep 2023 15:26:26 +0800
Chuang Wang <nashuiliang@gmail.com> wrote:

> There is a problem in the linux-6.5 when I execute the following
> command:
> 
>  $ perf ftrace -t irqsoff
>  failed to open trace_pipe
> 
> At the same time, when I open this file, the same error occurs.
> 
> Therefore, after carefully looking at the code, the open function of
> 'trace_pipe' returns -EBUSY in open_pipe_on_cpu() because no clearing
> operation was performed when 'trace_array->pipe_cpumask' was allocated.
> 
> With this patch, Use zalloc_cpumask_var() to ensure that
> 'trace_array->pipe_cpumask' is reset to 0 when allocated.

Good catch. This looks good to me.

Reviewed-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>

> 
> Cc: stable@vger.kernel.org
> Fixes: c2489bb7e6be ("tracing: Introduce pipe_cpumask to avoid race on trace_pipes")
> Signed-off-by: Chuang Wang <nashuiliang@gmail.com>
> ---
>  kernel/trace/trace.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> index d8233d34b5a0..c0b8a72f3466 100644
> --- a/kernel/trace/trace.c
> +++ b/kernel/trace/trace.c
> @@ -9461,7 +9461,7 @@ static struct trace_array *trace_array_create(const char *name)
>  	if (!alloc_cpumask_var(&tr->tracing_cpumask, GFP_KERNEL))
>  		goto out_free_tr;
>  
> -	if (!alloc_cpumask_var(&tr->pipe_cpumask, GFP_KERNEL))
> +	if (!zalloc_cpumask_var(&tr->pipe_cpumask, GFP_KERNEL))
>  		goto out_free_tr;
>  
>  	tr->trace_flags = global_trace.trace_flags & ~ZEROED_TRACE_FLAGS;
> @@ -10406,7 +10406,7 @@ __init static int tracer_alloc_buffers(void)
>  	if (trace_create_savedcmd() < 0)
>  		goto out_free_temp_buffer;
>  
> -	if (!alloc_cpumask_var(&global_trace.pipe_cpumask, GFP_KERNEL))
> +	if (!zalloc_cpumask_var(&global_trace.pipe_cpumask, GFP_KERNEL))
>  		goto out_free_savedcmd;
>  
>  	/* TODO: make the number of buffers hot pluggable with CPUS */
> -- 
> 2.39.2
>
Steven Rostedt Sept. 2, 2023, 1:26 a.m. UTC | #2
On Fri, 1 Sep 2023 23:02:06 +0900
Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote:

> On Fri,  1 Sep 2023 15:26:26 +0800
> Chuang Wang <nashuiliang@gmail.com> wrote:
> 
> > There is a problem in the linux-6.5 when I execute the following
> > command:
> > 
> >  $ perf ftrace -t irqsoff
> >  failed to open trace_pipe
> > 
> > At the same time, when I open this file, the same error occurs.
> > 
> > Therefore, after carefully looking at the code, the open function of
> > 'trace_pipe' returns -EBUSY in open_pipe_on_cpu() because no clearing
> > operation was performed when 'trace_array->pipe_cpumask' was allocated.
> > 
> > With this patch, Use zalloc_cpumask_var() to ensure that
> > 'trace_array->pipe_cpumask' is reset to 0 when allocated.  


Brian Foster beat you to it.

  https://lore.kernel.org/linux-trace-kernel/20230831125500.986862-1-bfoster@redhat.com


> 
> Good catch. This looks good to me.
> 
> Reviewed-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> 

Masami, I'll add your Reviewed-by to that commit.

Thanks!

-- Steve

> > 
> > Cc: stable@vger.kernel.org
> > Fixes: c2489bb7e6be ("tracing: Introduce pipe_cpumask to avoid race on trace_pipes")
> > Signed-off-by: Chuang Wang <nashuiliang@gmail.com>
> > ---
> >  kernel/trace/trace.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> > index d8233d34b5a0..c0b8a72f3466 100644
> > --- a/kernel/trace/trace.c
> > +++ b/kernel/trace/trace.c
> > @@ -9461,7 +9461,7 @@ static struct trace_array *trace_array_create(const char *name)
> >  	if (!alloc_cpumask_var(&tr->tracing_cpumask, GFP_KERNEL))
> >  		goto out_free_tr;
> >  
> > -	if (!alloc_cpumask_var(&tr->pipe_cpumask, GFP_KERNEL))
> > +	if (!zalloc_cpumask_var(&tr->pipe_cpumask, GFP_KERNEL))
> >  		goto out_free_tr;
> >  
> >  	tr->trace_flags = global_trace.trace_flags & ~ZEROED_TRACE_FLAGS;
> > @@ -10406,7 +10406,7 @@ __init static int tracer_alloc_buffers(void)
> >  	if (trace_create_savedcmd() < 0)
> >  		goto out_free_temp_buffer;
> >  
> > -	if (!alloc_cpumask_var(&global_trace.pipe_cpumask, GFP_KERNEL))
> > +	if (!zalloc_cpumask_var(&global_trace.pipe_cpumask, GFP_KERNEL))
> >  		goto out_free_savedcmd;
> >  
> >  	/* TODO: make the number of buffers hot pluggable with CPUS */
> > -- 
> > 2.39.2
> >   
> 
>
diff mbox series

Patch

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index d8233d34b5a0..c0b8a72f3466 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -9461,7 +9461,7 @@  static struct trace_array *trace_array_create(const char *name)
 	if (!alloc_cpumask_var(&tr->tracing_cpumask, GFP_KERNEL))
 		goto out_free_tr;
 
-	if (!alloc_cpumask_var(&tr->pipe_cpumask, GFP_KERNEL))
+	if (!zalloc_cpumask_var(&tr->pipe_cpumask, GFP_KERNEL))
 		goto out_free_tr;
 
 	tr->trace_flags = global_trace.trace_flags & ~ZEROED_TRACE_FLAGS;
@@ -10406,7 +10406,7 @@  __init static int tracer_alloc_buffers(void)
 	if (trace_create_savedcmd() < 0)
 		goto out_free_temp_buffer;
 
-	if (!alloc_cpumask_var(&global_trace.pipe_cpumask, GFP_KERNEL))
+	if (!zalloc_cpumask_var(&global_trace.pipe_cpumask, GFP_KERNEL))
 		goto out_free_savedcmd;
 
 	/* TODO: make the number of buffers hot pluggable with CPUS */