Messages in this thread | | | Date | Tue, 16 Aug 2022 13:48:43 -0700 | From | Krister Johansen <> | Subject | Re: [PATCH 1/1] tracing: fix a WARN from trace_event_dyn_put_ref |
| |
On Tue, Aug 16, 2022 at 03:24:11PM -0400, Steven Rostedt wrote: > Actually, I think the issue is that the unreg() calls trace_event_put_ref() > but the reg() does not do the try_get_ref(), but the perf_trace_init() > does. Which is the broken symmetry. > > I think if we pull out the trace_event_put_ref() from the unreg() function, > we fix the bug and also put back the symmetry. > > Does this patch work?
Yes, thanks, this change looks correct. It also does build and pass the tests that I included in the original e-mail.
> diff --git a/kernel/trace/trace_event_perf.c b/kernel/trace/trace_event_perf.c > index a114549720d6..61e3a2620fa3 100644 > --- a/kernel/trace/trace_event_perf.c > +++ b/kernel/trace/trace_event_perf.c > @@ -157,7 +157,7 @@ static void perf_trace_event_unreg(struct perf_event *p_event) > int i; > > if (--tp_event->perf_refcount > 0) > - goto out; > + return; > > tp_event->class->reg(tp_event, TRACE_REG_PERF_UNREGISTER, NULL); > > @@ -176,8 +176,6 @@ static void perf_trace_event_unreg(struct perf_event *p_event) > perf_trace_buf[i] = NULL; > } > } > -out: > - trace_event_put_ref(tp_event); > } > > static int perf_trace_event_open(struct perf_event *p_event) > @@ -241,6 +239,7 @@ void perf_trace_destroy(struct perf_event *p_event) > mutex_lock(&event_mutex); > perf_trace_event_close(p_event); > perf_trace_event_unreg(p_event); > + trace_event_put_ref(p_event->tp_event); > mutex_unlock(&event_mutex); > } > > @@ -292,6 +291,7 @@ void perf_kprobe_destroy(struct perf_event *p_event) > mutex_lock(&event_mutex); > perf_trace_event_close(p_event); > perf_trace_event_unreg(p_event); > + trace_event_put_ref(p_event->tp_event); > mutex_unlock(&event_mutex); > > destroy_local_trace_kprobe(p_event->tp_event); > @@ -347,6 +347,7 @@ void perf_uprobe_destroy(struct perf_event *p_event) > mutex_lock(&event_mutex); > perf_trace_event_close(p_event); > perf_trace_event_unreg(p_event); > + trace_event_put_ref(p_event->tp_event); > mutex_unlock(&event_mutex); > destroy_local_trace_uprobe(p_event->tp_event); > } >
If it matters:
Reviewed-by: Krister Johansen <kjlx@templeofstupid.com> Tested-by: Krister Johansen <kjlx@templeofstupid.com>
Thanks again,
-K
| |