From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
To: Rik van Riel <riel@surriel.com>
Cc: Steven Rostedt <rostedt@goodmis.org>,
Masami Hiramatsu <mhiramat@kernel.org>,
Mathieu Desnoyers <mathieu.desnoyers@efficios.com>,
linux-kernel@vger.kernel.org, linux-trace-kernel@vger.kernel.org,
kernel-team@meta.com, sashiko-bot@kernel.org,
sashiko-reviews@lists.linux.dev
Subject: Re: [PATCH v2] perf/ftrace: Fix WARNING in __unregister_ftrace_function
Date: Mon, 25 May 2026 14:39:47 +0900 [thread overview]
Message-ID: <20260525143947.ea1c0a7b29146d22faa5feda@kernel.org> (raw)
In-Reply-To: <8f8d0cd380f40b84557304d110f38ecefc994a35.camel@surriel.com>
On Fri, 22 May 2026 16:39:41 -0400
Rik van Riel <riel@surriel.com> 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 <riel@surriel.com>
> > > 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 <torvalds@linux-foundation.org>
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) <mhiramat@kernel.org>
next prev parent reply other threads:[~2026-05-25 5:39 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-05-13 20:19 [PATCH v2] perf/ftrace: Fix WARNING in __unregister_ftrace_function Rik van Riel
2026-05-14 4:43 ` Masami Hiramatsu
2026-05-20 20:41 ` Steven Rostedt
2026-05-22 20:39 ` Rik van Riel
2026-05-25 5:39 ` Masami Hiramatsu [this message]
-- strict thread matches above, loose matches on Subject: below --
2026-05-13 16:16 [PATCH] " Rik van Riel
2026-05-13 16:33 ` Steven Rostedt
2026-05-13 17:24 ` [PATCH v2] " Rik van Riel
2026-05-13 18:11 ` Steven Rostedt
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260525143947.ea1c0a7b29146d22faa5feda@kernel.org \
--to=mhiramat@kernel.org \
--cc=kernel-team@meta.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-trace-kernel@vger.kernel.org \
--cc=mathieu.desnoyers@efficios.com \
--cc=riel@surriel.com \
--cc=rostedt@goodmis.org \
--cc=sashiko-bot@kernel.org \
--cc=sashiko-reviews@lists.linux.dev \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox