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;
..
> > 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.
Is there a different tree I should be looking at
than upstream Linus?
--
All Rights Reversed.
