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

[Song Chen <chensong_2000@189.cn>]

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@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.
> 
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
> 
>