Re: [PATCH v9 2/3] tracing: Remove the backup instance automatically after read

[Masami Hiramatsu (Google) <mhiramat@kernel.org>]

On Wed, 1 Apr 2026 10:40:01 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:

> On Wed, 1 Apr 2026 12:19:57 +0900
> Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote:
> 
> > > 
> > >   CPU 0							CPU 1
> > >   -----							-----
> > >   open(trace_pipe);
> > >   read(..);
> > >   close(trace_pipe);
> > >      kick the work queue to delete it....
> > > 						rmdir();
> > > 							[instance deleted]  
> > 
> > I thought this requires trace_types_lock, and after kicked the queue,
> > can rmdir() gets the tr? (__trace_array_get() return error if
> > tr->free_on_close is set)
> 
> rmdir() doesn't use __trace_array_get(), it uses trace_array_find() which
> we shouldn't need to modify.
> 
> static int instance_rmdir(const char *name)
> {
> 	struct trace_array *tr;
> 
> 	guard(mutex)(&event_mutex);
> 	guard(mutex)(&trace_types_lock);
> 
> 	tr = trace_array_find(name);
> 	if (!tr)
> 		return -ENODEV;
> 
> 	return __remove_instance(tr);
> }

Oops, OK it must be updated too.

Thanks,

> 
> > 
> > > 
> > >   __remove_instance();
> > > 
> > >    [ now the tr is freed, and the remove will crash!]
> > > 
> > > 
> > > What would prevent this is this is to use trace_array_destroy() that checks
> > > this and also adds the proper locking:
> > > 
> > > static void trace_array_autoremove(struct work_struct *work)
> > > {
> > > 	struct trace_array *tr = container_of(work, struct trace_array, autoremove_work);
> > > 
> > > 	trace_array_destroy(tr);
> > > }  
> > 
> > OK, let's use it.
> 
> Yes, by using trace_array_destroy(), it will fix this.
> 
> Thanks,
> 
> -- Steve


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