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

[Song Chen]

Hi,


在 2026/3/6 17:57, Petr Mladek 写道:
> 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@xxxxxxx> 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.
Thanks so much, this is the solution i'm working on. I replaced next 
with a list_head in notifier_block and implemented 
anotifier_call_chain_reverse to address the order issues, like your 
suggestion. And a new robust revision for rolling back.

+static int notifier_call_chain_reverse(struct list_head *nl,
+                    struct notifier_block *start,
+                    unsigned long val, void *v,
+                    int nr_to_call, int *nr_calls)
+{
+    int ret = NOTIFY_DONE;
+    struct notifier_block *nb;
+    bool do_call = (start == NULL);
+
+    if (!nr_to_call)
+        return ret;
+
+    list_for_each_entry_reverse(nb, nl, entry) {
+        if (!do_call) {
+            if (nb == start)
+                do_call = true;
+            continue;
+        }
+#ifdef CONFIG_DEBUG_NOTIFIERS
+        if (unlikely(!func_ptr_is_kernel_text(nb->notifier_call))) {
+            WARN(1, "Invalid notifier called!");
+            continue;
+        }
+#endif
+        trace_notifier_run((void *)nb->notifier_call);
+        ret = nb->notifier_call(nb, val, v);
+
+        if (nr_calls)
+            (*nr_calls)++;
+
+        if (ret & NOTIFY_STOP_MASK)
+            break;
+
+        if (nr_to_call-- == 0)
+            break;
+    }
+    return ret;
+}
+NOKPROBE_SYMBOL(notifier_call_chain_reverse);

I'll send the patches for review soon.

Best regards

Song
>    + 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