* Re: [RFC PATCH 1/2] kernel/notifier: replace single-linked list with double-linked list for reverse traversal
From: Petr Mladek @ 2026-04-16 14:54 UTC (permalink / raw)
To: David Laight
Cc: chensong_2000, rafael, lenb, mturquette, sboyd, viresh.kumar, agk,
snitzer, mpatocka, bmarzins, song, yukuai, linan122, jason.wessel,
danielt, dianders, horms, davem, edumazet, kuba, pabeni, paulmck,
frederic, mcgrof, petr.pavlu, da.gomez, samitolvanen, atomlin,
jpoimboe, jikos, mbenes, joe.lawrence, rostedt, mhiramat,
mark.rutland, mathieu.desnoyers, linux-modules, linux-kernel,
linux-trace-kernel, linux-acpi, linux-clk, linux-pm,
live-patching, dm-devel, linux-raid, kgdb-bugreport, netdev
In-Reply-To: <20260416133004.07bd2886@pumpkin>
On Thu 2026-04-16 13:30:04, David Laight wrote:
> On Wed, 15 Apr 2026 15:01:37 +0800
> chensong_2000@189.cn wrote:
>
> > From: Song Chen <chensong_2000@189.cn>
> >
> > The current notifier chain implementation uses a single-linked list
> > (struct notifier_block *next), which only supports forward traversal
> > in priority order. This makes it difficult to handle cleanup/teardown
> > scenarios that require notifiers to be called in reverse priority order.
>
> If it is only cleanup/teardown then the list can be order-reversed
> as part of that process at the same time as the list is deleted.
Interesting idea. But it won't work in all situations.
Note that the motivation for this update are the module loader
notifiers which are called several times for each loaded/removed module.
Best Regards,
Petr
^ permalink raw reply
* Re: [RFC PATCH 2/2] kernel/module: Decouple klp and ftrace from load_module
From: Petr Mladek @ 2026-04-16 14:49 UTC (permalink / raw)
To: Petr Pavlu
Cc: Song Chen, rafael, lenb, mturquette, sboyd, viresh.kumar, agk,
snitzer, mpatocka, bmarzins, song, yukuai, linan122, jason.wessel,
danielt, dianders, horms, davem, edumazet, kuba, pabeni, paulmck,
frederic, mcgrof, da.gomez, samitolvanen, atomlin, jpoimboe,
jikos, mbenes, joe.lawrence, rostedt, mhiramat, mark.rutland,
mathieu.desnoyers, linux-modules, linux-kernel,
linux-trace-kernel, linux-acpi, linux-clk, linux-pm,
live-patching, dm-devel, linux-raid, kgdb-bugreport, netdev
In-Reply-To: <1db425bf-58a9-4768-8c38-3ae25d7662a5@suse.com>
On Thu 2026-04-16 13:18:30, Petr Pavlu wrote:
> On 4/15/26 8:43 AM, Song Chen wrote:
> > On 4/14/26 22:33, Petr Pavlu wrote:
> >> On 4/13/26 10:07 AM, chensong_2000@189.cn wrote:
> >>> diff --git a/include/linux/module.h b/include/linux/module.h
> >>> index 14f391b186c6..0bdd56f9defd 100644
> >>> --- a/include/linux/module.h
> >>> +++ b/include/linux/module.h
> >>> @@ -308,6 +308,14 @@ enum module_state {
> >>> MODULE_STATE_COMING, /* Full formed, running module_init. */
> >>> MODULE_STATE_GOING, /* Going away. */
> >>> MODULE_STATE_UNFORMED, /* Still setting it up. */
> >>> + MODULE_STATE_FORMED,
> >>
> >> I don't see a reason to add a new module state. Why is it necessary and
> >> how does it fit with the existing states?
> >>
> > because once notifier fails in state MODULE_STATE_UNFORMED (now only ftrace has someting to do in this state), notifier chain will roll back by calling blocking_notifier_call_chain_robust, i'm afraid MODULE_STATE_GOING is going to jeopardise the notifers which don't handle it appropriately, like:
> >
> > case MODULE_STATE_COMING:
> > kmalloc();
> > case MODULE_STATE_GOING:
> > kfree();
>
> My understanding is that the current module "state machine" operates as
> follows. Transitions marked with an asterisk (*) are announced via the
> module notifier.
>
> ---> UNFORMED --*> COMING --*> LIVE --*> GOING -.
> ^ | ^ |
> | '---------------------* |
> '---------------------------------------'
>
> The new code aims to replace the current ftrace_module_init() call in
> load_module(). To achieve this, it adds a notification for the UNFORMED
> state (only when loading a module) and introduces a new FORMED state for
> rollback. FORMED is purely a fake state because it never appears in
> module::state. The new structure is as follows:
>
> ,--*> (FORMED)
> |
> --*> UNFORMED --*> COMING --*> LIVE --*> GOING -.
> ^ | ^ |
> | '---------------------* |
> '---------------------------------------'
>
> I'm afraid this is quite complex and inconsistent. Unless it can be kept
> simple, we would be just replacing one special handling with a different
> complexity, which is not worth it.
> >>
> >>> + if (err)
> >>> + goto ddebug_cleanup;
> >>> /* Finally it's fully formed, ready to start executing. */
> >>> err = complete_formation(mod, info);
> >>> - if (err)
> >>> + if (err) {
> >>> + blocking_notifier_call_chain_reverse(&module_notify_list,
> >>> + MODULE_STATE_FORMED, mod);
> >>> goto ddebug_cleanup;
> >>> + }
> >>> - err = prepare_coming_module(mod);
> >>> + err = prepare_module_state_transaction(mod,
> >>> + MODULE_STATE_COMING, MODULE_STATE_GOING);
> >>> if (err)
> >>> goto bug_cleanup;
> >>> @@ -3522,7 +3519,6 @@ static int load_module(struct load_info *info, const char __user *uargs,
> >>> destroy_params(mod->kp, mod->num_kp);
> >>> blocking_notifier_call_chain(&module_notify_list,
> >>> MODULE_STATE_GOING, mod);
> >>
> >> My understanding is that all notifier chains for MODULE_STATE_GOING
> >> should be reversed.
> > yes, all, from lowest priority notifier to highest.
> > I will resend patch 1 which was failed due to my proxy setting.
>
> What I meant here is that the call:
>
> blocking_notifier_call_chain(&module_notify_list, MODULE_STATE_GOING, mod);
>
> should be replaced with:
>
> blocking_notifier_call_chain_reverse(&module_notify_list, MODULE_STATE_GOING, mod);
>
> >
> >>
> >>> - klp_module_going(mod);
> >>> bug_cleanup:
> >>> mod->state = MODULE_STATE_GOING;
> >>> /* module_bug_cleanup needs module_mutex protection */
> >>
> >> The patch removes the klp_module_going() cleanup call in load_module().
> >> Similarly, the ftrace_release_mod() call under the ddebug_cleanup label
> >> should be removed and appropriately replaced with a cleanup via
> >> a notifier.
> >>
> > err = prepare_module_state_transaction(mod,
> > MODULE_STATE_UNFORMED, MODULE_STATE_FORMED);
> > if (err)
> > goto ddebug_cleanup;
> >
> > ftrace will be cleanup in blocking_notifier_call_chain_robust rolling back.
> >
> > err = prepare_module_state_transaction(mod,
> > MODULE_STATE_COMING, MODULE_STATE_GOING);
> >
> > each notifier including ftrace and klp will be cleanup in blocking_notifier_call_chain_robust rolling back.
> >
> > if all notifiers are successful in MODULE_STATE_COMING, they all will be clean up in
> > coming_cleanup:
> > mod->state = MODULE_STATE_GOING;
> > destroy_params(mod->kp, mod->num_kp);
> > blocking_notifier_call_chain(&module_notify_list,
> > MODULE_STATE_GOING, mod);
> >
> > if something wrong underneath.
>
> My point is that the patch leaves a call to ftrace_release_mod() in
> load_module(), which I expected to be handled via a notifier.
I think that I have got it. The ftrace code needs two notifiers when
the module is being loaded and two when it is going.
This is why Sond added the new state. But I think that we would
need two new states to call:
+ ftrace_module_init() in MODULE_STATE_UNFORMED
+ ftrace_module_enable() in MODULE_STATE_FORMED
and
+ ftrace_free_mem() in MODULE_STATE_PRE_GOING
+ ftrace_free_mem() in MODULE_STATE_GOING
By using the ascii art:
-*> UNFORMED -*> FORMED -> COMING -*> LIVE -*> PRE_GOING -*> GOING -.
| | | ^ ^ ^
| | '----------------' | |
| '--------------------------------------' |
'------------------------------------------------------'
But I think that this is not worth it.
Best Regards,
Petr
^ permalink raw reply
* Re: [RFC PATCH 2/2] kernel/module: Decouple klp and ftrace from load_module
From: Petr Mladek @ 2026-04-16 13:09 UTC (permalink / raw)
To: Song Chen
Cc: Petr Pavlu, rafael, lenb, mturquette, sboyd, viresh.kumar, agk,
snitzer, mpatocka, bmarzins, song, yukuai, linan122, jason.wessel,
danielt, dianders, horms, davem, edumazet, kuba, pabeni, paulmck,
frederic, mcgrof, da.gomez, samitolvanen, atomlin, jpoimboe,
jikos, mbenes, joe.lawrence, rostedt, mhiramat, mark.rutland,
mathieu.desnoyers, linux-modules, linux-kernel,
linux-trace-kernel, linux-acpi, linux-clk, linux-pm,
live-patching, dm-devel, linux-raid, kgdb-bugreport, netdev
In-Reply-To: <a35f5f94-7d5a-4347-974b-b270c89ef241@189.cn>
On Wed 2026-04-15 14:43:53, Song Chen wrote:
> Hi,
>
> On 4/14/26 22:33, Petr Pavlu wrote:
> > On 4/13/26 10:07 AM, chensong_2000@189.cn wrote:
> > > From: Song Chen <chensong_2000@189.cn>
> > >
> > > ftrace and livepatch currently have their module load/unload callbacks
> > > hard-coded in the module loader as direct function calls to
> > > ftrace_module_enable(), klp_module_coming(), klp_module_going()
> > > and ftrace_release_mod(). This tight coupling was originally introduced
> > > to enforce strict call ordering that could not be guaranteed by the
> > > module notifier chain, which only supported forward traversal. Their
> > > notifiers were moved in and out back and forth. see [1] and [2].
> >
> > I'm unclear about what is meant by the notifiers being moved back and
> > forth. The links point to patches that converted ftrace+klp from using
> > module notifiers to explicit callbacks due to ordering issues, but this
> > switch occurred only once. Have there been other attempts to use
> > notifiers again?
> >
> > > diff --git a/include/linux/module.h b/include/linux/module.h
> > > index 14f391b186c6..0bdd56f9defd 100644
> > > --- a/include/linux/module.h
> > > +++ b/include/linux/module.h
> > > @@ -308,6 +308,14 @@ enum module_state {
> > > MODULE_STATE_COMING, /* Full formed, running module_init. */
> > > MODULE_STATE_GOING, /* Going away. */
> > > MODULE_STATE_UNFORMED, /* Still setting it up. */
> > > + MODULE_STATE_FORMED,
> >
> > I don't see a reason to add a new module state. Why is it necessary and
> > how does it fit with the existing states?
> >
> because once notifier fails in state MODULE_STATE_UNFORMED (now only ftrace
> has someting to do in this state), notifier chain will roll back by calling
> blocking_notifier_call_chain_robust, i'm afraid MODULE_STATE_GOING is going
> to jeopardise the notifers which don't handle it appropriately, like:
>
> case MODULE_STATE_COMING:
> kmalloc();
> case MODULE_STATE_GOING:
> kfree();
>
>
> > > +};
> > > +
> > > +enum module_notifier_prio {
> > > + MODULE_NOTIFIER_PRIO_LOW = INT_MIN, /* Low prioroty, coming last, going first */
> > > + MODULE_NOTIFIER_PRIO_MID = 0, /* Normal priority. */
> > > + MODULE_NOTIFIER_PRIO_SECOND_HIGH = INT_MAX - 1, /* Second high priorigy, coming second*/
> > > + MODULE_NOTIFIER_PRIO_HIGH = INT_MAX, /* High priorigy, coming first, going late. */
> >
> > I suggest being explicit about how the notifiers are ordered. For
> > example:
> >
> > enum module_notifier_prio {
> > MODULE_NOTIFIER_PRIO_NORMAL, /* Normal priority, coming last, going first. */
> > MODULE_NOTIFIER_PRIO_LIVEPATCH,
> > MODULE_NOTIFIER_PRIO_FTRACE, /* High priority, coming first, going late. */
> > };
> >
I like the explicit PRIO_LIVEPATCH/FTRACE names.
But I would keep the INT_MAX - 1 and INT_MAX priorities. I believe
that ftrace/livepatching will always be the first/last to call.
And INT_MAX would help to preserve kABI when PRIO_NORMAL is not
enough for the rest of notifiers.
That said, I am not sure whether this is worth the effort.
This patch tries to move the explicit callbacks in a generic
notifiers API. But it will still need to use some explictly
defined (reserved) priorities. And it will
not guarantee a misuse. Also it requires the double linked
list which complicates the notifiers code.
> > > };
> > > struct mod_tree_node {
> > > --- a/kernel/module/main.c
> > > +++ b/kernel/module/main.c
> > > @@ -3281,20 +3277,14 @@ static int complete_formation(struct module *mod, struct load_info *info)
> > > return err;
> > > }
> > > -static int prepare_coming_module(struct module *mod)
> > > +static int prepare_module_state_transaction(struct module *mod,
> > > + unsigned long val_up, unsigned long val_down)
> > > {
> > > int err;
> > > - ftrace_module_enable(mod);
> > > - err = klp_module_coming(mod);
> > > - if (err)
> > > - return err;
> > > -
> > > err = blocking_notifier_call_chain_robust(&module_notify_list,
> > > - MODULE_STATE_COMING, MODULE_STATE_GOING, mod);
> > > + val_up, val_down, mod);
> > > err = notifier_to_errno(err);
> > > - if (err)
> > > - klp_module_going(mod);
> > > return err;
> > > }
I personally find the name "prepare_module_state_transaction"
misleading. What is the "transaction" here? If this was a "preparation"
step then where is the transaction done/finished?
It might be better to just opencode the
blocking_notifier_call_chain_robust() instead.
> > > @@ -3468,14 +3458,21 @@ static int load_module(struct load_info *info, const char __user *uargs,
> > > init_build_id(mod, info);
> > > /* Ftrace init must be called in the MODULE_STATE_UNFORMED state */
> > > - ftrace_module_init(mod);
> > > + err = prepare_module_state_transaction(mod,
> > > + MODULE_STATE_UNFORMED, MODULE_STATE_FORMED);
> >
> > I believe val_down should be MODULE_STATE_GOING to reverse the
> > operation. Why is the new state MODULE_STATE_FORMED needed here?
> to avoid this:
>
> case MODULE_STATE_COMING:
> kmalloc();
> case MODULE_STATE_GOING:
> kfree();
Hmm, the module is in "FORMED" state here.
> > > + if (err)
> > > + goto ddebug_cleanup;
> > > /* Finally it's fully formed, ready to start executing. */
> > > err = complete_formation(mod, info);
And we call "complete_formation()" function. This sounds like
it was not really "FORMED" before. => It is confusing and nono.
Please, try to avoid the new state if possible. My experience
with reading the module loader code is that any new state
brings a lot of complexity. You need to take it into account
when checking correctness of other changes, features, ...
Something tells me that if the state was not needed before
then we could avoid it.
> > > - if (err)
> > > + if (err) {
> > > + blocking_notifier_call_chain_reverse(&module_notify_list,
> > > + MODULE_STATE_FORMED, mod);
> > > goto ddebug_cleanup;
> > > + }
> > > - err = prepare_coming_module(mod);
> > > + err = prepare_module_state_transaction(mod,
> > > + MODULE_STATE_COMING, MODULE_STATE_GOING);
> > > if (err)
> > > goto bug_cleanup;
> > > --- a/kernel/trace/ftrace.c
> > > +++ b/kernel/trace/ftrace.c
> > > @@ -5241,6 +5241,44 @@ static int __init ftrace_mod_cmd_init(void)
> > > }
> > > core_initcall(ftrace_mod_cmd_init);
> > > +static int ftrace_module_callback(struct notifier_block *nb, unsigned long op,
> > > + void *module)
> > > +{
> > > + struct module *mod = module;
> > > +
> > > + switch (op) {
> > > + case MODULE_STATE_UNFORMED:
> > > + ftrace_module_init(mod);
> > > + break;
> > > + case MODULE_STATE_COMING:
> > > + ftrace_module_enable(mod);
> > > + break;
> > > + case MODULE_STATE_LIVE:
> > > + ftrace_free_mem(mod, mod->mem[MOD_INIT_TEXT].base,
> > > + mod->mem[MOD_INIT_TEXT].base + mod->mem[MOD_INIT_TEXT].size);
> > > + break;
> > > + case MODULE_STATE_GOING:
> > > + case MODULE_STATE_FORMED:
> > > + ftrace_release_mod(mod);
This calls "release" in a "FORMED" state. It does not make any
sense. Something looks fishy, either the code or the naming.
> > > + break;
> > > + default:
> > > + break;
> > > + }
> >
I am sorry for being so picky about names. I believe that good names
help to prevent bugs and reduce headaches.
Best Regards,
Petr
^ permalink raw reply
* Re: [PATCH 01/61] Coccinelle: Prefer IS_ERR_OR_NULL over manual NULL check
From: Krzysztof Kozlowski @ 2026-04-16 12:30 UTC (permalink / raw)
To: Philipp Hahn, amd-gfx, apparmor, bpf, ceph-devel, cocci, dm-devel,
dri-devel, gfs2, intel-gfx, intel-wired-lan, iommu, kvm,
linux-arm-kernel, linux-block, linux-bluetooth, linux-btrfs,
linux-cifs, linux-clk, linux-erofs, linux-ext4, linux-fsdevel,
linux-gpio, linux-hyperv, linux-input, linux-kernel, linux-leds,
linux-media, linux-mips, linux-mm, linux-modules, linux-mtd,
linux-nfs, linux-omap, linux-phy, linux-pm, linux-rockchip,
linux-s390, linux-scsi, linux-sctp, linux-security-module,
linux-sh, linux-sound, linux-stm32, linux-trace-kernel, linux-usb,
linux-wireless, netdev, ntfs3, samba-technical, sched-ext,
target-devel, tipc-discussion, v9fs
Cc: Julia Lawall, Nicolas Palix
In-Reply-To: <20260310-b4-is_err_or_null-v1-1-bd63b656022d@avm.de>
On 10/03/2026 12:48, Philipp Hahn wrote:
> Find and convert uses of IS_ERR() plus NULL check to IS_ERR_OR_NULL().
>
> There are several cases where `!ptr && WARN_ON[_ONCE](IS_ERR(ptr))` is
> used:
> - arch/x86/kernel/callthunks.c:215 WARN_ON_ONCE
> - drivers/clk/clk.c:4561 WARN_ON_ONCE
> - drivers/interconnect/core.c:793 WARN_ON
> - drivers/reset/core.c:718 WARN_ON
> The change is not 100% semantical equivalent as the warning will now
> also happen when the pointer is NULL.
>
> To: Julia Lawall <Julia.Lawall@inria.fr>
> To: Nicolas Palix <nicolas.palix@imag.fr>
> Cc: cocci@inria.fr
> Cc: linux-kernel@vger.kernel.org
>
> ---
> drivers/clocksource/mips-gic-timer.c:283 looks suspicious: ret != clk,
> but Daniel Lezcano verified it as cottect.
>
> There are some cases where the checks are part of a larger expression:
> - mm/kmemleak.c:1095
> - mm/kmemleak.c:1155
> - mm/kmemleak.c:1173
> - mm/kmemleak.c:1290
> - mm/kmemleak.c:1328
> - mm/kmemleak.c:1241
> - mm/kmemleak.c:1310
> - mm/kmemleak.c:1258
> - net/netlink/af_netlink.c:2670
> Thanks to Julia Lawall for the help to also handle them.
>
> Signed-off-by: Philipp Hahn <phahn-oss@avm.de>
> ---
> scripts/coccinelle/api/is_err_or_null.cocci | 125 ++++++++++++++++++++++++++++
> 1 file changed, 125 insertions(+)
>
Neither this, nor try from 2011, nor any future try should be accepted,
because it creates impression IS_ERR_OR_NULL is somehow okay. No, it is
not okay, it is a discouraged pattern leading to less readable and
maintainable code. We should not have therefore any tools suggesting
usage of IS_ERR_OR_NULL, because people will be converting poor code
into that, instead of fixing that poor code.
Best regards,
Krzysztof
^ permalink raw reply
* Re: [RFC PATCH 1/2] kernel/notifier: replace single-linked list with double-linked list for reverse traversal
From: David Laight @ 2026-04-16 12:30 UTC (permalink / raw)
To: chensong_2000
Cc: rafael, lenb, mturquette, sboyd, viresh.kumar, agk, snitzer,
mpatocka, bmarzins, song, yukuai, linan122, jason.wessel, danielt,
dianders, horms, davem, edumazet, kuba, pabeni, paulmck, frederic,
mcgrof, petr.pavlu, da.gomez, samitolvanen, atomlin, jpoimboe,
jikos, mbenes, pmladek, joe.lawrence, rostedt, mhiramat,
mark.rutland, mathieu.desnoyers, linux-modules, linux-kernel,
linux-trace-kernel, linux-acpi, linux-clk, linux-pm,
live-patching, dm-devel, linux-raid, kgdb-bugreport, netdev
In-Reply-To: <20260415070137.17860-1-chensong_2000@189.cn>
On Wed, 15 Apr 2026 15:01:37 +0800
chensong_2000@189.cn wrote:
> From: Song Chen <chensong_2000@189.cn>
>
> The current notifier chain implementation uses a single-linked list
> (struct notifier_block *next), which only supports forward traversal
> in priority order. This makes it difficult to handle cleanup/teardown
> scenarios that require notifiers to be called in reverse priority order.
If it is only cleanup/teardown then the list can be order-reversed
as part of that process at the same time as the list is deleted.
David
^ permalink raw reply
* Re: [PATCH 55/61] interconnect: Prefer IS_ERR_OR_NULL over manual NULL check
From: Krzysztof Kozlowski @ 2026-04-16 12:24 UTC (permalink / raw)
To: Philipp Hahn, amd-gfx, apparmor, bpf, ceph-devel, cocci, dm-devel,
dri-devel, gfs2, intel-gfx, intel-wired-lan, iommu, kvm,
linux-arm-kernel, linux-block, linux-bluetooth, linux-btrfs,
linux-cifs, linux-clk, linux-erofs, linux-ext4, linux-fsdevel,
linux-gpio, linux-hyperv, linux-input, linux-kernel, linux-leds,
linux-media, linux-mips, linux-mm, linux-modules, linux-mtd,
linux-nfs, linux-omap, linux-phy, linux-pm, linux-rockchip,
linux-s390, linux-scsi, linux-sctp, linux-security-module,
linux-sh, linux-sound, linux-stm32, linux-trace-kernel, linux-usb,
linux-wireless, netdev, ntfs3, samba-technical, sched-ext,
target-devel, tipc-discussion, v9fs
Cc: Georgi Djakov
In-Reply-To: <20260310-b4-is_err_or_null-v1-55-bd63b656022d@avm.de>
On 10/03/2026 12:49, Philipp Hahn wrote:
> Prefer using IS_ERR_OR_NULL() over using IS_ERR() and a manual NULL
> check.
>
> Semantich change: Previously the code only printed the warning on error,
> but not when the pointer was NULL. Now the warning is printed in both
> cases!
NAK, read the code
>
> Change found with coccinelle.
>
> To: Georgi Djakov <djakov@kernel.org>
> Cc: linux-pm@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Philipp Hahn <phahn-oss@avm.de>
> ---
> drivers/interconnect/core.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/interconnect/core.c b/drivers/interconnect/core.c
> index 8569b78a18517b33abeafac091978b25cbc1acc7..22e92b30f73853d5bd2e05b4f52cb5aa22556468 100644
> --- a/drivers/interconnect/core.c
> +++ b/drivers/interconnect/core.c
> @@ -790,7 +790,7 @@ void icc_put(struct icc_path *path)
> size_t i;
> int ret;
>
> - if (!path || WARN_ON(IS_ERR(path)))
> + if (WARN_ON(IS_ERR_OR_NULL(path)))
IS_ERR_OR_NULL is simply discouraged, but beside of code preference, you
just added bug here. This is clearly not equivalent and you emit warn on
perfectly valid case!
Best regards,
Krzysztof
^ permalink raw reply
* Re: [RFC PATCH 2/2] kernel/module: Decouple klp and ftrace from load_module
From: Petr Pavlu @ 2026-04-16 11:18 UTC (permalink / raw)
To: Song Chen
Cc: rafael, lenb, mturquette, sboyd, viresh.kumar, agk, snitzer,
mpatocka, bmarzins, song, yukuai, linan122, jason.wessel, danielt,
dianders, horms, davem, edumazet, kuba, pabeni, paulmck, frederic,
mcgrof, da.gomez, samitolvanen, atomlin, jpoimboe, jikos, mbenes,
pmladek, joe.lawrence, rostedt, mhiramat, mark.rutland,
mathieu.desnoyers, linux-modules, linux-kernel,
linux-trace-kernel, linux-acpi, linux-clk, linux-pm,
live-patching, dm-devel, linux-raid, kgdb-bugreport, netdev
In-Reply-To: <a35f5f94-7d5a-4347-974b-b270c89ef241@189.cn>
On 4/15/26 8:43 AM, Song Chen wrote:
> On 4/14/26 22:33, Petr Pavlu wrote:
>> On 4/13/26 10:07 AM, chensong_2000@189.cn wrote:
>>> diff --git a/include/linux/module.h b/include/linux/module.h
>>> index 14f391b186c6..0bdd56f9defd 100644
>>> --- a/include/linux/module.h
>>> +++ b/include/linux/module.h
>>> @@ -308,6 +308,14 @@ enum module_state {
>>> MODULE_STATE_COMING, /* Full formed, running module_init. */
>>> MODULE_STATE_GOING, /* Going away. */
>>> MODULE_STATE_UNFORMED, /* Still setting it up. */
>>> + MODULE_STATE_FORMED,
>>
>> I don't see a reason to add a new module state. Why is it necessary and
>> how does it fit with the existing states?
>>
> because once notifier fails in state MODULE_STATE_UNFORMED (now only ftrace has someting to do in this state), notifier chain will roll back by calling blocking_notifier_call_chain_robust, i'm afraid MODULE_STATE_GOING is going to jeopardise the notifers which don't handle it appropriately, like:
>
> case MODULE_STATE_COMING:
> kmalloc();
> case MODULE_STATE_GOING:
> kfree();
My understanding is that the current module "state machine" operates as
follows. Transitions marked with an asterisk (*) are announced via the
module notifier.
---> UNFORMED --*> COMING --*> LIVE --*> GOING -.
^ | ^ |
| '---------------------* |
'---------------------------------------'
The new code aims to replace the current ftrace_module_init() call in
load_module(). To achieve this, it adds a notification for the UNFORMED
state (only when loading a module) and introduces a new FORMED state for
rollback. FORMED is purely a fake state because it never appears in
module::state. The new structure is as follows:
,--*> (FORMED)
|
--*> UNFORMED --*> COMING --*> LIVE --*> GOING -.
^ | ^ |
| '---------------------* |
'---------------------------------------'
I'm afraid this is quite complex and inconsistent. Unless it can be kept
simple, we would be just replacing one special handling with a different
complexity, which is not worth it.
>>
>>> + if (err)
>>> + goto ddebug_cleanup;
>>> /* Finally it's fully formed, ready to start executing. */
>>> err = complete_formation(mod, info);
>>> - if (err)
>>> + if (err) {
>>> + blocking_notifier_call_chain_reverse(&module_notify_list,
>>> + MODULE_STATE_FORMED, mod);
>>> goto ddebug_cleanup;
>>> + }
>>> - err = prepare_coming_module(mod);
>>> + err = prepare_module_state_transaction(mod,
>>> + MODULE_STATE_COMING, MODULE_STATE_GOING);
>>> if (err)
>>> goto bug_cleanup;
>>> @@ -3522,7 +3519,6 @@ static int load_module(struct load_info *info, const char __user *uargs,
>>> destroy_params(mod->kp, mod->num_kp);
>>> blocking_notifier_call_chain(&module_notify_list,
>>> MODULE_STATE_GOING, mod);
>>
>> My understanding is that all notifier chains for MODULE_STATE_GOING
>> should be reversed.
> yes, all, from lowest priority notifier to highest.
> I will resend patch 1 which was failed due to my proxy setting.
What I meant here is that the call:
blocking_notifier_call_chain(&module_notify_list, MODULE_STATE_GOING, mod);
should be replaced with:
blocking_notifier_call_chain_reverse(&module_notify_list, MODULE_STATE_GOING, mod);
>
>>
>>> - klp_module_going(mod);
>>> bug_cleanup:
>>> mod->state = MODULE_STATE_GOING;
>>> /* module_bug_cleanup needs module_mutex protection */
>>
>> The patch removes the klp_module_going() cleanup call in load_module().
>> Similarly, the ftrace_release_mod() call under the ddebug_cleanup label
>> should be removed and appropriately replaced with a cleanup via
>> a notifier.
>>
> err = prepare_module_state_transaction(mod,
> MODULE_STATE_UNFORMED, MODULE_STATE_FORMED);
> if (err)
> goto ddebug_cleanup;
>
> ftrace will be cleanup in blocking_notifier_call_chain_robust rolling back.
>
> err = prepare_module_state_transaction(mod,
> MODULE_STATE_COMING, MODULE_STATE_GOING);
>
> each notifier including ftrace and klp will be cleanup in blocking_notifier_call_chain_robust rolling back.
>
> if all notifiers are successful in MODULE_STATE_COMING, they all will be clean up in
> coming_cleanup:
> mod->state = MODULE_STATE_GOING;
> destroy_params(mod->kp, mod->num_kp);
> blocking_notifier_call_chain(&module_notify_list,
> MODULE_STATE_GOING, mod);
>
> if something wrong underneath.
My point is that the patch leaves a call to ftrace_release_mod() in
load_module(), which I expected to be handled via a notifier.
--
Thanks,
Petr
^ permalink raw reply
* Re: [RFC PATCH 1/2] kernel/notifier: replace single-linked list with double-linked list for reverse traversal
From: Petr Mladek @ 2026-04-16 10:33 UTC (permalink / raw)
To: chensong_2000
Cc: rafael, lenb, mturquette, sboyd, viresh.kumar, agk, snitzer,
mpatocka, bmarzins, song, yukuai, linan122, jason.wessel, danielt,
dianders, horms, davem, edumazet, kuba, pabeni, paulmck, frederic,
mcgrof, petr.pavlu, da.gomez, samitolvanen, atomlin, jpoimboe,
jikos, mbenes, joe.lawrence, rostedt, mhiramat, mark.rutland,
mathieu.desnoyers, linux-modules, linux-kernel,
linux-trace-kernel, linux-acpi, linux-clk, linux-pm,
live-patching, dm-devel, linux-raid, kgdb-bugreport, netdev
In-Reply-To: <20260415070137.17860-1-chensong_2000@189.cn>
On Wed 2026-04-15 15:01:37, chensong_2000@189.cn wrote:
> From: Song Chen <chensong_2000@189.cn>
>
> The current notifier chain implementation uses a single-linked list
> (struct notifier_block *next), which only supports forward traversal
> in priority order. This makes it difficult to handle cleanup/teardown
> scenarios that require notifiers to be called in reverse priority order.
>
> A concrete example is the ordering dependency between ftrace and
> livepatch during module load/unload. see the detail here [1].
>
> This patch replaces the single-linked list in struct notifier_block
> with a struct list_head, converting the notifier chain into a
> doubly-linked list sorted in descending priority order. Based on
> this, a new function notifier_call_chain_reverse() is introduced,
> which traverses the chain in reverse (ascending priority order).
> The corresponding blocking_notifier_call_chain_reverse() is also
> added as the locking wrapper for blocking notifier chains.
>
> The internal notifier_call_chain_robust() is updated to use
> notifier_call_chain_reverse() for rollback: on error, it records
> the failing notifier (last_nb) and the count of successfully called
> notifiers (nr), then rolls back exactly those nr-1 notifiers in
> reverse order starting from last_nb's predecessor, without needing
> to know the total length of the chain.
>
> With this change, subsystems with symmetric setup/teardown ordering
> requirements can register a single notifier_block with one priority
> value, and rely on blocking_notifier_call_chain() for forward
> traversal and blocking_notifier_call_chain_reverse() for reverse
> traversal, without needing hard-coded call sequences or separate
> notifier registrations for each direction.
>
> [1]:https://lore.kernel.org/all
> /alpine.LNX.2.00.1602172216491.22700@cbobk.fhfr.pm/
>
> --- a/include/linux/notifier.h
> +++ b/include/linux/notifier.h
> @@ -53,41 +53,41 @@ typedef int (*notifier_fn_t)(struct notifier_block *nb,
[...]
> struct notifier_block {
> notifier_fn_t notifier_call;
> - struct notifier_block __rcu *next;
> + struct list_head __rcu entry;
> int priority;
> };
[...]
> #define ATOMIC_INIT_NOTIFIER_HEAD(name) do { \
> spin_lock_init(&(name)->lock); \
> - (name)->head = NULL; \
> + INIT_LIST_HEAD(&(name)->head); \
I would expect the RCU variant here, aka INIT_LIST_HEAD_RCU().
> --- a/kernel/notifier.c
> +++ b/kernel/notifier.c
> @@ -14,39 +14,47 @@
> * are layered on top of these, with appropriate locking added.
> */
>
> -static int notifier_chain_register(struct notifier_block **nl,
> +static int notifier_chain_register(struct list_head *nl,
> struct notifier_block *n,
> bool unique_priority)
> {
> - while ((*nl) != NULL) {
> - if (unlikely((*nl) == n)) {
> + struct notifier_block *cur;
> +
> + list_for_each_entry(cur, nl, entry) {
> + if (unlikely(cur == n)) {
> WARN(1, "notifier callback %ps already registered",
> n->notifier_call);
> return -EEXIST;
> }
> - if (n->priority > (*nl)->priority)
> - break;
> - if (n->priority == (*nl)->priority && unique_priority)
> +
> + if (n->priority == cur->priority && unique_priority)
> return -EBUSY;
> - nl = &((*nl)->next);
> +
> + if (n->priority > cur->priority) {
> + list_add_tail(&n->entry, &cur->entry);
> + goto out;
> + }
> }
> - n->next = *nl;
> - rcu_assign_pointer(*nl, n);
> +
> + list_add_tail(&n->entry, nl);
I would expect list_add_tail_rcu() here.
> @@ -59,25 +67,25 @@ static int notifier_chain_unregister(struct notifier_block **nl,
> * value of this parameter is -1.
> * @nr_calls: Records the number of notifications sent. Don't care
> * value of this field is NULL.
> + * @last_nb: Records the last called notifier block for rolling back
> * Return: notifier_call_chain returns the value returned by the
> * last notifier function called.
> */
> -static int notifier_call_chain(struct notifier_block **nl,
> +static int notifier_call_chain(struct list_head *nl,
> unsigned long val, void *v,
> - int nr_to_call, int *nr_calls)
> + int nr_to_call, int *nr_calls,
> + struct notifier_block **last_nb)
> {
> int ret = NOTIFY_DONE;
> - struct notifier_block *nb, *next_nb;
> -
> - nb = rcu_dereference_raw(*nl);
> + struct notifier_block *nb;
>
> - while (nb && nr_to_call) {
> - next_nb = rcu_dereference_raw(nb->next);
> + if (!nr_to_call)
> + return ret;
>
> + list_for_each_entry(nb, nl, entry) {
I would expect the RCU variant here, aka list_for_each_rcu()
These are just two random examples which I found by a quick look.
I guess that the notifier API is very old and it does not use all
the RCU API features which allow to track safety when
CONFIG_PROVE_RCU and CONFIG_PROVE_RCU_LIST are enabled.
It actually might be worth to audit the code and make it right.
> #ifdef CONFIG_DEBUG_NOTIFIERS
> if (unlikely(!func_ptr_is_kernel_text(nb->notifier_call))) {
> WARN(1, "Invalid notifier called!");
> - nb = next_nb;
> continue;
> }
> #endif
That said, I am not sure if the ftrace/livepatching handlers are
the right motivation for this. Especially when I see the
complexity of the 2nd patch [*]
After thinking more about it. I am not even sure that the ftrace and
livepatching callbacks are good candidates for generic notifiers.
They are too special. It is not only about ordering them against
each other. But it is also about ordering them against other
notifiers. The ftrace/livepatching callbacks must be the first/last
during module load/release.
[*] The 2nd patch is not archived by lore for some reason.
I have found only a review but it gives a good picture, see
https://lore.kernel.org/all/1191caf5-6a61-4622-a15e-854d3701f4fc@suse.com/
Best Regards,
Petr
^ permalink raw reply
* Re: [PATCH] kbuild/btf: Remove broken module relinking exclusion
From: Ihor Solodrai @ 2026-04-15 15:47 UTC (permalink / raw)
To: Petr Pavlu, Nathan Chancellor, Nicolas Schier, Alexei Starovoitov,
Daniel Borkmann, Andrii Nakryiko
Cc: Martin KaFai Lau, Eduard Zingerman, Song Liu, Yonghong Song,
John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa,
Luis Chamberlain, Daniel Gomez, Sami Tolvanen, Aaron Tomlin,
Masahiro Yamada, Sasha Levin, linux-kbuild, bpf, linux-modules,
linux-kernel
In-Reply-To: <20260410131343.2519532-1-petr.pavlu@suse.com>
On 4/10/26 6:13 AM, Petr Pavlu wrote:
> Commit 5f9ae91f7c0d ("kbuild: Build kernel module BTFs if BTF is enabled
> and pahole supports it") in 2020 introduced CONFIG_DEBUG_INFO_BTF_MODULES
> to enable generation of split BTF for kernel modules. This change required
> the %.ko Makefile rule to additionally depend on vmlinux, which is used as
> a base for deduplication. The regular ld_ko_o command executed by the rule
> was then modified to be skipped if only vmlinux changes. This was done by
> introducing a new if_changed_except command and updating the original call
> to '+$(call if_changed_except,ld_ko_o,vmlinux)'.
>
> Later, commit 214c0eea43b2 ("kbuild: add $(objtree)/ prefix to some
> in-kernel build artifacts") in 2024 updated the rule's reference to vmlinux
> from 'vmlinux' to '$(objtree)/vmlinux'. This accidentally broke the
> previous logic to skip relinking modules if only vmlinux changes. The issue
> is that '$(objtree)' is typically '.' and GNU Make normalizes the resulting
> prerequisite './vmlinux' to just 'vmlinux', while the exclusion logic
> retains the raw './vmlinux'. As a result, if_changed_except doesn't
> correctly filter out vmlinux. Consequently, with
> CONFIG_DEBUG_INFO_BTF_MODULES=y, modules are relinked even if only vmlinux
> changes.
>
> It is possible to fix this Makefile issue. However, having the %.ko rule
> update the resulting file in place without starting from the original
> inputs is rather fragile. The logic is harder to debug if something breaks
> during a subsequent .ko update because the old input is lost due to the
> overwrite. Additionally, it requires that the BTF processing is idempotent.
> For example, sorting id+flags BTF_SET8 pairs in .BTF_ids by resolve_btfids
> currently doesn't have this property.
>
> One option is to split the %.ko target into two rules: the first for
> partial linking and the second one for generating the BTF data. However,
> this approach runs into an issue with requiring additional intermediate
> files, which increases the size of the build directory. On my system, when
> using a large distribution config with ~5500 modules, the size of the build
> directory with debuginfo enabled is already ~25 GB, with .ko files
> occupying ~8 GB. Duplicating these .ko files doesn't seem practical.
>
> Measuring the speed of the %.ko processing shows that the link step is
> actually relatively fast. It takes about 20% of the overall rule time,
> while the BTF processing accounts for 80%. Moreover, skipping the link part
> becomes relevant only during local development. In such cases, developers
> typically use configs that enable a limited number of modules, so having
> the %.ko rule slightly slower doesn't significantly impact the total
> rebuild time. This is supported by the fact that no one has complained
> about this optimization being broken for the past two years.
>
> Therefore, remove the logic that prevents module relinking when only
> vmlinux changes and simplify Makefile.modfinal.
The change makes sense to me.
Acked-by: Ihor Solodrai <ihor.solodrai@linux.dev>
Thank you!
>
> Signed-off-by: Petr Pavlu <petr.pavlu@suse.com>
> ---
> My previous attempt to fix this logic can be found at
> https://lore.kernel.org/linux-modules/20260402141911.1577711-1-petr.pavlu@suse.com/
> ---
> scripts/Makefile.modfinal | 10 +---------
> 1 file changed, 1 insertion(+), 9 deletions(-)
>
> diff --git a/scripts/Makefile.modfinal b/scripts/Makefile.modfinal
> index adcbcde16a07..01a37ec872b9 100644
> --- a/scripts/Makefile.modfinal
> +++ b/scripts/Makefile.modfinal
> @@ -46,17 +46,9 @@ quiet_cmd_btf_ko = BTF [M] $@
> $(CONFIG_SHELL) $(srctree)/scripts/gen-btf.sh --btf_base $(objtree)/vmlinux $@; \
> fi;
>
> -# Same as newer-prereqs, but allows to exclude specified extra dependencies
> -newer_prereqs_except = $(filter-out $(PHONY) $(1),$?)
> -
> -# Same as if_changed, but allows to exclude specified extra dependencies
> -if_changed_except = $(if $(call newer_prereqs_except,$(2))$(cmd-check), \
> - $(cmd); \
> - printf '%s\n' 'savedcmd_$@ := $(make-cmd)' > $(dot-target).cmd, @:)
> -
> # Re-generate module BTFs if either module's .ko or vmlinux changed
> %.ko: %.o %.mod.o .module-common.o $(objtree)/scripts/module.lds $(and $(CONFIG_DEBUG_INFO_BTF_MODULES),$(KBUILD_BUILTIN),$(objtree)/vmlinux) FORCE
> - +$(call if_changed_except,ld_ko_o,$(objtree)/vmlinux)
> + +$(call if_changed,ld_ko_o)
> ifdef CONFIG_DEBUG_INFO_BTF_MODULES
> +$(if $(newer-prereqs),$(call cmd,btf_ko))
> endif
>
> base-commit: 591cd656a1bf5ea94a222af5ef2ee76df029c1d2
^ permalink raw reply
* Re: [PATCH] kbuild/btf: Remove broken module relinking exclusion
From: Alan Maguire @ 2026-04-15 13:29 UTC (permalink / raw)
To: Alexei Starovoitov, Nathan Chancellor, Ihor Solodrai
Cc: Petr Pavlu, Nicolas Schier, Alexei Starovoitov, Daniel Borkmann,
Andrii Nakryiko, Martin KaFai Lau, Eduard Zingerman, Song Liu,
Yonghong Song, John Fastabend, KP Singh, Stanislav Fomichev,
Hao Luo, Jiri Olsa, Luis Chamberlain, Daniel Gomez, Sami Tolvanen,
Aaron Tomlin, Masahiro Yamada, Sasha Levin,
Linux Kbuild mailing list, bpf, linux-modules, LKML
In-Reply-To: <CAADnVQKnWftcW0gk8HgFPLXuBf81+mqHMMds3fRmXHgOrRE-1g@mail.gmail.com>
On 15/04/2026 06:14, Alexei Starovoitov wrote:
> On Tue, Apr 14, 2026 at 1:34 PM Nathan Chancellor <nathan@kernel.org> wrote:
>>
>> On Fri, Apr 10, 2026 at 03:13:29PM +0200, Petr Pavlu wrote:
>>> Commit 5f9ae91f7c0d ("kbuild: Build kernel module BTFs if BTF is enabled
>>> and pahole supports it") in 2020 introduced CONFIG_DEBUG_INFO_BTF_MODULES
>>> to enable generation of split BTF for kernel modules. This change required
>>> the %.ko Makefile rule to additionally depend on vmlinux, which is used as
>>> a base for deduplication. The regular ld_ko_o command executed by the rule
>>> was then modified to be skipped if only vmlinux changes. This was done by
>>> introducing a new if_changed_except command and updating the original call
>>> to '+$(call if_changed_except,ld_ko_o,vmlinux)'.
>>>
>>> Later, commit 214c0eea43b2 ("kbuild: add $(objtree)/ prefix to some
>>> in-kernel build artifacts") in 2024 updated the rule's reference to vmlinux
>>> from 'vmlinux' to '$(objtree)/vmlinux'. This accidentally broke the
>>> previous logic to skip relinking modules if only vmlinux changes. The issue
>>> is that '$(objtree)' is typically '.' and GNU Make normalizes the resulting
>>> prerequisite './vmlinux' to just 'vmlinux', while the exclusion logic
>>> retains the raw './vmlinux'. As a result, if_changed_except doesn't
>>> correctly filter out vmlinux. Consequently, with
>>> CONFIG_DEBUG_INFO_BTF_MODULES=y, modules are relinked even if only vmlinux
>>> changes.
>>>
>>> It is possible to fix this Makefile issue. However, having the %.ko rule
>>> update the resulting file in place without starting from the original
>>> inputs is rather fragile. The logic is harder to debug if something breaks
>>> during a subsequent .ko update because the old input is lost due to the
>>> overwrite. Additionally, it requires that the BTF processing is idempotent.
>>> For example, sorting id+flags BTF_SET8 pairs in .BTF_ids by resolve_btfids
>>> currently doesn't have this property.
>>>
>>> One option is to split the %.ko target into two rules: the first for
>>> partial linking and the second one for generating the BTF data. However,
>>> this approach runs into an issue with requiring additional intermediate
>>> files, which increases the size of the build directory. On my system, when
>>> using a large distribution config with ~5500 modules, the size of the build
>>> directory with debuginfo enabled is already ~25 GB, with .ko files
>>> occupying ~8 GB. Duplicating these .ko files doesn't seem practical.
>>>
>>> Measuring the speed of the %.ko processing shows that the link step is
>>> actually relatively fast. It takes about 20% of the overall rule time,
>>> while the BTF processing accounts for 80%. Moreover, skipping the link part
>>> becomes relevant only during local development. In such cases, developers
>>> typically use configs that enable a limited number of modules, so having
>>> the %.ko rule slightly slower doesn't significantly impact the total
>>> rebuild time. This is supported by the fact that no one has complained
>>> about this optimization being broken for the past two years.
>>>
>>> Therefore, remove the logic that prevents module relinking when only
>>> vmlinux changes and simplify Makefile.modfinal.
>>>
>>> Signed-off-by: Petr Pavlu <petr.pavlu@suse.com>
>>
>> If the BPF folks want to take this since it deals with BTF:
>>
>> Acked-by: Nathan Chancellor <nathan@kernel.org>
>>
>> Otherwise, either Nicolas can take this for 7.1 or I will pick it up for
>> 7.2 when 7.1-rc1 is out.
>
> Alan, Ihor,
>
> As resident btf gen experts, Please take a look.
Tried it out with full build, rebuild, module-only (M=) build etc. All work
well so feel free to add
Reviewed-by: Alan Maguire <alan.maguire@oracle.com>
Tested-by: Alan Maguire <alan.maguire@oracle.com>
^ permalink raw reply
* Re: [RFC PATCH 1/2] kernel/notifier: replace single-linked list with double-linked list for reverse traversal
From: Christoph Hellwig @ 2026-04-15 7:40 UTC (permalink / raw)
To: chensong_2000
Cc: rafael, lenb, mturquette, sboyd, viresh.kumar, agk, snitzer,
mpatocka, bmarzins, song, yukuai, linan122, jason.wessel, danielt,
dianders, horms, davem, edumazet, kuba, pabeni, paulmck, frederic,
mcgrof, petr.pavlu, da.gomez, samitolvanen, atomlin, jpoimboe,
jikos, mbenes, pmladek, joe.lawrence, rostedt, mhiramat,
mark.rutland, mathieu.desnoyers, linux-modules, linux-kernel,
linux-trace-kernel, linux-acpi, linux-clk, linux-pm,
live-patching, dm-devel, linux-raid, kgdb-bugreport, netdev
In-Reply-To: <20260415070137.17860-1-chensong_2000@189.cn>
On Wed, Apr 15, 2026 at 03:01:37PM +0800, chensong_2000@189.cn wrote:
> diff --git a/drivers/acpi/sleep.c b/drivers/acpi/sleep.c
> index 132a9df98471..b776dbd5a382 100644
> --- a/drivers/acpi/sleep.c
> +++ b/drivers/acpi/sleep.c
> @@ -56,7 +56,6 @@ static int tts_notify_reboot(struct notifier_block *this,
>
> static struct notifier_block tts_notifier = {
> .notifier_call = tts_notify_reboot,
> - .next = NULL,
> .priority = 0,
IFF this becomes important for some reason (and right now I don't see
it), please start by using proper wrappers for notifiers so that the
implementation details don't leak into the users. That would actually
be useful on it's own even.
^ permalink raw reply
* Re: [RFC PATCH 0/2] Decouple ftrace/livepatch from module loader via notifier priority and reverse traversal
From: Christoph Hellwig @ 2026-04-15 7:38 UTC (permalink / raw)
To: chensong_2000
Cc: rafael, lenb, mturquette, sboyd, viresh.kumar, agk, snitzer,
mpatocka, bmarzins, song, yukuai, linan122, jason.wessel, danielt,
dianders, horms, davem, edumazet, kuba, pabeni, paulmck, frederic,
mcgrof, petr.pavlu, da.gomez, samitolvanen, atomlin, jpoimboe,
jikos, mbenes, pmladek, joe.lawrence, rostedt, mhiramat,
mark.rutland, mathieu.desnoyers, linux-modules, linux-kernel,
linux-trace-kernel, linux-acpi, linux-clk, linux-pm,
live-patching, dm-devel, linux-raid, kgdb-bugreport, netdev
In-Reply-To: <20260413080140.180616-1-chensong_2000@189.cn>
On Mon, Apr 13, 2026 at 04:01:40PM +0800, chensong_2000@189.cn wrote:
> From: Song Chen <chensong_2000@189.cn>
>
> This patchset addresses a long-standing tight coupling between the
> module loader and two of its key consumers: ftrace and livepatch.
>
> Background:
>
> The module loader currently hard-codes direct calls to
> ftrace_module_enable(), klp_module_coming(), klp_module_going() and
> ftrace_release_mod() inside prepare_coming_module() and the module
> unload path.
And that is bad why?
> 13 files changed, 290 insertions(+), 74 deletions(-)
This is a lot of new complex code touching a lot of places for no obvious
gain. What is the reason for this series? Does it prepare for something
else?
^ permalink raw reply
* [RFC PATCH 1/2] kernel/notifier: replace single-linked list with double-linked list for reverse traversal
From: chensong_2000 @ 2026-04-15 7:01 UTC (permalink / raw)
To: rafael, lenb, mturquette, sboyd, viresh.kumar, agk, snitzer,
mpatocka, bmarzins, song, yukuai, linan122, jason.wessel, danielt,
dianders, horms, davem, edumazet, kuba, pabeni, paulmck, frederic,
mcgrof, petr.pavlu, da.gomez, samitolvanen, atomlin, jpoimboe,
jikos, mbenes, pmladek, joe.lawrence, rostedt, mhiramat,
mark.rutland, mathieu.desnoyers
Cc: linux-modules, linux-kernel, linux-trace-kernel, linux-acpi,
linux-clk, linux-pm, live-patching, dm-devel, linux-raid,
kgdb-bugreport, netdev, Song Chen
From: Song Chen <chensong_2000@189.cn>
The current notifier chain implementation uses a single-linked list
(struct notifier_block *next), which only supports forward traversal
in priority order. This makes it difficult to handle cleanup/teardown
scenarios that require notifiers to be called in reverse priority order.
A concrete example is the ordering dependency between ftrace and
livepatch during module load/unload. see the detail here [1].
This patch replaces the single-linked list in struct notifier_block
with a struct list_head, converting the notifier chain into a
doubly-linked list sorted in descending priority order. Based on
this, a new function notifier_call_chain_reverse() is introduced,
which traverses the chain in reverse (ascending priority order).
The corresponding blocking_notifier_call_chain_reverse() is also
added as the locking wrapper for blocking notifier chains.
The internal notifier_call_chain_robust() is updated to use
notifier_call_chain_reverse() for rollback: on error, it records
the failing notifier (last_nb) and the count of successfully called
notifiers (nr), then rolls back exactly those nr-1 notifiers in
reverse order starting from last_nb's predecessor, without needing
to know the total length of the chain.
With this change, subsystems with symmetric setup/teardown ordering
requirements can register a single notifier_block with one priority
value, and rely on blocking_notifier_call_chain() for forward
traversal and blocking_notifier_call_chain_reverse() for reverse
traversal, without needing hard-coded call sequences or separate
notifier registrations for each direction.
[1]:https://lore.kernel.org/all
/alpine.LNX.2.00.1602172216491.22700@cbobk.fhfr.pm/
Signed-off-by: Song Chen <chensong_2000@189.cn>
---
drivers/acpi/sleep.c | 1 -
drivers/clk/clk.c | 2 +-
drivers/cpufreq/cpufreq.c | 2 +-
drivers/md/dm-integrity.c | 1 -
drivers/md/md.c | 1 -
include/linux/notifier.h | 26 ++---
kernel/debug/debug_core.c | 1 -
kernel/notifier.c | 219 ++++++++++++++++++++++++++++++++------
net/ipv4/nexthop.c | 2 +-
9 files changed, 201 insertions(+), 54 deletions(-)
diff --git a/drivers/acpi/sleep.c b/drivers/acpi/sleep.c
index 132a9df98471..b776dbd5a382 100644
--- a/drivers/acpi/sleep.c
+++ b/drivers/acpi/sleep.c
@@ -56,7 +56,6 @@ static int tts_notify_reboot(struct notifier_block *this,
static struct notifier_block tts_notifier = {
.notifier_call = tts_notify_reboot,
- .next = NULL,
.priority = 0,
};
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 47093cda9df3..b6fe380d0468 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -4862,7 +4862,7 @@ int clk_notifier_unregister(struct clk *clk, struct notifier_block *nb)
clk->core->notifier_count--;
/* XXX the notifier code should handle this better */
- if (!cn->notifier_head.head) {
+ if (list_empty(&cn->notifier_head.head)) {
srcu_cleanup_notifier_head(&cn->notifier_head);
list_del(&cn->node);
kfree(cn);
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 277884d91913..12637e742ffa 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -445,7 +445,7 @@ static void cpufreq_list_transition_notifiers(void)
mutex_lock(&cpufreq_transition_notifier_list.mutex);
- for (nb = cpufreq_transition_notifier_list.head; nb; nb = nb->next)
+ list_for_each_entry(nb, &cpufreq_transition_notifier_list.head, entry)
pr_info("%pS\n", nb->notifier_call);
mutex_unlock(&cpufreq_transition_notifier_list.mutex);
diff --git a/drivers/md/dm-integrity.c b/drivers/md/dm-integrity.c
index 06e805902151..ccdf75c40b62 100644
--- a/drivers/md/dm-integrity.c
+++ b/drivers/md/dm-integrity.c
@@ -3909,7 +3909,6 @@ static void dm_integrity_resume(struct dm_target *ti)
}
ic->reboot_notifier.notifier_call = dm_integrity_reboot;
- ic->reboot_notifier.next = NULL;
ic->reboot_notifier.priority = INT_MAX - 1; /* be notified after md and before hardware drivers */
WARN_ON(register_reboot_notifier(&ic->reboot_notifier));
diff --git a/drivers/md/md.c b/drivers/md/md.c
index 3ce6f9e9d38e..8249e78636ab 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -10480,7 +10480,6 @@ static int md_notify_reboot(struct notifier_block *this,
static struct notifier_block md_notifier = {
.notifier_call = md_notify_reboot,
- .next = NULL,
.priority = INT_MAX, /* before any real devices */
};
diff --git a/include/linux/notifier.h b/include/linux/notifier.h
index 01b6c9d9956f..b2abbdfcaadd 100644
--- a/include/linux/notifier.h
+++ b/include/linux/notifier.h
@@ -53,41 +53,41 @@ typedef int (*notifier_fn_t)(struct notifier_block *nb,
struct notifier_block {
notifier_fn_t notifier_call;
- struct notifier_block __rcu *next;
+ struct list_head __rcu entry;
int priority;
};
struct atomic_notifier_head {
spinlock_t lock;
- struct notifier_block __rcu *head;
+ struct list_head __rcu head;
};
struct blocking_notifier_head {
struct rw_semaphore rwsem;
- struct notifier_block __rcu *head;
+ struct list_head __rcu head;
};
struct raw_notifier_head {
- struct notifier_block __rcu *head;
+ struct list_head __rcu head;
};
struct srcu_notifier_head {
struct mutex mutex;
struct srcu_usage srcuu;
struct srcu_struct srcu;
- struct notifier_block __rcu *head;
+ struct list_head __rcu head;
};
#define ATOMIC_INIT_NOTIFIER_HEAD(name) do { \
spin_lock_init(&(name)->lock); \
- (name)->head = NULL; \
+ INIT_LIST_HEAD(&(name)->head); \
} while (0)
#define BLOCKING_INIT_NOTIFIER_HEAD(name) do { \
init_rwsem(&(name)->rwsem); \
- (name)->head = NULL; \
+ INIT_LIST_HEAD(&(name)->head); \
} while (0)
#define RAW_INIT_NOTIFIER_HEAD(name) do { \
- (name)->head = NULL; \
+ INIT_LIST_HEAD(&(name)->head); \
} while (0)
/* srcu_notifier_heads must be cleaned up dynamically */
@@ -97,17 +97,17 @@ extern void srcu_init_notifier_head(struct srcu_notifier_head *nh);
#define ATOMIC_NOTIFIER_INIT(name) { \
.lock = __SPIN_LOCK_UNLOCKED(name.lock), \
- .head = NULL }
+ .head = LIST_HEAD_INIT((name).head) }
#define BLOCKING_NOTIFIER_INIT(name) { \
.rwsem = __RWSEM_INITIALIZER((name).rwsem), \
- .head = NULL }
+ .head = LIST_HEAD_INIT((name).head) }
#define RAW_NOTIFIER_INIT(name) { \
- .head = NULL }
+ .head = LIST_HEAD_INIT((name).head) }
#define SRCU_NOTIFIER_INIT(name, pcpu) \
{ \
.mutex = __MUTEX_INITIALIZER(name.mutex), \
- .head = NULL, \
+ .head = LIST_HEAD_INIT((name).head), \
.srcuu = __SRCU_USAGE_INIT(name.srcuu), \
.srcu = __SRCU_STRUCT_INIT(name.srcu, name.srcuu, pcpu, 0), \
}
@@ -170,6 +170,8 @@ extern int atomic_notifier_call_chain(struct atomic_notifier_head *nh,
unsigned long val, void *v);
extern int blocking_notifier_call_chain(struct blocking_notifier_head *nh,
unsigned long val, void *v);
+extern int blocking_notifier_call_chain_reverse(struct blocking_notifier_head *nh,
+ unsigned long val, void *v);
extern int raw_notifier_call_chain(struct raw_notifier_head *nh,
unsigned long val, void *v);
extern int srcu_notifier_call_chain(struct srcu_notifier_head *nh,
diff --git a/kernel/debug/debug_core.c b/kernel/debug/debug_core.c
index 0b9495187fba..a26a7683d142 100644
--- a/kernel/debug/debug_core.c
+++ b/kernel/debug/debug_core.c
@@ -1054,7 +1054,6 @@ dbg_notify_reboot(struct notifier_block *this, unsigned long code, void *x)
static struct notifier_block dbg_reboot_notifier = {
.notifier_call = dbg_notify_reboot,
- .next = NULL,
.priority = INT_MAX,
};
diff --git a/kernel/notifier.c b/kernel/notifier.c
index 2f9fe7c30287..6f4d887771c4 100644
--- a/kernel/notifier.c
+++ b/kernel/notifier.c
@@ -14,39 +14,47 @@
* are layered on top of these, with appropriate locking added.
*/
-static int notifier_chain_register(struct notifier_block **nl,
+static int notifier_chain_register(struct list_head *nl,
struct notifier_block *n,
bool unique_priority)
{
- while ((*nl) != NULL) {
- if (unlikely((*nl) == n)) {
+ struct notifier_block *cur;
+
+ list_for_each_entry(cur, nl, entry) {
+ if (unlikely(cur == n)) {
WARN(1, "notifier callback %ps already registered",
n->notifier_call);
return -EEXIST;
}
- if (n->priority > (*nl)->priority)
- break;
- if (n->priority == (*nl)->priority && unique_priority)
+
+ if (n->priority == cur->priority && unique_priority)
return -EBUSY;
- nl = &((*nl)->next);
+
+ if (n->priority > cur->priority) {
+ list_add_tail(&n->entry, &cur->entry);
+ goto out;
+ }
}
- n->next = *nl;
- rcu_assign_pointer(*nl, n);
+
+ list_add_tail(&n->entry, nl);
+out:
trace_notifier_register((void *)n->notifier_call);
return 0;
}
-static int notifier_chain_unregister(struct notifier_block **nl,
+static int notifier_chain_unregister(struct list_head *nl,
struct notifier_block *n)
{
- while ((*nl) != NULL) {
- if ((*nl) == n) {
- rcu_assign_pointer(*nl, n->next);
+ struct notifier_block *cur;
+
+ list_for_each_entry(cur, nl, entry) {
+ if (cur == n) {
+ list_del(&n->entry);
trace_notifier_unregister((void *)n->notifier_call);
return 0;
}
- nl = &((*nl)->next);
}
+
return -ENOENT;
}
@@ -59,25 +67,25 @@ static int notifier_chain_unregister(struct notifier_block **nl,
* value of this parameter is -1.
* @nr_calls: Records the number of notifications sent. Don't care
* value of this field is NULL.
+ * @last_nb: Records the last called notifier block for rolling back
* Return: notifier_call_chain returns the value returned by the
* last notifier function called.
*/
-static int notifier_call_chain(struct notifier_block **nl,
+static int notifier_call_chain(struct list_head *nl,
unsigned long val, void *v,
- int nr_to_call, int *nr_calls)
+ int nr_to_call, int *nr_calls,
+ struct notifier_block **last_nb)
{
int ret = NOTIFY_DONE;
- struct notifier_block *nb, *next_nb;
-
- nb = rcu_dereference_raw(*nl);
+ struct notifier_block *nb;
- while (nb && nr_to_call) {
- next_nb = rcu_dereference_raw(nb->next);
+ if (!nr_to_call)
+ return ret;
+ list_for_each_entry(nb, nl, entry) {
#ifdef CONFIG_DEBUG_NOTIFIERS
if (unlikely(!func_ptr_is_kernel_text(nb->notifier_call))) {
WARN(1, "Invalid notifier called!");
- nb = next_nb;
continue;
}
#endif
@@ -87,15 +95,118 @@ static int notifier_call_chain(struct notifier_block **nl,
if (nr_calls)
(*nr_calls)++;
+ if (last_nb)
+ *last_nb = nb;
+
if (ret & NOTIFY_STOP_MASK)
break;
- nb = next_nb;
- nr_to_call--;
+
+ if (nr_to_call-- == 0)
+ break;
}
return ret;
}
NOKPROBE_SYMBOL(notifier_call_chain);
+/**
+ * notifier_call_chain_reverse - Informs the registered notifiers
+ * about an event reversely.
+ * @nl: Pointer to head of the blocking notifier chain
+ * @val: Value passed unmodified to notifier function
+ * @v: Pointer passed unmodified to notifier function
+ * @nr_to_call: Number of notifier functions to be called. Don't care
+ * value of this parameter is -1.
+ * @nr_calls: Records the number of notifications sent. Don't care
+ * value of this field is NULL.
+ * Return: notifier_call_chain returns the value returned by the
+ * last notifier function called.
+ */
+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);
+
+/**
+ * notifier_call_chain_rcu - Informs the registered notifiers
+ * about an event for srcu notifier chain.
+ * @nl: Pointer to head of the blocking notifier chain
+ * @val: Value passed unmodified to notifier function
+ * @v: Pointer passed unmodified to notifier function
+ * @nr_to_call: Number of notifier functions to be called. Don't care
+ * value of this parameter is -1.
+ * @nr_calls: Records the number of notifications sent. Don't care
+ * value of this field is NULL.
+ * Return: notifier_call_chain returns the value returned by the
+ * last notifier function called.
+ */
+static int notifier_call_chain_rcu(struct list_head *nl,
+ unsigned long val, void *v,
+ int nr_to_call, int *nr_calls)
+{
+ int ret = NOTIFY_DONE;
+ struct notifier_block *nb;
+
+ if (!nr_to_call)
+ return ret;
+
+ list_for_each_entry_rcu(nb, nl, entry) {
+#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_rcu);
+
/**
* notifier_call_chain_robust - Inform the registered notifiers about an event
* and rollback on error.
@@ -111,15 +222,16 @@ NOKPROBE_SYMBOL(notifier_call_chain);
*
* Return: the return value of the @val_up call.
*/
-static int notifier_call_chain_robust(struct notifier_block **nl,
+static int notifier_call_chain_robust(struct list_head *nl,
unsigned long val_up, unsigned long val_down,
void *v)
{
int ret, nr = 0;
+ struct notifier_block *last_nb = NULL;
- ret = notifier_call_chain(nl, val_up, v, -1, &nr);
+ ret = notifier_call_chain(nl, val_up, v, -1, &nr, &last_nb);
if (ret & NOTIFY_STOP_MASK)
- notifier_call_chain(nl, val_down, v, nr-1, NULL);
+ notifier_call_chain_reverse(nl, last_nb, val_down, v, nr-1, NULL);
return ret;
}
@@ -220,7 +332,7 @@ int atomic_notifier_call_chain(struct atomic_notifier_head *nh,
int ret;
rcu_read_lock();
- ret = notifier_call_chain(&nh->head, val, v, -1, NULL);
+ ret = notifier_call_chain(&nh->head, val, v, -1, NULL, NULL);
rcu_read_unlock();
return ret;
@@ -238,7 +350,7 @@ NOKPROBE_SYMBOL(atomic_notifier_call_chain);
*/
bool atomic_notifier_call_chain_is_empty(struct atomic_notifier_head *nh)
{
- return !rcu_access_pointer(nh->head);
+ return list_empty(&nh->head);
}
/*
@@ -340,7 +452,7 @@ int blocking_notifier_call_chain_robust(struct blocking_notifier_head *nh,
* racy then it does not matter what the result of the test
* is, we re-check the list after having taken the lock anyway:
*/
- if (rcu_access_pointer(nh->head)) {
+ if (!list_empty(&nh->head)) {
down_read(&nh->rwsem);
ret = notifier_call_chain_robust(&nh->head, val_up, val_down, v);
up_read(&nh->rwsem);
@@ -375,15 +487,52 @@ int blocking_notifier_call_chain(struct blocking_notifier_head *nh,
* racy then it does not matter what the result of the test
* is, we re-check the list after having taken the lock anyway:
*/
- if (rcu_access_pointer(nh->head)) {
+ if (!list_empty(&nh->head)) {
down_read(&nh->rwsem);
- ret = notifier_call_chain(&nh->head, val, v, -1, NULL);
+ ret = notifier_call_chain(&nh->head, val, v, -1, NULL, NULL);
up_read(&nh->rwsem);
}
return ret;
}
EXPORT_SYMBOL_GPL(blocking_notifier_call_chain);
+/**
+ * blocking_notifier_call_chain_reverse - Call functions reversely in
+ * a blocking notifier chain
+ * @nh: Pointer to head of the blocking notifier chain
+ * @val: Value passed unmodified to notifier function
+ * @v: Pointer passed unmodified to notifier function
+ *
+ * Calls each function in a notifier chain in turn. The functions
+ * run in a process context, so they are allowed to block.
+ *
+ * If the return value of the notifier can be and'ed
+ * with %NOTIFY_STOP_MASK then blocking_notifier_call_chain()
+ * will return immediately, with the return value of
+ * the notifier function which halted execution.
+ * Otherwise the return value is the return value
+ * of the last notifier function called.
+ */
+
+int blocking_notifier_call_chain_reverse(struct blocking_notifier_head *nh,
+ unsigned long val, void *v)
+{
+ int ret = NOTIFY_DONE;
+
+ /*
+ * We check the head outside the lock, but if this access is
+ * racy then it does not matter what the result of the test
+ * is, we re-check the list after having taken the lock anyway:
+ */
+ if (!list_empty(&nh->head)) {
+ down_read(&nh->rwsem);
+ ret = notifier_call_chain_reverse(&nh->head, NULL, val, v, -1, NULL);
+ up_read(&nh->rwsem);
+ }
+ return ret;
+}
+EXPORT_SYMBOL_GPL(blocking_notifier_call_chain_reverse);
+
/*
* Raw notifier chain routines. There is no protection;
* the caller must provide it. Use at your own risk!
@@ -450,7 +599,7 @@ EXPORT_SYMBOL_GPL(raw_notifier_call_chain_robust);
int raw_notifier_call_chain(struct raw_notifier_head *nh,
unsigned long val, void *v)
{
- return notifier_call_chain(&nh->head, val, v, -1, NULL);
+ return notifier_call_chain(&nh->head, val, v, -1, NULL, NULL);
}
EXPORT_SYMBOL_GPL(raw_notifier_call_chain);
@@ -543,7 +692,7 @@ int srcu_notifier_call_chain(struct srcu_notifier_head *nh,
int idx;
idx = srcu_read_lock(&nh->srcu);
- ret = notifier_call_chain(&nh->head, val, v, -1, NULL);
+ ret = notifier_call_chain_rcu(&nh->head, val, v, -1, NULL);
srcu_read_unlock(&nh->srcu, idx);
return ret;
}
@@ -566,7 +715,7 @@ void srcu_init_notifier_head(struct srcu_notifier_head *nh)
mutex_init(&nh->mutex);
if (init_srcu_struct(&nh->srcu) < 0)
BUG();
- nh->head = NULL;
+ INIT_LIST_HEAD(&nh->head);
}
EXPORT_SYMBOL_GPL(srcu_init_notifier_head);
diff --git a/net/ipv4/nexthop.c b/net/ipv4/nexthop.c
index c942f1282236..0afcba2967c7 100644
--- a/net/ipv4/nexthop.c
+++ b/net/ipv4/nexthop.c
@@ -90,7 +90,7 @@ static const struct nla_policy rtm_nh_res_bucket_policy_get[] = {
static bool nexthop_notifiers_is_empty(struct net *net)
{
- return !net->nexthop.notifier_chain.head;
+ return list_empty(&net->nexthop.notifier_chain.head);
}
static void
--
2.43.0
^ permalink raw reply related
* Re: [RFC PATCH 2/2] kernel/module: Decouple klp and ftrace from load_module
From: Song Chen @ 2026-04-15 6:43 UTC (permalink / raw)
To: Petr Pavlu
Cc: rafael, lenb, mturquette, sboyd, viresh.kumar, agk, snitzer,
mpatocka, bmarzins, song, yukuai, linan122, jason.wessel, danielt,
dianders, horms, davem, edumazet, kuba, pabeni, paulmck, frederic,
mcgrof, da.gomez, samitolvanen, atomlin, jpoimboe, jikos, mbenes,
pmladek, joe.lawrence, rostedt, mhiramat, mark.rutland,
mathieu.desnoyers, linux-modules, linux-kernel,
linux-trace-kernel, linux-acpi, linux-clk, linux-pm,
live-patching, dm-devel, linux-raid, kgdb-bugreport, netdev
In-Reply-To: <1191caf5-6a61-4622-a15e-854d3701f4fc@suse.com>
Hi,
On 4/14/26 22:33, Petr Pavlu wrote:
> On 4/13/26 10:07 AM, chensong_2000@189.cn wrote:
>> From: Song Chen <chensong_2000@189.cn>
>>
>> ftrace and livepatch currently have their module load/unload callbacks
>> hard-coded in the module loader as direct function calls to
>> ftrace_module_enable(), klp_module_coming(), klp_module_going()
>> and ftrace_release_mod(). This tight coupling was originally introduced
>> to enforce strict call ordering that could not be guaranteed by the
>> module notifier chain, which only supported forward traversal. Their
>> notifiers were moved in and out back and forth. see [1] and [2].
>
> I'm unclear about what is meant by the notifiers being moved back and
> forth. The links point to patches that converted ftrace+klp from using
> module notifiers to explicit callbacks due to ordering issues, but this
> switch occurred only once. Have there been other attempts to use
> notifiers again?
>
Yes,only once,i will rephrase.
>>
>> Now that the notifier chain supports reverse traversal via
>> blocking_notifier_call_chain_reverse(), the ordering can be enforced
>> purely through notifier priority. As a result, the module loader is now
>> decoupled from the implementation details of ftrace and livepatch.
>> What's more, adding a new subsystem with symmetric setup/teardown ordering
>> requirements during module load/unload no longer requires modifying
>> kernel/module/main.c; it only needs to register a notifier_block with an
>> appropriate priority.
>>
>> [1]:https://lore.kernel.org/all/
>> alpine.LNX.2.00.1602172216491.22700@cbobk.fhfr.pm/
>> [2]:https://lore.kernel.org/all/
>> 20160301030034.GC12120@packer-debian-8-amd64.digitalocean.com/
>
> Nit: Avoid wrapping URLs, as it breaks autolinking and makes the links
> harder to copy.
>
> Better links would be:
> [1] https://lore.kernel.org/all/1455661953-15838-1-git-send-email-jeyu@redhat.com/
> [2] https://lore.kernel.org/all/1458176139-17455-1-git-send-email-jeyu@redhat.com/
>
> The first link is the final version of what landed as commit
> 7dcd182bec27 ("ftrace/module: remove ftrace module notifier"). The
> second is commit 7e545d6eca20 ("livepatch/module: remove livepatch
> module notifier").
>
Thank you, i will update.
>>
>> Signed-off-by: Song Chen <chensong_2000@189.cn>
>> ---
>> include/linux/module.h | 8 ++++++++
>> kernel/livepatch/core.c | 29 ++++++++++++++++++++++++++++-
>> kernel/module/main.c | 34 +++++++++++++++-------------------
>> kernel/trace/ftrace.c | 38 ++++++++++++++++++++++++++++++++++++++
>> 4 files changed, 89 insertions(+), 20 deletions(-)
>>
>> diff --git a/include/linux/module.h b/include/linux/module.h
>> index 14f391b186c6..0bdd56f9defd 100644
>> --- a/include/linux/module.h
>> +++ b/include/linux/module.h
>> @@ -308,6 +308,14 @@ enum module_state {
>> MODULE_STATE_COMING, /* Full formed, running module_init. */
>> MODULE_STATE_GOING, /* Going away. */
>> MODULE_STATE_UNFORMED, /* Still setting it up. */
>> + MODULE_STATE_FORMED,
>
> I don't see a reason to add a new module state. Why is it necessary and
> how does it fit with the existing states?
>
because once notifier fails in state MODULE_STATE_UNFORMED (now only
ftrace has someting to do in this state), notifier chain will roll back
by calling blocking_notifier_call_chain_robust, i'm afraid
MODULE_STATE_GOING is going to jeopardise the notifers which don't
handle it appropriately, like:
case MODULE_STATE_COMING:
kmalloc();
case MODULE_STATE_GOING:
kfree();
>> +};
>> +
>> +enum module_notifier_prio {
>> + MODULE_NOTIFIER_PRIO_LOW = INT_MIN, /* Low prioroty, coming last, going first */
>> + MODULE_NOTIFIER_PRIO_MID = 0, /* Normal priority. */
>> + MODULE_NOTIFIER_PRIO_SECOND_HIGH = INT_MAX - 1, /* Second high priorigy, coming second*/
>> + MODULE_NOTIFIER_PRIO_HIGH = INT_MAX, /* High priorigy, coming first, going late. */
>
> I suggest being explicit about how the notifiers are ordered. For
> example:
>
> enum module_notifier_prio {
> MODULE_NOTIFIER_PRIO_NORMAL, /* Normal priority, coming last, going first. */
> MODULE_NOTIFIER_PRIO_LIVEPATCH,
> MODULE_NOTIFIER_PRIO_FTRACE, /* High priority, coming first, going late. */
> };
>
accepted.
>> };
>>
>> struct mod_tree_node {
>> diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
>> index 28d15ba58a26..ce78bb23e24b 100644
>> --- a/kernel/livepatch/core.c
>> +++ b/kernel/livepatch/core.c
>> @@ -1375,13 +1375,40 @@ void *klp_find_section_by_name(const struct module *mod, const char *name,
>> }
>> EXPORT_SYMBOL_GPL(klp_find_section_by_name);
>>
>> +static int klp_module_callback(struct notifier_block *nb, unsigned long op,
>> + void *module)
>> +{
>> + struct module *mod = module;
>> + int err = 0;
>> +
>> + switch (op) {
>> + case MODULE_STATE_COMING:
>> + err = klp_module_coming(mod);
>> + break;
>> + case MODULE_STATE_LIVE:
>> + break;
>> + case MODULE_STATE_GOING:
>> + klp_module_going(mod);
>> + break;
>> + default:
>> + break;
>> + }
>
> klp_module_coming() and klp_module_going() are now used only in
> kernel/livepatch/core.c where they are also defined. This means the
> functions can be static and their declarations removed from
> include/linux/livepatch.h.
>
> Nit: The MODULE_STATE_LIVE and default cases in the switch can be
> removed.
>
accepted.
>> +
>> + return notifier_from_errno(err);
>> +}
>> +
>> +static struct notifier_block klp_module_nb = {
>> + .notifier_call = klp_module_callback,
>> + .priority = MODULE_NOTIFIER_PRIO_SECOND_HIGH
>> +};
>> +
>> static int __init klp_init(void)
>> {
>> klp_root_kobj = kobject_create_and_add("livepatch", kernel_kobj);
>> if (!klp_root_kobj)
>> return -ENOMEM;
>>
>> - return 0;
>> + return register_module_notifier(&klp_module_nb);
>> }
>>
>> module_init(klp_init);
>> diff --git a/kernel/module/main.c b/kernel/module/main.c
>> index c3ce106c70af..226dd5b80997 100644
>> --- a/kernel/module/main.c
>> +++ b/kernel/module/main.c
>> @@ -833,10 +833,8 @@ SYSCALL_DEFINE2(delete_module, const char __user *, name_user,
>> /* Final destruction now no one is using it. */
>> if (mod->exit != NULL)
>> mod->exit();
>> - blocking_notifier_call_chain(&module_notify_list,
>> + blocking_notifier_call_chain_reverse(&module_notify_list,
>> MODULE_STATE_GOING, mod);
>> - klp_module_going(mod);
>> - ftrace_release_mod(mod);
>>
>> async_synchronize_full();
>>
>> @@ -3135,10 +3133,8 @@ static noinline int do_init_module(struct module *mod)
>> mod->state = MODULE_STATE_GOING;
>> synchronize_rcu();
>> module_put(mod);
>> - blocking_notifier_call_chain(&module_notify_list,
>> + blocking_notifier_call_chain_reverse(&module_notify_list,
>> MODULE_STATE_GOING, mod);
>> - klp_module_going(mod);
>> - ftrace_release_mod(mod);
>> free_module(mod);
>> wake_up_all(&module_wq);
>>
>
> The patch unexpectedly leaves a call to ftrace_free_mem() in
> do_init_module().
Thanks for pointing it out, it was removed when i implemented and
tested, but when i organized the patch, it was left. I will remove it.
>
>> @@ -3281,20 +3277,14 @@ static int complete_formation(struct module *mod, struct load_info *info)
>> return err;
>> }
>>
>> -static int prepare_coming_module(struct module *mod)
>> +static int prepare_module_state_transaction(struct module *mod,
>> + unsigned long val_up, unsigned long val_down)
>> {
>> int err;
>>
>> - ftrace_module_enable(mod);
>> - err = klp_module_coming(mod);
>> - if (err)
>> - return err;
>> -
>> err = blocking_notifier_call_chain_robust(&module_notify_list,
>> - MODULE_STATE_COMING, MODULE_STATE_GOING, mod);
>> + val_up, val_down, mod);
>> err = notifier_to_errno(err);
>> - if (err)
>> - klp_module_going(mod);
>>
>> return err;
>> }
>> @@ -3468,14 +3458,21 @@ static int load_module(struct load_info *info, const char __user *uargs,
>> init_build_id(mod, info);
>>
>> /* Ftrace init must be called in the MODULE_STATE_UNFORMED state */
>> - ftrace_module_init(mod);
>> + err = prepare_module_state_transaction(mod,
>> + MODULE_STATE_UNFORMED, MODULE_STATE_FORMED);
>
> I believe val_down should be MODULE_STATE_GOING to reverse the
> operation. Why is the new state MODULE_STATE_FORMED needed here?
to avoid this:
case MODULE_STATE_COMING:
kmalloc();
case MODULE_STATE_GOING:
kfree();
>
>> + if (err)
>> + goto ddebug_cleanup;
>>
>> /* Finally it's fully formed, ready to start executing. */
>> err = complete_formation(mod, info);
>> - if (err)
>> + if (err) {
>> + blocking_notifier_call_chain_reverse(&module_notify_list,
>> + MODULE_STATE_FORMED, mod);
>> goto ddebug_cleanup;
>> + }
>>
>> - err = prepare_coming_module(mod);
>> + err = prepare_module_state_transaction(mod,
>> + MODULE_STATE_COMING, MODULE_STATE_GOING);
>> if (err)
>> goto bug_cleanup;
>>
>> @@ -3522,7 +3519,6 @@ static int load_module(struct load_info *info, const char __user *uargs,
>> destroy_params(mod->kp, mod->num_kp);
>> blocking_notifier_call_chain(&module_notify_list,
>> MODULE_STATE_GOING, mod);
>
> My understanding is that all notifier chains for MODULE_STATE_GOING
> should be reversed.
yes, all, from lowest priority notifier to highest.
I will resend patch 1 which was failed due to my proxy setting.
>
>> - klp_module_going(mod);
>> bug_cleanup:
>> mod->state = MODULE_STATE_GOING;
>> /* module_bug_cleanup needs module_mutex protection */
>
> The patch removes the klp_module_going() cleanup call in load_module().
> Similarly, the ftrace_release_mod() call under the ddebug_cleanup label
> should be removed and appropriately replaced with a cleanup via
> a notifier.
>
err = prepare_module_state_transaction(mod,
MODULE_STATE_UNFORMED, MODULE_STATE_FORMED);
if (err)
goto ddebug_cleanup;
ftrace will be cleanup in blocking_notifier_call_chain_robust rolling back.
err = prepare_module_state_transaction(mod,
MODULE_STATE_COMING, MODULE_STATE_GOING);
each notifier including ftrace and klp will be cleanup in
blocking_notifier_call_chain_robust rolling back.
if all notifiers are successful in MODULE_STATE_COMING, they all will be
clean up in
coming_cleanup:
mod->state = MODULE_STATE_GOING;
destroy_params(mod->kp, mod->num_kp);
blocking_notifier_call_chain(&module_notify_list,
MODULE_STATE_GOING, mod);
if something wrong underneath.
>> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
>> index 8df69e702706..efedb98d3db4 100644
>> --- a/kernel/trace/ftrace.c
>> +++ b/kernel/trace/ftrace.c
>> @@ -5241,6 +5241,44 @@ static int __init ftrace_mod_cmd_init(void)
>> }
>> core_initcall(ftrace_mod_cmd_init);
>>
>> +static int ftrace_module_callback(struct notifier_block *nb, unsigned long op,
>> + void *module)
>> +{
>> + struct module *mod = module;
>> +
>> + switch (op) {
>> + case MODULE_STATE_UNFORMED:
>> + ftrace_module_init(mod);
>> + break;
>> + case MODULE_STATE_COMING:
>> + ftrace_module_enable(mod);
>> + break;
>> + case MODULE_STATE_LIVE:
>> + ftrace_free_mem(mod, mod->mem[MOD_INIT_TEXT].base,
>> + mod->mem[MOD_INIT_TEXT].base + mod->mem[MOD_INIT_TEXT].size);
>> + break;
>> + case MODULE_STATE_GOING:
>> + case MODULE_STATE_FORMED:
>> + ftrace_release_mod(mod);
>> + break;
>> + default:
>> + break;
>> + }
>
> ftrace_module_init(), ftrace_module_enable(), ftrace_free_mem() and
> ftrace_release_mod() should be newly used only in kernel/trace/ftrace.c
> where they are also defined. The functions can then be made static and
> removed from include/linux/ftrace.h.
>
> Nit: The default case in the switch can be removed.
>
accepted.
>> +
>> + return notifier_from_errno(0);
>
> Nit: This can be simply "return NOTIFY_OK;".
accepted
>
>> +}
>> +
>> +static struct notifier_block ftrace_module_nb = {
>> + .notifier_call = ftrace_module_callback,
>> + .priority = MODULE_NOTIFIER_PRIO_HIGH
>> +};
>> +
>> +static int __init ftrace_register_module_notifier(void)
>> +{
>> + return register_module_notifier(&ftrace_module_nb);
>> +}
>> +core_initcall(ftrace_register_module_notifier);
>> +
>> static void function_trace_probe_call(unsigned long ip, unsigned long parent_ip,
>> struct ftrace_ops *op, struct ftrace_regs *fregs)
>> {
>
Best regards
Song
^ permalink raw reply
* Re: [PATCH] kbuild/btf: Remove broken module relinking exclusion
From: Alexei Starovoitov @ 2026-04-15 5:14 UTC (permalink / raw)
To: Nathan Chancellor, Ihor Solodrai, Alan Maguire
Cc: Petr Pavlu, Nicolas Schier, Alexei Starovoitov, Daniel Borkmann,
Andrii Nakryiko, Martin KaFai Lau, Eduard Zingerman, Song Liu,
Yonghong Song, John Fastabend, KP Singh, Stanislav Fomichev,
Hao Luo, Jiri Olsa, Luis Chamberlain, Daniel Gomez, Sami Tolvanen,
Aaron Tomlin, Masahiro Yamada, Sasha Levin,
Linux Kbuild mailing list, bpf, linux-modules, LKML
In-Reply-To: <20260414203418.GA1022044@ax162>
On Tue, Apr 14, 2026 at 1:34 PM Nathan Chancellor <nathan@kernel.org> wrote:
>
> On Fri, Apr 10, 2026 at 03:13:29PM +0200, Petr Pavlu wrote:
> > Commit 5f9ae91f7c0d ("kbuild: Build kernel module BTFs if BTF is enabled
> > and pahole supports it") in 2020 introduced CONFIG_DEBUG_INFO_BTF_MODULES
> > to enable generation of split BTF for kernel modules. This change required
> > the %.ko Makefile rule to additionally depend on vmlinux, which is used as
> > a base for deduplication. The regular ld_ko_o command executed by the rule
> > was then modified to be skipped if only vmlinux changes. This was done by
> > introducing a new if_changed_except command and updating the original call
> > to '+$(call if_changed_except,ld_ko_o,vmlinux)'.
> >
> > Later, commit 214c0eea43b2 ("kbuild: add $(objtree)/ prefix to some
> > in-kernel build artifacts") in 2024 updated the rule's reference to vmlinux
> > from 'vmlinux' to '$(objtree)/vmlinux'. This accidentally broke the
> > previous logic to skip relinking modules if only vmlinux changes. The issue
> > is that '$(objtree)' is typically '.' and GNU Make normalizes the resulting
> > prerequisite './vmlinux' to just 'vmlinux', while the exclusion logic
> > retains the raw './vmlinux'. As a result, if_changed_except doesn't
> > correctly filter out vmlinux. Consequently, with
> > CONFIG_DEBUG_INFO_BTF_MODULES=y, modules are relinked even if only vmlinux
> > changes.
> >
> > It is possible to fix this Makefile issue. However, having the %.ko rule
> > update the resulting file in place without starting from the original
> > inputs is rather fragile. The logic is harder to debug if something breaks
> > during a subsequent .ko update because the old input is lost due to the
> > overwrite. Additionally, it requires that the BTF processing is idempotent.
> > For example, sorting id+flags BTF_SET8 pairs in .BTF_ids by resolve_btfids
> > currently doesn't have this property.
> >
> > One option is to split the %.ko target into two rules: the first for
> > partial linking and the second one for generating the BTF data. However,
> > this approach runs into an issue with requiring additional intermediate
> > files, which increases the size of the build directory. On my system, when
> > using a large distribution config with ~5500 modules, the size of the build
> > directory with debuginfo enabled is already ~25 GB, with .ko files
> > occupying ~8 GB. Duplicating these .ko files doesn't seem practical.
> >
> > Measuring the speed of the %.ko processing shows that the link step is
> > actually relatively fast. It takes about 20% of the overall rule time,
> > while the BTF processing accounts for 80%. Moreover, skipping the link part
> > becomes relevant only during local development. In such cases, developers
> > typically use configs that enable a limited number of modules, so having
> > the %.ko rule slightly slower doesn't significantly impact the total
> > rebuild time. This is supported by the fact that no one has complained
> > about this optimization being broken for the past two years.
> >
> > Therefore, remove the logic that prevents module relinking when only
> > vmlinux changes and simplify Makefile.modfinal.
> >
> > Signed-off-by: Petr Pavlu <petr.pavlu@suse.com>
>
> If the BPF folks want to take this since it deals with BTF:
>
> Acked-by: Nathan Chancellor <nathan@kernel.org>
>
> Otherwise, either Nicolas can take this for 7.1 or I will pick it up for
> 7.2 when 7.1-rc1 is out.
Alan, Ihor,
As resident btf gen experts, Please take a look.
^ permalink raw reply
* Re: [GIT PULL] Modules changes for v7.1-rc1
From: pr-tracker-bot @ 2026-04-15 0:58 UTC (permalink / raw)
To: Sami Tolvanen
Cc: Linus Torvalds, Sami Tolvanen, Aaron Tomlin, Daniel Gomez,
Joe Lawrence, linux-kernel, linux-modules, Lucas De Marchi,
Luis Chamberlain, Nicholas Sielicki, Petr Pavlu, Siddharth Nayyar,
Thomas Weißschuh
In-Reply-To: <20260413002616.1966468-2-samitolvanen@google.com>
The pull request you sent on Mon, 13 Apr 2026 00:26:14 +0000:
> git://git.kernel.org/pub/scm/linux/kernel/git/modules/linux.git tags/modules-7.1-rc1
has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/88b29f3f579987fff0d2bd726d5fa95a53f857fa
Thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/prtracker.html
^ permalink raw reply
* Re: [PATCH] kbuild/btf: Remove broken module relinking exclusion
From: Nathan Chancellor @ 2026-04-14 20:34 UTC (permalink / raw)
To: Petr Pavlu
Cc: Nicolas Schier, Alexei Starovoitov, Daniel Borkmann,
Andrii Nakryiko, Martin KaFai Lau, Eduard Zingerman, Song Liu,
Yonghong Song, John Fastabend, KP Singh, Stanislav Fomichev,
Hao Luo, Jiri Olsa, Luis Chamberlain, Daniel Gomez, Sami Tolvanen,
Aaron Tomlin, Ihor Solodrai, Masahiro Yamada, Sasha Levin,
linux-kbuild, bpf, linux-modules, linux-kernel
In-Reply-To: <20260410131343.2519532-1-petr.pavlu@suse.com>
On Fri, Apr 10, 2026 at 03:13:29PM +0200, Petr Pavlu wrote:
> Commit 5f9ae91f7c0d ("kbuild: Build kernel module BTFs if BTF is enabled
> and pahole supports it") in 2020 introduced CONFIG_DEBUG_INFO_BTF_MODULES
> to enable generation of split BTF for kernel modules. This change required
> the %.ko Makefile rule to additionally depend on vmlinux, which is used as
> a base for deduplication. The regular ld_ko_o command executed by the rule
> was then modified to be skipped if only vmlinux changes. This was done by
> introducing a new if_changed_except command and updating the original call
> to '+$(call if_changed_except,ld_ko_o,vmlinux)'.
>
> Later, commit 214c0eea43b2 ("kbuild: add $(objtree)/ prefix to some
> in-kernel build artifacts") in 2024 updated the rule's reference to vmlinux
> from 'vmlinux' to '$(objtree)/vmlinux'. This accidentally broke the
> previous logic to skip relinking modules if only vmlinux changes. The issue
> is that '$(objtree)' is typically '.' and GNU Make normalizes the resulting
> prerequisite './vmlinux' to just 'vmlinux', while the exclusion logic
> retains the raw './vmlinux'. As a result, if_changed_except doesn't
> correctly filter out vmlinux. Consequently, with
> CONFIG_DEBUG_INFO_BTF_MODULES=y, modules are relinked even if only vmlinux
> changes.
>
> It is possible to fix this Makefile issue. However, having the %.ko rule
> update the resulting file in place without starting from the original
> inputs is rather fragile. The logic is harder to debug if something breaks
> during a subsequent .ko update because the old input is lost due to the
> overwrite. Additionally, it requires that the BTF processing is idempotent.
> For example, sorting id+flags BTF_SET8 pairs in .BTF_ids by resolve_btfids
> currently doesn't have this property.
>
> One option is to split the %.ko target into two rules: the first for
> partial linking and the second one for generating the BTF data. However,
> this approach runs into an issue with requiring additional intermediate
> files, which increases the size of the build directory. On my system, when
> using a large distribution config with ~5500 modules, the size of the build
> directory with debuginfo enabled is already ~25 GB, with .ko files
> occupying ~8 GB. Duplicating these .ko files doesn't seem practical.
>
> Measuring the speed of the %.ko processing shows that the link step is
> actually relatively fast. It takes about 20% of the overall rule time,
> while the BTF processing accounts for 80%. Moreover, skipping the link part
> becomes relevant only during local development. In such cases, developers
> typically use configs that enable a limited number of modules, so having
> the %.ko rule slightly slower doesn't significantly impact the total
> rebuild time. This is supported by the fact that no one has complained
> about this optimization being broken for the past two years.
>
> Therefore, remove the logic that prevents module relinking when only
> vmlinux changes and simplify Makefile.modfinal.
>
> Signed-off-by: Petr Pavlu <petr.pavlu@suse.com>
If the BPF folks want to take this since it deals with BTF:
Acked-by: Nathan Chancellor <nathan@kernel.org>
Otherwise, either Nicolas can take this for 7.1 or I will pick it up for
7.2 when 7.1-rc1 is out.
> ---
> My previous attempt to fix this logic can be found at
> https://lore.kernel.org/linux-modules/20260402141911.1577711-1-petr.pavlu@suse.com/
> ---
> scripts/Makefile.modfinal | 10 +---------
> 1 file changed, 1 insertion(+), 9 deletions(-)
>
> diff --git a/scripts/Makefile.modfinal b/scripts/Makefile.modfinal
> index adcbcde16a07..01a37ec872b9 100644
> --- a/scripts/Makefile.modfinal
> +++ b/scripts/Makefile.modfinal
> @@ -46,17 +46,9 @@ quiet_cmd_btf_ko = BTF [M] $@
> $(CONFIG_SHELL) $(srctree)/scripts/gen-btf.sh --btf_base $(objtree)/vmlinux $@; \
> fi;
>
> -# Same as newer-prereqs, but allows to exclude specified extra dependencies
> -newer_prereqs_except = $(filter-out $(PHONY) $(1),$?)
> -
> -# Same as if_changed, but allows to exclude specified extra dependencies
> -if_changed_except = $(if $(call newer_prereqs_except,$(2))$(cmd-check), \
> - $(cmd); \
> - printf '%s\n' 'savedcmd_$@ := $(make-cmd)' > $(dot-target).cmd, @:)
> -
> # Re-generate module BTFs if either module's .ko or vmlinux changed
> %.ko: %.o %.mod.o .module-common.o $(objtree)/scripts/module.lds $(and $(CONFIG_DEBUG_INFO_BTF_MODULES),$(KBUILD_BUILTIN),$(objtree)/vmlinux) FORCE
> - +$(call if_changed_except,ld_ko_o,$(objtree)/vmlinux)
> + +$(call if_changed,ld_ko_o)
> ifdef CONFIG_DEBUG_INFO_BTF_MODULES
> +$(if $(newer-prereqs),$(call cmd,btf_ko))
> endif
>
> base-commit: 591cd656a1bf5ea94a222af5ef2ee76df029c1d2
> --
> 2.53.0
>
^ permalink raw reply
* Re: [RFC PATCH 2/2] kernel/module: Decouple klp and ftrace from load_module
From: Petr Pavlu @ 2026-04-14 14:33 UTC (permalink / raw)
To: chensong_2000
Cc: rafael, lenb, mturquette, sboyd, viresh.kumar, agk, snitzer,
mpatocka, bmarzins, song, yukuai, linan122, jason.wessel, danielt,
dianders, horms, davem, edumazet, kuba, pabeni, paulmck, frederic,
mcgrof, da.gomez, samitolvanen, atomlin, jpoimboe, jikos, mbenes,
pmladek, joe.lawrence, rostedt, mhiramat, mark.rutland,
mathieu.desnoyers, linux-modules, linux-kernel,
linux-trace-kernel, linux-acpi, linux-clk, linux-pm,
live-patching, dm-devel, linux-raid, kgdb-bugreport, netdev
In-Reply-To: <20260413080701.180976-1-chensong_2000@189.cn>
On 4/13/26 10:07 AM, chensong_2000@189.cn wrote:
> From: Song Chen <chensong_2000@189.cn>
>
> ftrace and livepatch currently have their module load/unload callbacks
> hard-coded in the module loader as direct function calls to
> ftrace_module_enable(), klp_module_coming(), klp_module_going()
> and ftrace_release_mod(). This tight coupling was originally introduced
> to enforce strict call ordering that could not be guaranteed by the
> module notifier chain, which only supported forward traversal. Their
> notifiers were moved in and out back and forth. see [1] and [2].
I'm unclear about what is meant by the notifiers being moved back and
forth. The links point to patches that converted ftrace+klp from using
module notifiers to explicit callbacks due to ordering issues, but this
switch occurred only once. Have there been other attempts to use
notifiers again?
>
> Now that the notifier chain supports reverse traversal via
> blocking_notifier_call_chain_reverse(), the ordering can be enforced
> purely through notifier priority. As a result, the module loader is now
> decoupled from the implementation details of ftrace and livepatch.
> What's more, adding a new subsystem with symmetric setup/teardown ordering
> requirements during module load/unload no longer requires modifying
> kernel/module/main.c; it only needs to register a notifier_block with an
> appropriate priority.
>
> [1]:https://lore.kernel.org/all/
> alpine.LNX.2.00.1602172216491.22700@cbobk.fhfr.pm/
> [2]:https://lore.kernel.org/all/
> 20160301030034.GC12120@packer-debian-8-amd64.digitalocean.com/
Nit: Avoid wrapping URLs, as it breaks autolinking and makes the links
harder to copy.
Better links would be:
[1] https://lore.kernel.org/all/1455661953-15838-1-git-send-email-jeyu@redhat.com/
[2] https://lore.kernel.org/all/1458176139-17455-1-git-send-email-jeyu@redhat.com/
The first link is the final version of what landed as commit
7dcd182bec27 ("ftrace/module: remove ftrace module notifier"). The
second is commit 7e545d6eca20 ("livepatch/module: remove livepatch
module notifier").
>
> Signed-off-by: Song Chen <chensong_2000@189.cn>
> ---
> include/linux/module.h | 8 ++++++++
> kernel/livepatch/core.c | 29 ++++++++++++++++++++++++++++-
> kernel/module/main.c | 34 +++++++++++++++-------------------
> kernel/trace/ftrace.c | 38 ++++++++++++++++++++++++++++++++++++++
> 4 files changed, 89 insertions(+), 20 deletions(-)
>
> diff --git a/include/linux/module.h b/include/linux/module.h
> index 14f391b186c6..0bdd56f9defd 100644
> --- a/include/linux/module.h
> +++ b/include/linux/module.h
> @@ -308,6 +308,14 @@ enum module_state {
> MODULE_STATE_COMING, /* Full formed, running module_init. */
> MODULE_STATE_GOING, /* Going away. */
> MODULE_STATE_UNFORMED, /* Still setting it up. */
> + MODULE_STATE_FORMED,
I don't see a reason to add a new module state. Why is it necessary and
how does it fit with the existing states?
> +};
> +
> +enum module_notifier_prio {
> + MODULE_NOTIFIER_PRIO_LOW = INT_MIN, /* Low prioroty, coming last, going first */
> + MODULE_NOTIFIER_PRIO_MID = 0, /* Normal priority. */
> + MODULE_NOTIFIER_PRIO_SECOND_HIGH = INT_MAX - 1, /* Second high priorigy, coming second*/
> + MODULE_NOTIFIER_PRIO_HIGH = INT_MAX, /* High priorigy, coming first, going late. */
I suggest being explicit about how the notifiers are ordered. For
example:
enum module_notifier_prio {
MODULE_NOTIFIER_PRIO_NORMAL, /* Normal priority, coming last, going first. */
MODULE_NOTIFIER_PRIO_LIVEPATCH,
MODULE_NOTIFIER_PRIO_FTRACE, /* High priority, coming first, going late. */
};
> };
>
> struct mod_tree_node {
> diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
> index 28d15ba58a26..ce78bb23e24b 100644
> --- a/kernel/livepatch/core.c
> +++ b/kernel/livepatch/core.c
> @@ -1375,13 +1375,40 @@ void *klp_find_section_by_name(const struct module *mod, const char *name,
> }
> EXPORT_SYMBOL_GPL(klp_find_section_by_name);
>
> +static int klp_module_callback(struct notifier_block *nb, unsigned long op,
> + void *module)
> +{
> + struct module *mod = module;
> + int err = 0;
> +
> + switch (op) {
> + case MODULE_STATE_COMING:
> + err = klp_module_coming(mod);
> + break;
> + case MODULE_STATE_LIVE:
> + break;
> + case MODULE_STATE_GOING:
> + klp_module_going(mod);
> + break;
> + default:
> + break;
> + }
klp_module_coming() and klp_module_going() are now used only in
kernel/livepatch/core.c where they are also defined. This means the
functions can be static and their declarations removed from
include/linux/livepatch.h.
Nit: The MODULE_STATE_LIVE and default cases in the switch can be
removed.
> +
> + return notifier_from_errno(err);
> +}
> +
> +static struct notifier_block klp_module_nb = {
> + .notifier_call = klp_module_callback,
> + .priority = MODULE_NOTIFIER_PRIO_SECOND_HIGH
> +};
> +
> static int __init klp_init(void)
> {
> klp_root_kobj = kobject_create_and_add("livepatch", kernel_kobj);
> if (!klp_root_kobj)
> return -ENOMEM;
>
> - return 0;
> + return register_module_notifier(&klp_module_nb);
> }
>
> module_init(klp_init);
> diff --git a/kernel/module/main.c b/kernel/module/main.c
> index c3ce106c70af..226dd5b80997 100644
> --- a/kernel/module/main.c
> +++ b/kernel/module/main.c
> @@ -833,10 +833,8 @@ SYSCALL_DEFINE2(delete_module, const char __user *, name_user,
> /* Final destruction now no one is using it. */
> if (mod->exit != NULL)
> mod->exit();
> - blocking_notifier_call_chain(&module_notify_list,
> + blocking_notifier_call_chain_reverse(&module_notify_list,
> MODULE_STATE_GOING, mod);
> - klp_module_going(mod);
> - ftrace_release_mod(mod);
>
> async_synchronize_full();
>
> @@ -3135,10 +3133,8 @@ static noinline int do_init_module(struct module *mod)
> mod->state = MODULE_STATE_GOING;
> synchronize_rcu();
> module_put(mod);
> - blocking_notifier_call_chain(&module_notify_list,
> + blocking_notifier_call_chain_reverse(&module_notify_list,
> MODULE_STATE_GOING, mod);
> - klp_module_going(mod);
> - ftrace_release_mod(mod);
> free_module(mod);
> wake_up_all(&module_wq);
>
The patch unexpectedly leaves a call to ftrace_free_mem() in
do_init_module().
> @@ -3281,20 +3277,14 @@ static int complete_formation(struct module *mod, struct load_info *info)
> return err;
> }
>
> -static int prepare_coming_module(struct module *mod)
> +static int prepare_module_state_transaction(struct module *mod,
> + unsigned long val_up, unsigned long val_down)
> {
> int err;
>
> - ftrace_module_enable(mod);
> - err = klp_module_coming(mod);
> - if (err)
> - return err;
> -
> err = blocking_notifier_call_chain_robust(&module_notify_list,
> - MODULE_STATE_COMING, MODULE_STATE_GOING, mod);
> + val_up, val_down, mod);
> err = notifier_to_errno(err);
> - if (err)
> - klp_module_going(mod);
>
> return err;
> }
> @@ -3468,14 +3458,21 @@ static int load_module(struct load_info *info, const char __user *uargs,
> init_build_id(mod, info);
>
> /* Ftrace init must be called in the MODULE_STATE_UNFORMED state */
> - ftrace_module_init(mod);
> + err = prepare_module_state_transaction(mod,
> + MODULE_STATE_UNFORMED, MODULE_STATE_FORMED);
I believe val_down should be MODULE_STATE_GOING to reverse the
operation. Why is the new state MODULE_STATE_FORMED needed here?
> + if (err)
> + goto ddebug_cleanup;
>
> /* Finally it's fully formed, ready to start executing. */
> err = complete_formation(mod, info);
> - if (err)
> + if (err) {
> + blocking_notifier_call_chain_reverse(&module_notify_list,
> + MODULE_STATE_FORMED, mod);
> goto ddebug_cleanup;
> + }
>
> - err = prepare_coming_module(mod);
> + err = prepare_module_state_transaction(mod,
> + MODULE_STATE_COMING, MODULE_STATE_GOING);
> if (err)
> goto bug_cleanup;
>
> @@ -3522,7 +3519,6 @@ static int load_module(struct load_info *info, const char __user *uargs,
> destroy_params(mod->kp, mod->num_kp);
> blocking_notifier_call_chain(&module_notify_list,
> MODULE_STATE_GOING, mod);
My understanding is that all notifier chains for MODULE_STATE_GOING
should be reversed.
> - klp_module_going(mod);
> bug_cleanup:
> mod->state = MODULE_STATE_GOING;
> /* module_bug_cleanup needs module_mutex protection */
The patch removes the klp_module_going() cleanup call in load_module().
Similarly, the ftrace_release_mod() call under the ddebug_cleanup label
should be removed and appropriately replaced with a cleanup via
a notifier.
> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> index 8df69e702706..efedb98d3db4 100644
> --- a/kernel/trace/ftrace.c
> +++ b/kernel/trace/ftrace.c
> @@ -5241,6 +5241,44 @@ static int __init ftrace_mod_cmd_init(void)
> }
> core_initcall(ftrace_mod_cmd_init);
>
> +static int ftrace_module_callback(struct notifier_block *nb, unsigned long op,
> + void *module)
> +{
> + struct module *mod = module;
> +
> + switch (op) {
> + case MODULE_STATE_UNFORMED:
> + ftrace_module_init(mod);
> + break;
> + case MODULE_STATE_COMING:
> + ftrace_module_enable(mod);
> + break;
> + case MODULE_STATE_LIVE:
> + ftrace_free_mem(mod, mod->mem[MOD_INIT_TEXT].base,
> + mod->mem[MOD_INIT_TEXT].base + mod->mem[MOD_INIT_TEXT].size);
> + break;
> + case MODULE_STATE_GOING:
> + case MODULE_STATE_FORMED:
> + ftrace_release_mod(mod);
> + break;
> + default:
> + break;
> + }
ftrace_module_init(), ftrace_module_enable(), ftrace_free_mem() and
ftrace_release_mod() should be newly used only in kernel/trace/ftrace.c
where they are also defined. The functions can then be made static and
removed from include/linux/ftrace.h.
Nit: The default case in the switch can be removed.
> +
> + return notifier_from_errno(0);
Nit: This can be simply "return NOTIFY_OK;".
> +}
> +
> +static struct notifier_block ftrace_module_nb = {
> + .notifier_call = ftrace_module_callback,
> + .priority = MODULE_NOTIFIER_PRIO_HIGH
> +};
> +
> +static int __init ftrace_register_module_notifier(void)
> +{
> + return register_module_notifier(&ftrace_module_nb);
> +}
> +core_initcall(ftrace_register_module_notifier);
> +
> static void function_trace_probe_call(unsigned long ip, unsigned long parent_ip,
> struct ftrace_ops *op, struct ftrace_regs *fregs)
> {
--
Thanks,
Petr
^ permalink raw reply
* Re: [PATCH 0/1] rust: module_param: support bool parameters
From: Greg KH @ 2026-04-13 12:25 UTC (permalink / raw)
To: Andreas Hindborg
Cc: Wenzhao Liao, mcgrof, petr.pavlu, da.gomez, samitolvanen, ojeda,
linux-modules, rust-for-linux, atomlin, boqun, gary, bjorn3_gh,
lossin, aliceryhl, tmgross, dakr, linux-kernel
In-Reply-To: <87fr4zvzn2.fsf@kernel.org>
On Mon, Apr 13, 2026 at 02:00:17PM +0200, Andreas Hindborg wrote:
> "Greg KH" <greg@kroah.com> writes:
>
> > On Sat, Apr 11, 2026 at 09:02:53AM -0400, Wenzhao Liao wrote:
> >> Sorry for the earlier noise and for our unfamiliarity with parts of the
> >> kernel submission process, which created extra burden for maintainers.
> >>
> >> This patch adds boolean module parameter support to the Rust `module!`
> >> parameter path.
> >>
> >> It implements `ModuleParam` for `bool` and wires `PARAM_OPS_BOOL` into
> >> the Rust module parameter machinery, so Rust-side parsing reuses the
> >> existing kernel `kstrtobool()` semantics through `kstrtobool_bytes()`
> >> instead of introducing a separate parser. A boolean parameter is also
> >> added to `samples/rust/rust_minimal.rs` as a small reference user and
> >> build-time validation point.
> >
> > What driver needs this feature? Module options should be very rare
> > going forward as they are 1990's technology and do not fit the "modern"
> > kernel model at all.
>
> Rust null block uses module parameters, and was requested to use proper
> boolean parsing rather than overloading u8 parsing for boolean
> parameters [1].
>
> Best regards,
> Andreas Hindborg
>
>
> [1] https://lore.kernel.org/rust-for-linux/abfK4eji5jKSeO_W@google.com/
>
Ok, then that needs to be said somewhere, and ideally, have the code
that uses that as part of the patch series.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH 0/1] rust: module_param: support bool parameters
From: Andreas Hindborg @ 2026-04-13 12:00 UTC (permalink / raw)
To: Greg KH, Wenzhao Liao
Cc: mcgrof, petr.pavlu, da.gomez, samitolvanen, ojeda, linux-modules,
rust-for-linux, atomlin, boqun, gary, bjorn3_gh, lossin,
aliceryhl, tmgross, dakr, linux-kernel
In-Reply-To: <2026041118-croak-serving-ff5e@gregkh>
"Greg KH" <greg@kroah.com> writes:
> On Sat, Apr 11, 2026 at 09:02:53AM -0400, Wenzhao Liao wrote:
>> Sorry for the earlier noise and for our unfamiliarity with parts of the
>> kernel submission process, which created extra burden for maintainers.
>>
>> This patch adds boolean module parameter support to the Rust `module!`
>> parameter path.
>>
>> It implements `ModuleParam` for `bool` and wires `PARAM_OPS_BOOL` into
>> the Rust module parameter machinery, so Rust-side parsing reuses the
>> existing kernel `kstrtobool()` semantics through `kstrtobool_bytes()`
>> instead of introducing a separate parser. A boolean parameter is also
>> added to `samples/rust/rust_minimal.rs` as a small reference user and
>> build-time validation point.
>
> What driver needs this feature? Module options should be very rare
> going forward as they are 1990's technology and do not fit the "modern"
> kernel model at all.
Rust null block uses module parameters, and was requested to use proper
boolean parsing rather than overloading u8 parsing for boolean
parameters [1].
Best regards,
Andreas Hindborg
[1] https://lore.kernel.org/rust-for-linux/abfK4eji5jKSeO_W@google.com/
^ permalink raw reply
* [RFC PATCH 0/2] Decouple ftrace/livepatch from module loader via notifier priority and reverse traversal
From: chensong_2000 @ 2026-04-13 8:01 UTC (permalink / raw)
To: rafael, lenb, mturquette, sboyd, viresh.kumar, agk, snitzer,
mpatocka, bmarzins, song, yukuai, linan122, jason.wessel, danielt,
dianders, horms, davem, edumazet, kuba, pabeni, paulmck, frederic,
mcgrof, petr.pavlu, da.gomez, samitolvanen, atomlin, jpoimboe,
jikos, mbenes, pmladek, joe.lawrence, rostedt, mhiramat,
mark.rutland, mathieu.desnoyers
Cc: linux-modules, linux-kernel, linux-trace-kernel, linux-acpi,
linux-clk, linux-pm, live-patching, dm-devel, linux-raid,
kgdb-bugreport, netdev, Song Chen
From: Song Chen <chensong_2000@189.cn>
This patchset addresses a long-standing tight coupling between the
module loader and two of its key consumers: ftrace and livepatch.
Background:
The module loader currently hard-codes direct calls to
ftrace_module_enable(), klp_module_coming(), klp_module_going() and
ftrace_release_mod() inside prepare_coming_module() and the module
unload path. This hard-coding was necessary because the module notifier
chain could not guarantee the strict call ordering that ftrace and
livepatch require:
During MODULE_STATE_COMING, ftrace must run before livepatch, so
that per-module function records are ready before livepatch registers
its ftrace hooks.
During MODULE_STATE_GOING, livepatch must run before ftrace, so that
livepatch removes its hooks before ftrace releases those records.
This symmetric setup/teardown ordering could not be expressed through
the notifier chain because the chain only supported forward (descending
priority) traversal. Without reverse traversal, it was impossible to
guarantee that the GOING order would be the strict inverse of the
COMING order using a single priority value per notifier.
Patch 1 - notifier: replace single-linked list with double-linked list.
Patch 2 - ftrace/klp: decouple from module loader using notifier
priority.
headsup: somehow the smtp of my mailbox doesn't work very well lately,
if i receive return letter, i have to resend, sorry in advance.
Song Chen (2):
kernel/notifier: replace single-linked list with double-linked list
for reverse traversal
kernel/module: Decouple klp and ftrace from load_module
drivers/acpi/sleep.c | 1 -
drivers/clk/clk.c | 2 +-
drivers/cpufreq/cpufreq.c | 2 +-
drivers/md/dm-integrity.c | 1 -
drivers/md/md.c | 1 -
include/linux/module.h | 8 ++
include/linux/notifier.h | 26 ++---
kernel/debug/debug_core.c | 1 -
kernel/livepatch/core.c | 29 ++++-
kernel/module/main.c | 34 +++---
kernel/notifier.c | 219 ++++++++++++++++++++++++++++++++------
kernel/trace/ftrace.c | 38 +++++++
net/ipv4/nexthop.c | 2 +-
13 files changed, 290 insertions(+), 74 deletions(-)
--
2.43.0
^ permalink raw reply
* Re: [PATCH] kbuild/btf: Remove broken module relinking exclusion
From: Petr Pavlu @ 2026-04-13 7:54 UTC (permalink / raw)
To: Nathan Chancellor, Nicolas Schier, Alexei Starovoitov,
Daniel Borkmann, Andrii Nakryiko
Cc: Martin KaFai Lau, Eduard Zingerman, Song Liu, Yonghong Song,
John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa,
Luis Chamberlain, Daniel Gomez, Sami Tolvanen, Aaron Tomlin,
Ihor Solodrai, Masahiro Yamada, Sasha Levin, linux-kbuild, bpf,
linux-modules, linux-kernel
In-Reply-To: <20260410131343.2519532-1-petr.pavlu@suse.com>
On 4/10/26 3:13 PM, Petr Pavlu wrote:
> Commit 5f9ae91f7c0d ("kbuild: Build kernel module BTFs if BTF is enabled
> and pahole supports it") in 2020 introduced CONFIG_DEBUG_INFO_BTF_MODULES
> to enable generation of split BTF for kernel modules. This change required
> the %.ko Makefile rule to additionally depend on vmlinux, which is used as
> a base for deduplication. The regular ld_ko_o command executed by the rule
> was then modified to be skipped if only vmlinux changes. This was done by
> introducing a new if_changed_except command and updating the original call
> to '+$(call if_changed_except,ld_ko_o,vmlinux)'.
>
> Later, commit 214c0eea43b2 ("kbuild: add $(objtree)/ prefix to some
> in-kernel build artifacts") in 2024 updated the rule's reference to vmlinux
> from 'vmlinux' to '$(objtree)/vmlinux'. This accidentally broke the
> previous logic to skip relinking modules if only vmlinux changes. The issue
> is that '$(objtree)' is typically '.' and GNU Make normalizes the resulting
> prerequisite './vmlinux' to just 'vmlinux', while the exclusion logic
> retains the raw './vmlinux'. As a result, if_changed_except doesn't
> correctly filter out vmlinux. Consequently, with
> CONFIG_DEBUG_INFO_BTF_MODULES=y, modules are relinked even if only vmlinux
> changes.
>
> It is possible to fix this Makefile issue. However, having the %.ko rule
> update the resulting file in place without starting from the original
> inputs is rather fragile. The logic is harder to debug if something breaks
> during a subsequent .ko update because the old input is lost due to the
> overwrite. Additionally, it requires that the BTF processing is idempotent.
> For example, sorting id+flags BTF_SET8 pairs in .BTF_ids by resolve_btfids
> currently doesn't have this property.
>
> One option is to split the %.ko target into two rules: the first for
> partial linking and the second one for generating the BTF data. However,
> this approach runs into an issue with requiring additional intermediate
> files, which increases the size of the build directory. On my system, when
> using a large distribution config with ~5500 modules, the size of the build
> directory with debuginfo enabled is already ~25 GB, with .ko files
> occupying ~8 GB. Duplicating these .ko files doesn't seem practical.
>
> Measuring the speed of the %.ko processing shows that the link step is
> actually relatively fast. It takes about 20% of the overall rule time,
> while the BTF processing accounts for 80%. Moreover, skipping the link part
> becomes relevant only during local development. In such cases, developers
> typically use configs that enable a limited number of modules, so having
> the %.ko rule slightly slower doesn't significantly impact the total
> rebuild time. This is supported by the fact that no one has complained
> about this optimization being broken for the past two years.
>
> Therefore, remove the logic that prevents module relinking when only
> vmlinux changes and simplify Makefile.modfinal.
>
> Signed-off-by: Petr Pavlu <petr.pavlu@suse.com>
> ---
> My previous attempt to fix this logic can be found at
> https://lore.kernel.org/linux-modules/20260402141911.1577711-1-petr.pavlu@suse.com/
> ---
> scripts/Makefile.modfinal | 10 +---------
> 1 file changed, 1 insertion(+), 9 deletions(-)
>
> diff --git a/scripts/Makefile.modfinal b/scripts/Makefile.modfinal
> index adcbcde16a07..01a37ec872b9 100644
> --- a/scripts/Makefile.modfinal
> +++ b/scripts/Makefile.modfinal
> @@ -46,17 +46,9 @@ quiet_cmd_btf_ko = BTF [M] $@
> $(CONFIG_SHELL) $(srctree)/scripts/gen-btf.sh --btf_base $(objtree)/vmlinux $@; \
> fi;
>
> -# Same as newer-prereqs, but allows to exclude specified extra dependencies
> -newer_prereqs_except = $(filter-out $(PHONY) $(1),$?)
> -
> -# Same as if_changed, but allows to exclude specified extra dependencies
> -if_changed_except = $(if $(call newer_prereqs_except,$(2))$(cmd-check), \
> - $(cmd); \
> - printf '%s\n' 'savedcmd_$@ := $(make-cmd)' > $(dot-target).cmd, @:)
> -
> # Re-generate module BTFs if either module's .ko or vmlinux changed
> %.ko: %.o %.mod.o .module-common.o $(objtree)/scripts/module.lds $(and $(CONFIG_DEBUG_INFO_BTF_MODULES),$(KBUILD_BUILTIN),$(objtree)/vmlinux) FORCE
> - +$(call if_changed_except,ld_ko_o,$(objtree)/vmlinux)
> + +$(call if_changed,ld_ko_o)
> ifdef CONFIG_DEBUG_INFO_BTF_MODULES
> +$(if $(newer-prereqs),$(call cmd,btf_ko))
> endif
>
> base-commit: 591cd656a1bf5ea94a222af5ef2ee76df029c1d2
The Sashiko review system makes the following observation [1]:
> Will this result in missing BTF data if the module is rebuilt solely due
> to a command-line change?
>
> Since Kbuild's if_changed executes when either a prerequisite is newer or
> the command line has changed, a command-line-only change (such as modifying
> LDFLAGS_MODULE) will trigger ld_ko_o. This overwrites the .ko file with a
> freshly linked object that lacks a BTF section.
>
> However, because no prerequisite files actually changed, Make's $? variable
> is empty, meaning newer-prereqs is also empty. The subsequent check
> $(if $(newer-prereqs),$(call cmd,btf_ko)) would then evaluate to false and
> skip generating the BTF data entirely.
>
> Could these commands be unified under a single custom Kbuild rule
> (e.g., rule_ld_ko_o) and invoked together using $(call if_changed_rule,ld_ko_o)
> to ensure both linking and BTF generation occur atomically?
I'm aware of this issue. However, it is a separate problem that has
already been present. I'd like to address the exclusion rule first.
[1] https://sashiko.dev/#/patchset/20260410131343.2519532-1-petr.pavlu%40suse.com
-- Petr
^ permalink raw reply
* [GIT PULL] Modules changes for v7.1-rc1
From: Sami Tolvanen @ 2026-04-13 0:26 UTC (permalink / raw)
To: Linus Torvalds
Cc: Sami Tolvanen, Aaron Tomlin, Daniel Gomez, Joe Lawrence,
linux-kernel, linux-modules, Lucas De Marchi, Luis Chamberlain,
Nicholas Sielicki, Petr Pavlu, Siddharth Nayyar,
Thomas Weißschuh
The following changes since commit f338e77383789c0cae23ca3d48adcc5e9e137e3c:
Linux 7.0-rc4 (2026-03-15 13:52:05 -0700)
are available in the Git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/modules/linux.git tags/modules-7.1-rc1
for you to fetch changes up to 663385f9155f27892a97a5824006f806a32eb8dc:
module: Simplify warning on positive returns from module_init() (2026-04-04 00:04:48 +0000)
----------------------------------------------------------------
Modules changes for v7.1-rc1
Kernel symbol flags:
- Replace the separate *_gpl symbol sections (__ksymtab_gpl and
__kcrctab_gpl) with a unified symbol table and a new
__kflagstab section. This section stores symbol flags, such as
the GPL-only flag, as an 8-bit bitset for each exported symbol.
This is a cleanup that simplifies symbol lookup in the module
loader by avoiding table fragmentation and will allow a cleaner
way to add more flags later if needed.
Module signature UAPI:
- Move struct module_signature to the UAPI headers to allow reuse
by tools outside the kernel proper, such as kmod and
scripts/sign-file. This also renames a few constants for clarity
and drops unused signature types as preparation for hash-based
module integrity checking work that's in progress.
Sysfs:
- Add a /sys/module/<module>/import_ns sysfs attribute to show
the symbol namespaces imported by loaded modules. This makes it
easier to verify driver API access at runtime on systems that
care about such things (e.g. Android).
Cleanups and fixes:
- Force sh_addr to 0 for all sections in module.lds. This prevents
non-zero section addresses when linking modules with ld.bfd -r,
which confused elfutils.
- Fix a memory leak of charp module parameters on module unload
when the kernel is configured with CONFIG_SYSFS=n.
- Override the -EEXIST error code returned by module_init() to
userspace. This prevents confusion with the errno reserved by
the module loader to indicate that a module is already loaded.
- Simplify the warning message and drop the stack dump on positive
returns from module_init().
- Drop unnecessary extern keywords from function declarations and
synchronize parse_args() arguments with their implementation.
Signed-off-by: Sami Tolvanen <samitolvanen@google.com>
----------------------------------------------------------------
Conflicts:
There is a small merge conflict in scripts/module.lds.S between the
parisc tree, where the commit
eab5a8054a5cf ("module.lds.S: Fix modules on 32-bit parisc architecture")
conflicts with
4afc71bba8b7 ("module.lds,codetag: force 0 sh_addr for sections")
----------------------------------------------------------------
Joe Lawrence (1):
module.lds,codetag: force 0 sh_addr for sections
Lucas De Marchi (2):
module: Override -EEXIST module return
module: Simplify warning on positive returns from module_init()
Nicholas Sielicki (2):
module: expose imported namespaces via sysfs
docs: symbol-namespaces: mention sysfs attribute
Petr Pavlu (3):
module: Fix freeing of charp module parameters when CONFIG_SYSFS=n
module: Clean up parse_args() arguments
module: Remove extern keyword from param prototypes
Siddharth Nayyar (7):
module: define ksym_flags enumeration to represent kernel symbol flags
module: add kflagstab section to vmlinux and modules
module: populate kflagstab in modpost
module: use kflagstab instead of *_gpl sections
module: deprecate usage of *_gpl sections in module loader
module: remove *_gpl sections from vmlinux and modules
documentation: remove references to *_gpl sections
Thomas Weißschuh (8):
extract-cert: drop unused definition of PKEY_ID_PKCS7
module: Drop unused signature types
module: Give 'enum pkey_id_type' a more specific name
module: Give MODULE_SIG_STRING a more descriptive name
module: Move 'struct module_signature' to UAPI
tools uapi headers: add linux/module_signature.h
sign-file: use 'struct module_signature' from the UAPI headers
selftests/bpf: verify_pkcs7_sig: Use 'struct module_signature' from the UAPI headers
Documentation/ABI/testing/sysfs-module | 9 +
Documentation/core-api/symbol-namespaces.rst | 5 +
Documentation/kbuild/modules.rst | 11 +-
arch/s390/kernel/machine_kexec_file.c | 6 +-
certs/extract-cert.c | 2 -
include/asm-generic/codetag.lds.h | 2 +-
include/asm-generic/vmlinux.lds.h | 21 +--
include/linux/export-internal.h | 28 +--
include/linux/module.h | 5 +-
include/linux/module_signature.h | 30 +---
include/linux/module_symbol.h | 5 +
include/linux/moduleparam.h | 100 +++++------
include/uapi/linux/module_signature.h | 41 +++++
kernel/module/internal.h | 4 +-
kernel/module/main.c | 192 ++++++++++++++-------
kernel/module/signing.c | 4 +-
kernel/module_signature.c | 2 +-
kernel/params.c | 29 ++--
scripts/Makefile | 1 +
scripts/mod/modpost.c | 16 +-
scripts/module.lds.S | 15 +-
scripts/sign-file.c | 19 +-
security/integrity/ima/ima_modsig.c | 6 +-
tools/include/uapi/linux/module_signature.h | 41 +++++
tools/testing/selftests/bpf/Makefile | 1 +
.../selftests/bpf/prog_tests/verify_pkcs7_sig.c | 28 +--
26 files changed, 368 insertions(+), 255 deletions(-)
create mode 100644 include/uapi/linux/module_signature.h
create mode 100644 tools/include/uapi/linux/module_signature.h
^ permalink raw reply
* Re: [PATCH] kernel/trace/ftrace: introduce ftrace module notifier
From: Song Chen @ 2026-04-12 14:10 UTC (permalink / raw)
To: Petr Mladek
Cc: Steven Rostedt, Miroslav Benes, mcgrof, petr.pavlu, da.gomez,
samitolvanen, atomlin, mhiramat, mark.rutland, mathieu.desnoyers,
linux-modules, linux-kernel, linux-trace-kernel, live-patching
In-Reply-To: <aaqk-GrpCTqO36xj@pathway.suse.cz>
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
>
>
^ permalink raw reply
* Re: [PATCH 0/1] rust: module_param: support bool parameters
From: Greg KH @ 2026-04-11 13:20 UTC (permalink / raw)
To: Wenzhao Liao
Cc: mcgrof, petr.pavlu, da.gomez, samitolvanen, ojeda, linux-modules,
rust-for-linux, atomlin, boqun, gary, bjorn3_gh, lossin,
a.hindborg, aliceryhl, tmgross, dakr, linux-kernel
In-Reply-To: <20260411130254.3510128-1-wenzhaoliao@ruc.edu.cn>
On Sat, Apr 11, 2026 at 09:02:53AM -0400, Wenzhao Liao wrote:
> Sorry for the earlier noise and for our unfamiliarity with parts of the
> kernel submission process, which created extra burden for maintainers.
>
> This patch adds boolean module parameter support to the Rust `module!`
> parameter path.
>
> It implements `ModuleParam` for `bool` and wires `PARAM_OPS_BOOL` into
> the Rust module parameter machinery, so Rust-side parsing reuses the
> existing kernel `kstrtobool()` semantics through `kstrtobool_bytes()`
> instead of introducing a separate parser. A boolean parameter is also
> added to `samples/rust/rust_minimal.rs` as a small reference user and
> build-time validation point.
What driver needs this feature? Module options should be very rare
going forward as they are 1990's technology and do not fit the "modern"
kernel model at all.
thanks,
greg k-h
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox