On Fri, 22 May 2026 16:39:41 -0400 Rik van Riel <[email protected]> wrote:
> On Wed, 2026-05-20 at 16:41 -0400, Steven Rostedt wrote: > > > > Rik, > > > > Can you reply to Sashiko here? > > > > The below comes from Sashiko: > > https://sashiko.dev/?list=org.kernel.vger.linux-trace-kernel#/patchset/20260513161916.04151502%40fangorn > > > > Thanks, > > > > -- Steve > > > > > > > commit f90ac36f39b79a19c015522bb1caf7ef903caf02 > > > Author: Rik van Riel <[email protected]> > > > perf/ftrace: Fix WARNING in __unregister_ftrace_function > > > This commit attempts to fix a WARN_ON triggered during > > > perf_event_alloc() error > > > cleanup by checking FTRACE_OPS_FL_ENABLED before unregistering the > > > ftrace > > > function. The warning occurs because the ftrace_ops registration > > > failed or was > > > torn down by perf_try_init_event() earlier, but the event->destroy > > > callback is > > > executed again during the error path. > > > > diff --git a/kernel/trace/trace_event_perf.c > > > > b/kernel/trace/trace_event_perf.c > > > > index 61e3a2620fa3c..8d64f17135e8f 100644 > > > > --- a/kernel/trace/trace_event_perf.c > > > > +++ b/kernel/trace/trace_event_perf.c > > > > @@ -495,7 +495,11 @@ static int > > > > perf_ftrace_function_register(struct perf_event *event) > > > > static int perf_ftrace_function_unregister(struct perf_event > > > > *event) > > > > { > > > > struct ftrace_ops *ops = &event->ftrace_ops; > > > > - int ret = unregister_ftrace_function(ops); > > > > + int ret = 0; > > > > + > > > > + if (ops->flags & FTRACE_OPS_FL_ENABLED) > > > > + ret = unregister_ftrace_function(ops); > > > > + > > > > ftrace_free_filter(ops); > > > > > Does calling ftrace_free_filter() unconditionally here mask a > > > double-teardown > > > regression while leaving the underlying double-free active? > > I don't see how calling ftrace_free_filter() twice would > call issues, given that it sets the ->*_hash values to > EMPTY_HASH: > > void ftrace_free_filter(struct ftrace_ops *ops) > { > ftrace_ops_init(ops); > if (WARN_ON(ops->flags & FTRACE_OPS_FL_ENABLED)) > return; > free_ftrace_hash(ops->func_hash->filter_hash); > free_ftrace_hash(ops->func_hash->notrace_hash); > ops->func_hash->filter_hash = EMPTY_HASH; > ops->func_hash->notrace_hash = EMPTY_HASH; > } > > void free_ftrace_hash(struct ftrace_hash *hash) > { > if (!hash || hash == EMPTY_HASH) > return; > .. > Yeah, confirmed. > > > > In perf_try_init_event(), if a PMU event_init() succeeds but a > > > subsequent > > > capability check fails, it explicitly calls event->destroy(event) > > > to roll back: > > > kernel/events/core.c:perf_try_init_event() { > > > ... > > > if (ret && event->destroy) > > > event->destroy(event); > > > ... > > > } > > The error handling there all seems to "goto err_destroy" > > err_destroy: > if (event->destroy) { > event->destroy(event); > event->destroy = NULL; > } > > > > > However, it does not set event->destroy to NULL. > > ... but it does? > > I am not sure what code Sashiko is looking at, > but it does not look like the code I just pulled. Indeed. > > Is there a different tree I should be looking at > than upstream Linus? You can see the baseline info if you expand the collapsed triangle. Anyway, it said: linux-trace/HEAD (70575e77839f4c5337ce2653b39b86bb365a870e) So that is linux-trace/master. commit 70575e77839f4c5337ce2653b39b86bb365a870e (linux-trace/master) Merge: 7bc6e90d7aa4 a43ae8057cc1 Author: Linus Torvalds <[email protected]> Date: Fri Sep 30 09:41:34 2022 -0700 Merge tag 'for_linus' of git://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost Hmm, this is too old. And linux-trace/master is not used anymore. Reported to Sashiko. https://github.com/sashiko-dev/sashiko/issues/218 Thank you, -- Masami Hiramatsu (Google) <[email protected]>
