public inbox for linux-trace-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Petr Mladek <pmladek@suse.com>
To: Song Chen <chensong_2000@189.cn>
Cc: Steven Rostedt <rostedt@goodmis.org>,
	Miroslav Benes <mbenes@suse.cz>,
	mcgrof@kernel.org, petr.pavlu@suse.com, da.gomez@kernel.org,
	samitolvanen@google.com, atomlin@atomlin.com,
	mhiramat@kernel.org, mark.rutland@arm.com,
	mathieu.desnoyers@efficios.com, linux-modules@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-trace-kernel@vger.kernel.org,
	live-patching@vger.kernel.org
Subject: Re: [PATCH] kernel/trace/ftrace: introduce ftrace module notifier
Date: Fri, 6 Mar 2026 10:57:12 +0100	[thread overview]
Message-ID: <aaqk-GrpCTqO36xj@pathway.suse.cz> (raw)
In-Reply-To: <321d4670-27cb-453f-a50d-426c83894074@189.cn>

On Fri 2026-02-27 09:34:59, Song Chen wrote:
> Hi,
> 
> 在 2026/2/27 01:30, Steven Rostedt 写道:
> > On Thu, 26 Feb 2026 11:51:53 +0100 (CET)
> > Miroslav Benes <mbenes@suse.cz> wrote:
> > 
> > > > Let me see if there is any way to use notifier and remain below calling
> > > > sequence:
> > > > 
> > > > ftrace_module_enable
> > > > klp_module_coming
> > > > blocking_notifier_call_chain_robust(MODULE_STATE_COMING)
> > > > 
> > > > blocking_notifier_call_chain(MODULE_STATE_GOING)
> > > > klp_module_going
> > > > ftrace_release_mod
> > > 
> > > Both klp and ftrace used module notifiers in the past. We abandoned that
> > > and opted for direct calls due to issues with ordering at the time. I do
> > > not have the list of problems at hand but I remember it was very fragile.
> > > 
> > > See commits 7dcd182bec27 ("ftrace/module: remove ftrace module
> > > notifier"), 7e545d6eca20 ("livepatch/module: remove livepatch module
> > > notifier") and their surroundings.
> > > 
> > > So unless there is a reason for the change (which should be then carefully
> > > reviewed and properly tested), I would prefer to keep it as is. What is
> > > the motivation? I am failing to find it in the commit log.
> 
> There is no special motivation, i just read btf initialization in module
> loading and found direct calls of ftrace and klp, i thought they were just
> forgotten to use notifier and i even didn't search git log to verify, sorry
> about that.
> 
> > 
> > Honestly, I do think just decoupling ftrace and live kernel patching from
> > modules is rationale enough, as it makes the code a bit cleaner. But to do
> > so, we really need to make sure there is absolutely no regressions.
> > 
> > Thus, to allow such a change, I would ask those that are proposing it, show
> > a full work flow of how ftrace, live kernel patching, and modules work with
> > each other and why those functions are currently injected in the module code.
> > 
> > As Miroslav stated, we tried to do it via notifiers in the past and it
> > failed. I don't want to find out why they failed by just adding them back
> > to notifiers again. Instead, the reasons must be fully understood and
> > updates made to make sure they will not fail in the future.
> 
> Yes, you are right, i read commit msg of 7dcd182bec27, this patch just
> reverses it simply and will introduce order issue back. I will try to find
> out the problem in the past at first.

AFAIK, the root of the problem is that livepatch uses the ftrace
framework. It means that:

   + ftrace must be initialized before livepatch gets enabled
   + livepatch must be disabled before ftrace support gets removed

My understanding is that this can't be achieved by notifiers easily
because they are always proceed in the same order.

An elegant solution would be to introduce  notifier_reverse_call_chain()
which would process the callbacks in the reverse order. But it might
be non-trivial:

  + We would need to make sure that it does not break some
    existing "hidden" dependencies.

  + notifier_call_chain() uses RCU to process the list of registered
    callbacks. I am not sure how complicated would be to make it safe
    in both directions.

Best Regards,
Petr

      reply	other threads:[~2026-03-06  9:57 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-02-25  5:46 [PATCH] kernel/trace/ftrace: introduce ftrace module notifier chensong_2000
2026-02-26  0:27 ` Steven Rostedt
2026-02-26 10:12   ` Song Chen
2026-02-26 10:51     ` Miroslav Benes
2026-02-26 17:30       ` Steven Rostedt
2026-02-27  1:34         ` Song Chen
2026-03-06  9:57           ` Petr Mladek [this message]

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=aaqk-GrpCTqO36xj@pathway.suse.cz \
    --to=pmladek@suse.com \
    --cc=atomlin@atomlin.com \
    --cc=chensong_2000@189.cn \
    --cc=da.gomez@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-modules@vger.kernel.org \
    --cc=linux-trace-kernel@vger.kernel.org \
    --cc=live-patching@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=mathieu.desnoyers@efficios.com \
    --cc=mbenes@suse.cz \
    --cc=mcgrof@kernel.org \
    --cc=mhiramat@kernel.org \
    --cc=petr.pavlu@suse.com \
    --cc=rostedt@goodmis.org \
    --cc=samitolvanen@google.com \
    /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