Re: [PATCH] kernel/trace/ftrace: introduce ftrace module notifier

[Miroslav Benes <mbenes@suse.cz>]

[-- Attachment #1: Type: text/plain, Size: 3620 bytes --]

Hi,

+ Cc: live-patching@vger.kernel.org

On Thu, 26 Feb 2026, Song Chen wrote:

> Hi,
> 
> 在 2026/2/26 08:27, Steven Rostedt 写道:
> > On Wed, 25 Feb 2026 13:46:39 +0800
> > chensong_2000@189.cn wrote:
> > 
> >> From: Song Chen <chensong_2000@189.cn>
> >>
> >> Like kprobe, fprobe and btf, this patch attempts to introduce
> >> a notifier_block for ftrace to decouple its initialization from
> >> load_module.
> >>
> >> Below is the table of ftrace fucntions calls in different
> >> module state:
> >>
> >>  MODULE_STATE_UNFORMED	ftrace_module_init
> >>  MODULE_STATE_COMING	ftrace_module_enable
> >>  MODULE_STATE_LIVE	ftrace_free_mem
> >>  MODULE_STATE_GOING	ftrace_release_mod
> >>
> >> Unlike others, ftrace module notifier must take care of state
> >> MODULE_STATE_UNFORMED to ensure calling ftrace_module_init
> >> before complete_formation which changes module's text property.
> >>
> >> That pretty much remains same logic with its original design,
> >> the only thing that changes is blocking_notifier_call_chain
> >> (MODULE_STATE_GOING) has to be moved from coming_cleanup to
> >> ddebug_cleanup in function load_module to ensure
> >> ftrace_release_mod is invoked in case complete_formation fails.
> >>
> >> Signed-off-by: Song Chen <chensong_2000@189.cn>
> >> ---
> >>   kernel/module/main.c  | 14 ++++----------
> >>   kernel/trace/ftrace.c | 37 +++++++++++++++++++++++++++++++++++++
> >>   2 files changed, 41 insertions(+), 10 deletions(-)
> >>
> >> diff --git a/kernel/module/main.c b/kernel/module/main.c
> >> index 710ee30b3bea..5dc0a980e9bd 100644
> >> --- a/kernel/module/main.c
> >> +++ b/kernel/module/main.c
> >> @@ -45,7 +45,6 @@
> >>   #include <linux/license.h>
> >>   #include <asm/sections.h>
> >>   #include <linux/tracepoint.h>
> >> -#include <linux/ftrace.h>
> >>   #include <linux/livepatch.h>
> >>   #include <linux/async.h>
> >>   #include <linux/percpu.h>
> >> @@ -836,7 +835,6 @@ SYSCALL_DEFINE2(delete_module, const char __user *,
> >> name_user,
> >>    blocking_notifier_call_chain(&module_notify_list,
> >>    			     MODULE_STATE_GOING, mod);
> >>   	klp_module_going(mod);
> >> -	ftrace_release_mod(mod);
> > 
> > Is the above safe? klp uses ftrace. That means klp_module_going() may
> > need to be called before ftrace_release_mod(). That said, I wonder if
> > klp_module_going() could be moved into ftrace_release_mod()?
> > 
> >>   
> 
> I didn't test with klp, so i'm not sure if it's safe. But i consider klp is
> the other part which should be decoupled after ftrace and klp should introduce
> its own notifier.
> 
> If klp_module_going must be running before ftrace_release_mod, i can try to
> use priority in notifier_block to ensure their order.
> 
> 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.

Regards,
Miroslav