Linux Trace Kernel
 help / color / mirror / Atom feed
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>

  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