On Mon, 10 Feb 2025 19:13:50 -0500
Steven Rostedt <rost...@goodmis.org> wrote:
> On Tue, 11 Feb 2025 00:44:17 +0900
> "Masami Hiramatsu (Google)" <mhira...@kernel.org> wrote:
>
> > @@ -9857,8 +9861,10 @@ static int __remove_instance(struct trace_array *tr)
> > int i;
> >
> > /* Reference counter for a newly created trace array = 1. */
> > - if (tr->ref > 1 || (tr->current_trace && tr->trace_ref))
> > + if (tr->ref > 1 || (tr->current_trace && tr->trace_ref)) {
> > + pr_info("Instance is busy: ref %d trace_ref %d\n", tr->ref,
> > tr->trace_ref);
>
> I don't think we need the print. The function will error with -EBUSY
> letting the user know that it is busy. This is a very common error, where I
> found that I had a program that didn't completely die, and still had a file
> descriptor open on an instance, and when I go to remove it, I get:
>
> # cd /sys/kernel/tracing/
> # mkdir instances/foo
> # bash 5< instances/foo/events/enable
> # rmdir instances/foo
> rmdir: failed to remove 'instances/foo': Device or resource busy
> # exit
> # rmdir instances/foo
> #
>
> If you only have it for debugging that is fine, but we should remove it
> before we accept it.
Oops, that is my debug code. I missed to remove that! Sorry for confusion.
Thank you!
>
> -- Steve
>
> > return -EBUSY;
> > + }
> >
> > list_del(&tr->list);
> >
> > @@ -9881,6 +9887,11 @@ static int __remove_instance(struct trace_array *tr)
> > free_trace_buffers(tr);
> > clear_tracing_err_log(tr);
> >
> > + if (tr->range_name) {
> > + reserved_mem_release_by_name(tr->range_name);
> > + kfree(tr->range_name);
> > + }
> > +
> > for (i = 0; i < tr->nr_topts; i++) {
> > kfree(tr->topts[i].topts);
> > }
> > @@ -10740,6 +10751,7 @@ __init static void enable_instances(void)
> > bool traceoff = false;
> > char *flag_delim;
> > char *addr_delim;
> > + char *rname __free(kfree) = NULL;
> >
> > tok = strsep(&curr_str, ",");
> >
> > @@ -10796,6 +10808,7 @@ __init static void enable_instances(void)
> > pr_warn("Failed to map boot instance %s to
> > %s\n", name, tok);
> > continue;
> > }
> > + rname = kstrdup(tok, GFP_KERNEL);
> > }
> >
> > if (start) {
> > @@ -10832,7 +10845,7 @@ __init static void enable_instances(void)
> > */
> > if (start) {
> > tr->flags |= TRACE_ARRAY_FL_BOOT |
> > TRACE_ARRAY_FL_LAST_BOOT;
> > - tr->ref++;
> > + tr->range_name = no_free_ptr(rname);
> > }
> >
> > while ((tok = strsep(&curr_str, ","))) {
> > diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
> > index 47c0742fe9ec..ae8f7fac6592 100644
> > --- a/kernel/trace/trace.h
> > +++ b/kernel/trace/trace.h
> > @@ -348,6 +348,7 @@ struct trace_array {
> > unsigned int mapped;
> > unsigned long range_addr_start;
> > unsigned long range_addr_size;
> > + char *range_name;
> > long text_delta;
> > int nr_modules;
> > long *module_delta;
>
--
Masami Hiramatsu (Google) <mhira...@kernel.org>
