* [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 v2 4/5] md: skip ID_BITMAP_NONE when show available bitmap types
From: Li Nan @ 2026-04-13 8:15 UTC (permalink / raw)
To: Su Yue, linux-raid; +Cc: song, xni, yukuai, heming.zhao, l
In-Reply-To: <20260407102625.5686-5-glass.su@suse.com>
在 2026/4/7 18:26, Su Yue 写道:
> As none_bitmap_ops is introduced, ID_BITMAP_NONE should be skipped while
> iterating md submodules, otherwise:
>
> $ cat /sys/block/md0/md/bitmap_type
> [none] bitmap llbitmap [none]
>
> Signed-off-by: Su Yue <glass.su@suse.com>
> ---
> drivers/md/md.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index 55a95b227b83..20a953676319 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -4269,6 +4269,8 @@ bitmap_type_show(struct mddev *mddev, char *page)
> xa_for_each(&md_submodule, i, head) {
> if (head->type != MD_BITMAP)
> continue;
> + if (head->id == ID_BITMAP_NONE)
> + continue;
>
> if (mddev->bitmap_id == head->id)
> len += sprintf(page + len, "[%s] ", head->name);
If we indeed add ID_BITMAP_NONE as patch 3, would it be better to delete
this piece of code instead?
if (mddev->bitmap_id == ID_BITMAP_NONE)
len += sprintf(page + len, "[none] ");
else
len += sprintf(page + len, "none ");
--
Thanks,
Nan
^ permalink raw reply
* Re: [PATCH v2 1/5] md/md-bitmap: call md_bitmap_create,destroy in location_store
From: Su Yue @ 2026-04-13 10:18 UTC (permalink / raw)
To: Li Nan; +Cc: Su Yue, linux-raid, song, xni, yukuai, heming.zhao
In-Reply-To: <5fae3215-4280-fc77-f9e0-2f607176fcd3@huaweicloud.com>
aOn Mon 13 Apr 2026 at 15:47, Li Nan <linan666@huaweicloud.com>
wrote:
> 在 2026/4/7 18:26, Su Yue 写道:
>> If bitmap/location is present, mdadm will call
>> update_array_info()
>> while growing bitmap from none to internal via
>> location_store().
>> md_bitmap_create() is needed to set mddev->bitmap_ops otherwise
>> mddev->bitmap_ops->get_stats() in update_array_info() will
>> trigger
>> kernel NULL pointer dereference.
>> Fixes: fb8cc3b0d9db ("md/md-bitmap: delay registration of
>> bitmap_ops until
>> creating bitmap")
>> Signed-off-by: Su Yue <glass.su@suse.com>
>> ---
>> drivers/md/md-bitmap.c | 11 ++++++++---
>> drivers/md/md.c | 4 ++--
>> drivers/md/md.h | 2 ++
>> 3 files changed, 12 insertions(+), 5 deletions(-)
>> diff --git a/drivers/md/md-bitmap.c b/drivers/md/md-bitmap.c
>> index 83378c033c72..2f24aae05552 100644
>> --- a/drivers/md/md-bitmap.c
>> +++ b/drivers/md/md-bitmap.c
>> @@ -2618,7 +2618,7 @@ location_store(struct mddev *mddev, const
>> char *buf, size_t len)
>> goto out;
>> }
>> - bitmap_destroy(mddev);
>> + md_bitmap_destroy(mddev);
>> mddev->bitmap_info.offset = 0;
>> if (mddev->bitmap_info.file) {
>> struct file *f = mddev->bitmap_info.file;
>> @@ -2653,15 +2653,20 @@ location_store(struct mddev *mddev,
>> const char *buf, size_t len)
>> goto out;
>> }
>> + /*
>> + * lockless bitmap shoudle have set bitmap_id
>> + * using bitmap_type, so always ID_BITMAP.
>> + */
>> + mddev->bitmap_id = ID_BITMAP;
>> mddev->bitmap_info.offset = offset;
>> - rv = bitmap_create(mddev);
>> + rv = md_bitmap_create(mddev);
>> if (rv)
>> goto out;
>> rv = bitmap_load(mddev);
>
> mddev->bitmap_ops->load() should also be used here.
/NOD.
location_store() is only used for ID_BITMAP_NONE and ID_BITMAP, so
mddev->bitmap_ops->load() is always bitmap_load().
But for code consistency, mddev->bitmap_ops->load() is better,
will fix it.
--
Su
^ permalink raw reply
* Re: [PATCH v2 4/5] md: skip ID_BITMAP_NONE when show available bitmap types
From: Su Yue @ 2026-04-13 10:23 UTC (permalink / raw)
To: Li Nan; +Cc: Su Yue, linux-raid, song, xni, yukuai, heming.zhao
In-Reply-To: <4be300df-3943-acf3-ace1-cf6316f2c936@huaweicloud.com>
On Mon 13 Apr 2026 at 16:15, Li Nan <linan666@huaweicloud.com>
wrote:
> 在 2026/4/7 18:26, Su Yue 写道:
>> As none_bitmap_ops is introduced, ID_BITMAP_NONE should be
>> skipped while
>> iterating md submodules, otherwise:
>> $ cat /sys/block/md0/md/bitmap_type
>> [none] bitmap llbitmap [none]
>> Signed-off-by: Su Yue <glass.su@suse.com>
>> ---
>> drivers/md/md.c | 2 ++
>> 1 file changed, 2 insertions(+)
>> diff --git a/drivers/md/md.c b/drivers/md/md.c
>> index 55a95b227b83..20a953676319 100644
>> --- a/drivers/md/md.c
>> +++ b/drivers/md/md.c
>> @@ -4269,6 +4269,8 @@ bitmap_type_show(struct mddev *mddev,
>> char *page)
>> xa_for_each(&md_submodule, i, head) {
>> if (head->type != MD_BITMAP)
>> continue;
>> + if (head->id == ID_BITMAP_NONE)
>> + continue;
>> if (mddev->bitmap_id == head->id)
>> len += sprintf(page + len, "[%s] ", head->name);
>
> If we indeed add ID_BITMAP_NONE as patch 3, would it be better
> to delete
> this piece of code instead?
>
I dont' think so. There is one case CONFIG_MD_BITMAP is not
enabled which
means register_md_submodule(&none_bitmap_ops.head) is not called
but in
mddev_init():
int mddev_init(struct mddev *mddev)
|
{
|
int err = 0;
|
|
if (!IS_ENABLED(CONFIG_MD_BITMAP))
|
mddev->bitmap_id = ID_BITMAP_NONE;
...
}
--
Su
>
> if (mddev->bitmap_id == ID_BITMAP_NONE)
> len += sprintf(page + len, "[none] ");
> else
> len += sprintf(page + len, "none ");
^ permalink raw reply
* [PATCH v2] md: fix kobject reference leak in md_import_device()
From: Guangshuo Li @ 2026-04-13 14:17 UTC (permalink / raw)
To: Song Liu, Yu Kuai, Greg Kroah-Hartman, linux-raid, linux-kernel
Cc: Guangshuo Li, stable
md_import_device() initializes rdev->kobj with kobject_init() before
checking the device size and loading the superblock.
When one of the later checks fails, the error path still frees rdev
directly with kfree(). This bypasses the kobject release path and leaves
the kobject reference unbalanced.
The issue was identified by a static analysis tool I developed and
confirmed by manual review.
After kobject_init(), release rdev through kobject_put() instead of
kfree().
Fixes: f9cb074bff8e ("Kobject: rename kobject_init_ng() to kobject_init()")
Cc: stable@vger.kernel.org
Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
---
v2:
- note that the issue was identified by my static analysis tool
- and confirmed by manual review
drivers/md/md.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/md/md.c b/drivers/md/md.c
index 6d73f6e196a9..4ce7512dc834 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -3871,6 +3871,9 @@ static struct md_rdev *md_import_device(dev_t newdev, int super_format, int supe
out_blkdev_put:
fput(rdev->bdev_file);
+ md_rdev_clear(rdev);
+ kobject_put(&rdev->kobj);
+ return ERR_PTR(err);
out_clear_rdev:
md_rdev_clear(rdev);
out_free_rdev:
--
2.43.0
^ permalink raw reply related
* [PATCH] md/raid5: Don't set bi_status on STRIPE_WAIT_RESHAPE
From: Benjamin Marzinski @ 2026-04-13 22:45 UTC (permalink / raw)
To: Yu Kuai, Song Liu, Li Nan; +Cc: linux-raid, dm-devel, Xiao Ni, Nigel Croxon
When make_stripe_request() encounters a clone bio that crosses the
reshape position while the reshape cannot make progress, it was setting
bi->bi_status to BLK_STS_RESOURCE when returning STRIPE_WAIT_RESHAPE.
This will update the original bio's bi_status in md_end_clone_io().
Afterwards, md_handle_request() will wait for the device to become
unsuspended and submit a new cloned bio. However, even if that clone
completes successfully, it will not clear the original bio's bi_status.
There's no need to set bi_status when retrying the bio. md will already
error out the bio correctly if it is set REQ_NOWAIT. Otherwise it will
be retried. dm-raid will already end the bio with DM_MAPIO_REQUEUE.
Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
drivers/md/raid5.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index dc0c680ca199..690c65cd1e29 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -6042,7 +6042,6 @@ static enum stripe_result make_stripe_request(struct mddev *mddev,
raid5_release_stripe(sh);
out:
if (ret == STRIPE_SCHEDULE_AND_RETRY && reshape_interrupted(mddev)) {
- bi->bi_status = BLK_STS_RESOURCE;
ret = STRIPE_WAIT_RESHAPE;
pr_err_ratelimited("dm-raid456: io across reshape position while reshape can't make progress");
}
--
2.53.0
^ permalink raw reply related
* Re: [PATCH] md/raid5: Don't set bi_status on STRIPE_WAIT_RESHAPE
From: Li Nan @ 2026-04-14 1:25 UTC (permalink / raw)
To: Benjamin Marzinski, Yu Kuai, Song Liu
Cc: linux-raid, dm-devel, Xiao Ni, Nigel Croxon
In-Reply-To: <20260413224556.1914504-1-bmarzins@redhat.com>
在 2026/4/14 6:45, Benjamin Marzinski 写道:
> When make_stripe_request() encounters a clone bio that crosses the
> reshape position while the reshape cannot make progress, it was setting
> bi->bi_status to BLK_STS_RESOURCE when returning STRIPE_WAIT_RESHAPE.
> This will update the original bio's bi_status in md_end_clone_io().
> Afterwards, md_handle_request() will wait for the device to become
> unsuspended and submit a new cloned bio. However, even if that clone
> completes successfully, it will not clear the original bio's bi_status.
>
> There's no need to set bi_status when retrying the bio. md will already
> error out the bio correctly if it is set REQ_NOWAIT. Otherwise it will
> be retried. dm-raid will already end the bio with DM_MAPIO_REQUEUE.
>
> Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
> ---
> drivers/md/raid5.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> index dc0c680ca199..690c65cd1e29 100644
> --- a/drivers/md/raid5.c
> +++ b/drivers/md/raid5.c
> @@ -6042,7 +6042,6 @@ static enum stripe_result make_stripe_request(struct mddev *mddev,
> raid5_release_stripe(sh);
> out:
> if (ret == STRIPE_SCHEDULE_AND_RETRY && reshape_interrupted(mddev)) {
> - bi->bi_status = BLK_STS_RESOURCE;
> ret = STRIPE_WAIT_RESHAPE;
> pr_err_ratelimited("dm-raid456: io across reshape position while reshape can't make progress");
> }
The link below leads to the same patch, which Kuai has already replied to.
https://lore.kernel.org/all/20260203095156.2349174-1-yangxiuwei@kylinos.cn/
--
Thanks,
Nan
^ permalink raw reply
* Re: [PATCH v2] md: fix kobject reference leak in md_import_device()
From: Su Yue @ 2026-04-14 1:28 UTC (permalink / raw)
To: Guangshuo Li
Cc: Song Liu, Yu Kuai, Greg Kroah-Hartman, linux-raid, linux-kernel,
stable
In-Reply-To: <20260413141759.2970973-1-lgs201920130244@gmail.com>
On Mon 13 Apr 2026 at 22:17, Guangshuo Li
<lgs201920130244@gmail.com> wrote:
> md_import_device() initializes rdev->kobj with kobject_init()
> before
> checking the device size and loading the superblock.
>
> When one of the later checks fails, the error path still frees
> rdev
> directly with kfree(). This bypasses the kobject release path
> and leaves
> the kobject reference unbalanced.
>
> The issue was identified by a static analysis tool I developed
> and
> confirmed by manual review.
>
> After kobject_init(), release rdev through kobject_put() instead
> of
> kfree().
>
> Fixes: f9cb074bff8e ("Kobject: rename kobject_init_ng() to
> kobject_init()")
> Cc: stable@vger.kernel.org
> Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
> ---
> v2:
> - note that the issue was identified by my static analysis
> tool
> - and confirmed by manual review
>
> drivers/md/md.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index 6d73f6e196a9..4ce7512dc834 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -3871,6 +3871,9 @@ static struct md_rdev
> *md_import_device(dev_t newdev, int super_format, int supe
>
> out_blkdev_put:
> fput(rdev->bdev_file);
> + md_rdev_clear(rdev);
> + kobject_put(&rdev->kobj);
> + return ERR_PTR(err);
>
Why not just:
out_blkdev_put:
kobject_put(&rdev->kobj);
fput(rdev->bdev_file);
out_clear_rdev:
md_rdev_clear(rdev);
out_free_rdev:
kfree(rdev);
return ERR_PTR(err);
--
Su
^ permalink raw reply
* [PATCH 1/1] MAINTAINERS: Add Xiao Ni as md/raid reviewer
From: Xiao Ni @ 2026-04-14 2:29 UTC (permalink / raw)
To: song, yukuai; +Cc: linan666, linux-raid, linux-kernel
From: Xiao Ni <xiao@kernel.org>
I've been actively involved in the md subsystem, contributing bug
fixes, performance improvements, and participating in code reviews.
I will help improve patch review coverage and response time.
Signed-off-by: Xiao Ni <xiao@kernel.org>
---
MAINTAINERS | 1 +
1 file changed, 1 insertion(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index 3adc870d523b..dca564db7cfa 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -24562,6 +24562,7 @@ SOFTWARE RAID (Multiple Disks) SUPPORT
M: Song Liu <song@kernel.org>
M: Yu Kuai <yukuai@fnnas.com>
R: Li Nan <linan122@huawei.com>
+R: Xiao Ni <xiao@kernel.org>
L: linux-raid@vger.kernel.org
S: Supported
Q: https://patchwork.kernel.org/project/linux-raid/list/
--
2.50.1 (Apple Git-155)
^ permalink raw reply related
* Re: [PATCH V2 2/2] nvme-multipath: enable PCI P2PDMA for multipath devices
From: Chaitanya Kulkarni @ 2026-04-14 2:57 UTC (permalink / raw)
To: Christoph Hellwig, Chaitanya Kulkarni
Cc: song@kernel.org, yukuai@fnnas.com, linan122@huawei.com,
kbusch@kernel.org, axboe@kernel.dk, sagi@grimberg.me,
linux-raid@vger.kernel.org, linux-nvme@lists.infradead.org,
Kiran Modukuri
In-Reply-To: <20260409062659.GA7335@lst.de>
On 4/8/26 23:26, Christoph Hellwig wrote:
> On Wed, Apr 08, 2026 at 12:25:37AM -0700, Chaitanya Kulkarni wrote:
>> From: Kiran Kumar Modukuri <kmodukuri@nvidia.com>
>>
>> NVMe multipath does not expose BLK_FEAT_PCI_P2PDMA on the head disk
>> even when the underlying controller supports it.
>>
>> Set BLK_FEAT_PCI_P2PDMA in nvme_mpath_alloc_disk() when the controller
>> advertises P2PDMA support via ctrl->ops->supports_pci_p2pdma.
>>
>> Since multipath can match paths across different transports (e.g. PCIe
>> and FC), not all paths are guaranteed to support P2PDMA. Clear
>> BLK_FEAT_PCI_P2PDMA from the head disk in nvme_mpath_add_disk() if the
>> newly added path does not support it, ensuring the feature is only
>> advertised when every member supports it.
>>
>> Signed-off-by: Kiran Kumar Modukuri <kmodukuri@nvidia.com>
>> Signed-off-by: Chaitanya Kulkarni <kch@nvidia.com>
>> ---
>> drivers/nvme/host/multipath.c | 18 ++++++++++++++++++
>> 1 file changed, 18 insertions(+)
>>
>> diff --git a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipath.c
>> index ba00f0b72b85..48d920ce803f 100644
>> --- a/drivers/nvme/host/multipath.c
>> +++ b/drivers/nvme/host/multipath.c
>> @@ -737,6 +737,9 @@ int nvme_mpath_alloc_disk(struct nvme_ctrl *ctrl, struct nvme_ns_head *head)
>> BLK_FEAT_POLL | BLK_FEAT_ATOMIC_WRITES;
>> if (head->ids.csi == NVME_CSI_ZNS)
>> lim.features |= BLK_FEAT_ZONED;
>> + if (ctrl->ops && ctrl->ops->supports_pci_p2pdma &&
>> + ctrl->ops->supports_pci_p2pdma(ctrl))
>> + lim.features |= BLK_FEAT_PCI_P2PDMA;
> I think we can just add this unconditionally here, similar to all the
> other feaures above the ZNS conditional as any non-supporting controller
> will clear it later.
>
>>
>> void nvme_mpath_add_disk(struct nvme_ns *ns, __le32 anagrpid)
>> {
>> + struct nvme_ns_head *head = ns->head;
>> +
>> + /*
>> + * Clear BLK_FEAT_PCI_P2PDMA on the head disk if this path does not
>> + * support it. Multipath may span different transports (e.g. PCIe and
>> + * FC), so every member must support P2PDMA for it to be safe on the
>> + * head disk.
>> + */
>> + if (head->disk && !blk_queue_pci_p2pdma(ns->queue)) {
>> + struct queue_limits lim =
>> + queue_limits_start_update(head->disk->queue);
>> + lim.features &= ~BLK_FEAT_PCI_P2PDMA;
>> + queue_limits_commit_update(head->disk->queue, &lim);
>> + }
> And this really should go into the core block code in blk_stack_limits,
> so that BLK_FEAT_PCI_P2PDMA is cleared whenever a non-confirming
> device is added, similar to BLK_FEAT_NOWAIT and BLK_FEAT_POLL.
>
I've a V3 with all the comments fixed including the prep patch
to move the above into the block layer.
Waiting to get it tested on with Kiran's setup to generate the test log.
-ck
^ permalink raw reply
* Re: [PATCH 1/1] MAINTAINERS: Add Xiao Ni as md/raid reviewer
From: Paul Menzel @ 2026-04-14 6:18 UTC (permalink / raw)
To: Xiao Ni; +Cc: song, yukuai, linan666, linux-raid, linux-kernel
In-Reply-To: <20260414022956.48271-1-xiaoraid25@gmail.com>
Dear Xiao,
Am 14.04.26 um 04:29 schrieb Xiao Ni:
> From: Xiao Ni <xiao@kernel.org>
>
> I've been actively involved in the md subsystem, contributing bug
> fixes, performance improvements, and participating in code reviews.
> I will help improve patch review coverage and response time.
Thank you for doing this.
> Signed-off-by: Xiao Ni <xiao@kernel.org>
> ---
> MAINTAINERS | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 3adc870d523b..dca564db7cfa 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -24562,6 +24562,7 @@ SOFTWARE RAID (Multiple Disks) SUPPORT
> M: Song Liu <song@kernel.org>
> M: Yu Kuai <yukuai@fnnas.com>
> R: Li Nan <linan122@huawei.com>
> +R: Xiao Ni <xiao@kernel.org>
> L: linux-raid@vger.kernel.org
> S: Supported
> Q: https://patchwork.kernel.org/project/linux-raid/list/
Reviewed-by: Paul Menzel <pmenzel@molgen.mpg.de>
Kind regards,
Paul
^ permalink raw reply
* Re: [PATCH] md/raid5: Don't set bi_status on STRIPE_WAIT_RESHAPE
From: Yu Kuai @ 2026-04-14 6:20 UTC (permalink / raw)
To: Li Nan, Benjamin Marzinski, Yu Kuai, Song Liu, yukuai
Cc: linux-raid, dm-devel, Xiao Ni, Nigel Croxon
In-Reply-To: <0e53701a-2471-9b72-eef8-27b168a0f3ea@huaweicloud.com>
Hi,
在 2026/4/14 9:25, Li Nan 写道:
>
>
> 在 2026/4/14 6:45, Benjamin Marzinski 写道:
>> When make_stripe_request() encounters a clone bio that crosses the
>> reshape position while the reshape cannot make progress, it was setting
>> bi->bi_status to BLK_STS_RESOURCE when returning STRIPE_WAIT_RESHAPE.
>> This will update the original bio's bi_status in md_end_clone_io().
>> Afterwards, md_handle_request() will wait for the device to become
>> unsuspended and submit a new cloned bio. However, even if that clone
>> completes successfully, it will not clear the original bio's bi_status.
>>
>> There's no need to set bi_status when retrying the bio. md will already
>> error out the bio correctly if it is set REQ_NOWAIT. Otherwise it will
>> be retried. dm-raid will already end the bio with DM_MAPIO_REQUEUE.
>>
>> Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
>> ---
>> drivers/md/raid5.c | 1 -
>> 1 file changed, 1 deletion(-)
>>
>> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
>> index dc0c680ca199..690c65cd1e29 100644
>> --- a/drivers/md/raid5.c
>> +++ b/drivers/md/raid5.c
>> @@ -6042,7 +6042,6 @@ static enum stripe_result
>> make_stripe_request(struct mddev *mddev,
>> raid5_release_stripe(sh);
>> out:
>> if (ret == STRIPE_SCHEDULE_AND_RETRY &&
>> reshape_interrupted(mddev)) {
>> - bi->bi_status = BLK_STS_RESOURCE;
>> ret = STRIPE_WAIT_RESHAPE;
>> pr_err_ratelimited("dm-raid456: io across reshape position
>> while reshape can't make progress");
>> }
>
> The link below leads to the same patch, which Kuai has already replied
> to.
>
> https://lore.kernel.org/all/20260203095156.2349174-1-yangxiuwei@kylinos.cn/
Perhaps instead of clearing the error code from error path, this problem can be fixed
by resetting the error code from the issue path if original bio is resubmitted.
>
>
> --
> Thanks,
> Nan
>
>
--
Thansk,
Kuai
^ permalink raw reply
* Re: [PATCH] md/raid5: fix race between reshape and chunk-aligned read
From: FengWei Shih @ 2026-04-14 8:27 UTC (permalink / raw)
To: Li Nan, song, yukuai; +Cc: linux-raid, linux-kernel, yangerkun@huawei.com
In-Reply-To: <f032ef85-8cd3-1615-3c42-e434011b0056@huaweicloud.com>
Hi Nan,
Li Nan 於 2026/4/13 下午 03:19 寫道:
>
>
> 在 2026/4/9 13:17, FengWei Shih 写道:
>> raid5_make_request() checks mddev->reshape_position to decide whether
>> to allow chunk-aligned reads. However in raid5_start_reshape(), the
>> layout configuration (raid_disks, algorithm, etc.) is updated before
>> mddev->reshape_position is set:
>>
>> reshape (raid5_start_reshape) read (raid5_make_request)
>> ============================== ===========================
>> write_seqcount_begin
>> update raid_disks, algorithm...
>> set conf->reshape_progress
>> write_seqcount_end
>> check mddev->reshape_position
>> * still MaxSector, allow
>> raid5_read_one_chunk()
>> * use new layout
>> raid5_quiesce()
>> set mddev->reshape_position
>>
>> Since reshape_position is not yet updated, raid5_make_request()
>> considers no reshape is in progress and proceeds with the
>> chunk-aligned path, but the layout has already changed, causing
>> raid5_compute_sector() to return an incorrect physical address.
>>
>> Fix this by reading conf->reshape_progress under gen_lock in
>> raid5_read_one_chunk() and falling back to the stripe path if a
>> reshape is in progress.
>>
>> Signed-off-by: FengWei Shih <dannyshih@synology.com>
>> ---
>> drivers/md/raid5.c | 8 ++++++++
>> 1 file changed, 8 insertions(+)
>>
>> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
>> index a8e8d431071b..bded2b86f0ef 100644
>> --- a/drivers/md/raid5.c
>> +++ b/drivers/md/raid5.c
>> @@ -5421,6 +5421,11 @@ static int raid5_read_one_chunk(struct mddev
>> *mddev, struct bio *raid_bio)
>> sector_t sector, end_sector;
>> int dd_idx;
>> bool did_inc;
>> + int seq;
>> +
>> + seq = read_seqcount_begin(&conf->gen_lock);
>> + if (unlikely(conf->reshape_progress != MaxSector))
>> + return 0;
>> if (!in_chunk_boundary(mddev, raid_bio)) {
>> pr_debug("%s: non aligned\n", __func__);
>> @@ -5431,6 +5436,9 @@ static int raid5_read_one_chunk(struct mddev
>> *mddev, struct bio *raid_bio)
>> &dd_idx, NULL);
>> end_sector = sector + bio_sectors(raid_bio);
>> + if (read_seqcount_retry(&conf->gen_lock, seq))
>> + return 0;
>> +
>> if (r5c_big_stripe_cached(conf, sector))
>> return 0;
>
> It seems that there might be race issues wherever raid5_compute_sector is
> used? This fix only addresses one of the problems.
>
Thanks for the review. You are right that this race pattern affects
more than just raid5_read_one_chunk(). I checked the callers of
raid5_compute_*() and some lockless reshape_progress / reshape_position
checks:
Already safe:
- make_stripe_request(): already uses gen_lock seqcount properly.
- init_stripe() / stripe_set_idx(): init_stripe() is under gen_lock;
stripe_set_idx() is only used for the dst stripe in handle_stripe().
- handle_stripe_expansion() / reshape_request(): reshape-internal,
intentional new layout.
- raid5-cache.c / raid5-ppl.c: journal/PPL are not allowed with
reshape, so no race.
- raid5_bio_lowest_chunk_sector(): no lock protection, but the return
value is bounded within the bio's stripe range, so the worst case
is suboptimal I/O ordering — no data corruption or lost I/O.
Needs fix:
- raid5_read_one_chunk(): fixed by this patch.
- retry_aligned_read(): has a similar issue. I will fix it with
gen_lock seqcount in the next version of this patch.
- raid5_bitmap_sector(): if the check sees LOC_NO_RESHAPE but reshape
starts and passes this region before the stripe is processed, the
bitmap position will not match the layout used for the write.
- make_discard_request(): checks mddev->reshape_position and computes
logical sectors from the layout fields, all without any lock.
- raid5_make_request(): the lockless reshape_progress check that decides
on_wq can race with reshape start.
I think there are two possible solutions:
1. All callers should check reshape within proper locking
(conf->device_lock or conf->gen_lock).
2. Suspend I/O during start_reshape via mddev_suspend() /
mddev_resume(), so readers do not need to worry about seeing an
inconsistent state.
I am going with direction 1 for now since gen_lock seems to be
designed for exactly this kind of race.
Disclaimer: The contents of this e-mail message and any attachments are confidential and are intended solely for addressee. The information may also be legally privileged. This transmission is sent in trust, for the sole purpose of delivery to the intended recipient. If you have received this transmission in error, any use, reproduction or dissemination of this transmission is strictly prohibited. If you are not the intended recipient, please immediately notify the sender by reply e-mail or phone and delete this message and its attachments, if any.
^ permalink raw reply
* [PATCH] md: factor out cloned bio cleanup into md_free_bio()
From: Abd-Alrhman Masalkhi @ 2026-04-14 10:38 UTC (permalink / raw)
To: song, yukuai; +Cc: linux-raid, linux-kernel, Abd-Alrhman Masalkhi
Refactor duplicated cloned bio completion and cleanup logic into
a new helper, md_free_bio().
md_end_clone_io() and md_free_cloned_bio() previously shared nearly
identical teardown code, differing only in whether the original
bio’s endio callback was invoked. Introduce a boolean parameter
orig_endio to control this behavior and consolidate the logic.
Signed-off-by: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
---
drivers/md/md.c | 26 +++++++++-----------------
1 file changed, 9 insertions(+), 17 deletions(-)
diff --git a/drivers/md/md.c b/drivers/md/md.c
index ac71640ff3a8..707d605fee61 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -9208,7 +9208,7 @@ static void md_bitmap_end(struct mddev *mddev, struct md_io_clone *md_io_clone)
fn(mddev, md_io_clone->offset, md_io_clone->sectors);
}
-static void md_end_clone_io(struct bio *bio)
+static void md_free_bio(struct bio *bio, bool orig_endio)
{
struct md_io_clone *md_io_clone = bio->bi_private;
struct bio *orig_bio = md_io_clone->orig_bio;
@@ -9224,10 +9224,16 @@ static void md_end_clone_io(struct bio *bio)
bio_end_io_acct(orig_bio, md_io_clone->start_time);
bio_put(bio);
- bio_endio(orig_bio);
+ if (orig_endio)
+ bio_endio(orig_bio);
percpu_ref_put(&mddev->active_io);
}
+static void md_end_clone_io(struct bio *bio)
+{
+ md_free_bio(bio, true);
+}
+
static void md_clone_bio(struct mddev *mddev, struct bio **bio)
{
struct block_device *bdev = (*bio)->bi_bdev;
@@ -9262,21 +9268,7 @@ EXPORT_SYMBOL_GPL(md_account_bio);
void md_free_cloned_bio(struct bio *bio)
{
- struct md_io_clone *md_io_clone = bio->bi_private;
- struct bio *orig_bio = md_io_clone->orig_bio;
- struct mddev *mddev = md_io_clone->mddev;
-
- if (bio_data_dir(orig_bio) == WRITE && md_bitmap_enabled(mddev, false))
- md_bitmap_end(mddev, md_io_clone);
-
- if (bio->bi_status && !orig_bio->bi_status)
- orig_bio->bi_status = bio->bi_status;
-
- if (md_io_clone->start_time)
- bio_end_io_acct(orig_bio, md_io_clone->start_time);
-
- bio_put(bio);
- percpu_ref_put(&mddev->active_io);
+ md_free_bio(bio, false);
}
EXPORT_SYMBOL_GPL(md_free_cloned_bio);
--
2.43.0
^ permalink raw reply related
* Re: [PATCH v2] md: fix kobject reference leak in md_import_device()
From: Guangshuo Li @ 2026-04-14 11:32 UTC (permalink / raw)
To: Su Yue
Cc: Song Liu, Yu Kuai, Greg Kroah-Hartman, linux-raid, linux-kernel,
stable
In-Reply-To: <fr4y8h4f.fsf@damenly.org>
Hi Su,
Thanks for reviewing.
On Tue, 14 Apr 2026 at 09:29, Su Yue <l@damenly.org> wrote:
> Why not just:
>
> out_blkdev_put:
> kobject_put(&rdev->kobj);
> fput(rdev->bdev_file);
> out_clear_rdev:
> md_rdev_clear(rdev);
> out_free_rdev:
> kfree(rdev);
> return ERR_PTR(err);
>
> --
> Su
I wonder if that ordering might cause a problem.
After kobject_init(&rdev->kobj, &rdev_ktype), kobject_put(&rdev->kobj)
may immediately drop the last reference and run the release callback
from rdev_ktype:
static const struct kobj_type rdev_ktype = {
.release = rdev_free,
.sysfs_ops = &rdev_sysfs_ops,
.default_groups = rdev_default_groups,
};
static void rdev_free(struct kobject *ko)
{
struct md_rdev *rdev = container_of(ko, struct md_rdev, kobj);
kfree(rdev);
}
So in:
out_blkdev_put:
kobject_put(&rdev->kobj);
fput(rdev->bdev_file);
it seems possible that kobject_put() would already free rdev via
rdev_free(), and then fput(rdev->bdev_file) would dereference rdev
after free.
That was why I changed it to:
out_blkdev_put:
fput(rdev->bdev_file);
md_rdev_clear(rdev);
kobject_put(&rdev->kobj);
return ERR_PTR(err);
so that the cleanup which still needs rdev is done before
kobject_put(), and this path returns directly instead of falling
through to the old kfree(rdev) path.
Please let me know if I overlooked something.
Thanks,
Guangshuo
^ permalink raw reply
* Re: [PATCH v2] md: fix kobject reference leak in md_import_device()
From: Su Yue @ 2026-04-14 14:05 UTC (permalink / raw)
To: Guangshuo Li
Cc: Song Liu, Yu Kuai, Greg Kroah-Hartman, linux-raid, linux-kernel,
stable
In-Reply-To: <CANUHTR-G1X5OBXQNiw8-mXGiugnuP8ryHcsrMXLKcD4VefuKmw@mail.gmail.com>
On Tue 14 Apr 2026 at 19:32, Guangshuo Li
<lgs201920130244@gmail.com> wrote:
> Hi Su,
>
> Thanks for reviewing.
>
> On Tue, 14 Apr 2026 at 09:29, Su Yue <l@damenly.org> wrote:
>> Why not just:
>>
>> out_blkdev_put:
>> kobject_put(&rdev->kobj);
>> fput(rdev->bdev_file);
>> out_clear_rdev:
>> md_rdev_clear(rdev);
>> out_free_rdev:
>> kfree(rdev);
>> return ERR_PTR(err);
>>
>> --
>> Su
>
> I wonder if that ordering might cause a problem.
>
> After kobject_init(&rdev->kobj, &rdev_ktype),
> kobject_put(&rdev->kobj)
> may immediately drop the last reference and run the release
> callback
> from rdev_ktype:
>
> static const struct kobj_type rdev_ktype = {
> .release = rdev_free,
> .sysfs_ops = &rdev_sysfs_ops,
> .default_groups = rdev_default_groups,
> };
>
> static void rdev_free(struct kobject *ko)
> {
> struct md_rdev *rdev = container_of(ko, struct md_rdev,
> kobj);
> kfree(rdev);
> }
>
> So in:
>
> out_blkdev_put:
> kobject_put(&rdev->kobj);
> fput(rdev->bdev_file);
>
> it seems possible that kobject_put() would already free rdev via
> rdev_free(), and then fput(rdev->bdev_file) would dereference
> rdev
> after free.
>
> That was why I changed it to:
>
> out_blkdev_put:
> fput(rdev->bdev_file);
> md_rdev_clear(rdev);
> kobject_put(&rdev->kobj);
> return ERR_PTR(err);
>
> so that the cleanup which still needs rdev is done before
> kobject_put(), and this path returns directly instead of falling
> through to the old kfree(rdev) path.
>
> Please let me know if I overlooked something.
>
Thanks for your detailed explanation. It's totally correct.
--
Su
> Thanks,
> Guangshuo
^ 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] md/raid5: Don't set bi_status on STRIPE_WAIT_RESHAPE
From: Benjamin Marzinski @ 2026-04-14 18:19 UTC (permalink / raw)
To: Yu Kuai
Cc: Li Nan, Yu Kuai, Song Liu, linux-raid, dm-devel, Xiao Ni,
Nigel Croxon, Yang Xiuwei
In-Reply-To: <717caf7b-03ef-41d8-bbb3-f6ce5d4a49fa@fnnas.com>
On Tue, Apr 14, 2026 at 02:20:40PM +0800, Yu Kuai wrote:
> Hi,
>
> 在 2026/4/14 9:25, Li Nan 写道:
> >
> >
> > 在 2026/4/14 6:45, Benjamin Marzinski 写道:
> >> When make_stripe_request() encounters a clone bio that crosses the
> >> reshape position while the reshape cannot make progress, it was setting
> >> bi->bi_status to BLK_STS_RESOURCE when returning STRIPE_WAIT_RESHAPE.
> >> This will update the original bio's bi_status in md_end_clone_io().
> >> Afterwards, md_handle_request() will wait for the device to become
> >> unsuspended and submit a new cloned bio. However, even if that clone
> >> completes successfully, it will not clear the original bio's bi_status.
> >>
> >> There's no need to set bi_status when retrying the bio. md will already
> >> error out the bio correctly if it is set REQ_NOWAIT. Otherwise it will
> >> be retried. dm-raid will already end the bio with DM_MAPIO_REQUEUE.
> >>
> >> Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
> >> ---
> >> drivers/md/raid5.c | 1 -
> >> 1 file changed, 1 deletion(-)
> >>
> >> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> >> index dc0c680ca199..690c65cd1e29 100644
> >> --- a/drivers/md/raid5.c
> >> +++ b/drivers/md/raid5.c
> >> @@ -6042,7 +6042,6 @@ static enum stripe_result
> >> make_stripe_request(struct mddev *mddev,
> >> raid5_release_stripe(sh);
> >> out:
> >> if (ret == STRIPE_SCHEDULE_AND_RETRY &&
> >> reshape_interrupted(mddev)) {
> >> - bi->bi_status = BLK_STS_RESOURCE;
> >> ret = STRIPE_WAIT_RESHAPE;
> >> pr_err_ratelimited("dm-raid456: io across reshape position
> >> while reshape can't make progress");
> >> }
> >
> > The link below leads to the same patch, which Kuai has already replied
> > to.
> >
> > https://lore.kernel.org/all/20260203095156.2349174-1-yangxiuwei@kylinos.cn/
>
> Perhaps instead of clearing the error code from error path, this problem can be fixed
> by resetting the error code from the issue path if original bio is resubmitted.
I saw your comments at
https://lore.kernel.org/all/71e50b0e-0669-4a40-84d5-3c3061dfb229@fnnas.com/
and I'm a little confused.
The only code path where STRIPE_WAIT_RESHAPE is returned and
bi->bi_status is currently set to BLK_STS_RESOURCE is:
md_handle_request -> raid5_make_request -> make_stripe_request()
make_stripe_request() returning STRIPE_WAIT_RESHAPE, means that
raid5_make_request() will return false (this is the only situation where
raid5_make_request() returns false). This causes the cloned bio to be
freed without completing the original bio.
raid5_make_request() returning false will cause md_handle_request() to
do different things, depending on whether the device is a dm device or a
md device.
For dm devices, md_handle_request() will return false, causing
dm-raid.c:raid_map() to return DM_MAPIO_REQUEUE. This will either
requeue dm's original bio (md's orignal bio is itself a clone of dm's
original bio) if the device is currently in a noflush suspend or
complete dm's original bio with BLK_STS_IOERR if the device is not.
Since the DM_MAPIO_REQUEUE overrides any error for bios that should be
requeued, removing "bi->bi_status = BLK_STS_RESOURCE" doesn't actually
seem important for DM.
But for md devices, md_handle_request() will loop back to check_suspend,
which will complete the bio with BLK_STS_AGAIN if it's a REQ_NOWAIT bio,
and will otherwise wait until the device is no longer suspended to call
raid5_make_request() again. If that later call to raid5_make_request()
completes successfully, the original bio will retain the
BLK_STS_RESOURCE status from the earlier failed call, instead of
completing successfully like it should.
I don't see where a bio could get completed without bio->bi_status
getting set to an approriate error here. Am I missing something?
Obviously clearing the error when you resubmit would fix the issue
as well. It just seems odd to set it and then clear it when AFAICT
nothing requires it to be set in the first place. But perhaps I'm
overlooking something.
Yang Xiuwei, have you verified that this fix actually solves your
problems? If a dm map() function completes with DM_MAPIO_REQUEUE, and
the device is in a noflush suspend, it shouldn't set the error on the
original bio, regardless of the clone bio. It should requeue the bio. If
a dm map() function completes with DM_MAPIO_REQUEUE, and the device
isn't in a noflush suspend, the original bio will always be completed
with an error.
To me, it seems more likely that what you are seeing is
make_stripe_request() returning STRIPE_WAIT_RESHAPE when the dm device
isn't actually in a noflush suspend. I have seen this myself.
-Ben
>
> >
> >
> > --
> > Thanks,
> > Nan
> >
> >
> --
> Thansk,
> Kuai
^ permalink raw reply
* [RFC PATCH] dm-raid: only requeue bios when dm is suspending.
From: Benjamin Marzinski @ 2026-04-14 19:03 UTC (permalink / raw)
To: Yang Xiuwei
Cc: Yu Kuai, Li Nan, Song Liu, linux-raid, dm-devel, Xiao Ni,
Nigel Croxon
In-Reply-To: <ad6FNUtSCS_zTrHW@redhat.com>
returning DM_MAPIO_REQUEUE from the target map() function only requeues
the bio during noflush suspends. During regular operations or during
flushing suspends, it fails the bio. Failing the bio during flushing
suspends is the correct behavior here. We cannot handle the bio, and we
cannot suspends while it is outstanding. But during normal operations,
we should not push the bio back to do. Instead, wait for the reshape
to be resumed.
Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
Yang Xiuwei, if you are still able to see I/O errors during LVM testing,
does this patch fix them?
drivers/md/dm-raid.c | 7 +++++++
drivers/md/md.h | 1 +
drivers/md/raid5.c | 6 ++++--
3 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/drivers/md/dm-raid.c b/drivers/md/dm-raid.c
index 4bacdc499984..cac61d57e7e2 100644
--- a/drivers/md/dm-raid.c
+++ b/drivers/md/dm-raid.c
@@ -3831,6 +3831,7 @@ static void raid_presuspend(struct dm_target *ti)
* resume, raid_postsuspend() is too late.
*/
set_bit(RT_FLAG_RS_FROZEN, &rs->runtime_flags);
+ WRITE_ONCE(mddev->dm_suspending, 1);
if (!reshape_interrupted(mddev))
return;
@@ -3847,6 +3848,9 @@ static void raid_presuspend(struct dm_target *ti)
static void raid_presuspend_undo(struct dm_target *ti)
{
struct raid_set *rs = ti->private;
+ struct mddev *mddev = &rs->md;
+
+ WRITE_ONCE(mddev->dm_suspending, 0);
clear_bit(RT_FLAG_RS_FROZEN, &rs->runtime_flags);
}
@@ -3854,6 +3858,7 @@ static void raid_presuspend_undo(struct dm_target *ti)
static void raid_postsuspend(struct dm_target *ti)
{
struct raid_set *rs = ti->private;
+ struct mddev *mddev = &rs->md;
if (!test_and_set_bit(RT_FLAG_RS_SUSPENDED, &rs->runtime_flags)) {
/*
@@ -3864,6 +3869,8 @@ static void raid_postsuspend(struct dm_target *ti)
mddev_suspend(&rs->md, false);
rs->md.ro = MD_RDONLY;
}
+ WRITE_ONCE(mddev->dm_suspending, 0);
+
}
static void attempt_restore_of_faulty_devices(struct raid_set *rs)
diff --git a/drivers/md/md.h b/drivers/md/md.h
index ac84289664cd..e8d7332c5cb9 100644
--- a/drivers/md/md.h
+++ b/drivers/md/md.h
@@ -463,6 +463,7 @@ struct mddev {
int delta_disks, new_level, new_layout;
int new_chunk_sectors;
int reshape_backwards;
+ int dm_suspending;
struct md_thread __rcu *thread; /* management thread */
struct md_thread __rcu *sync_thread; /* doing resync or reconstruct */
diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index 8854e024f311..d528263f92a3 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -6042,8 +6042,10 @@ static enum stripe_result make_stripe_request(struct mddev *mddev,
raid5_release_stripe(sh);
out:
if (ret == STRIPE_SCHEDULE_AND_RETRY && reshape_interrupted(mddev)) {
- bi->bi_status = BLK_STS_RESOURCE;
- ret = STRIPE_WAIT_RESHAPE;
+ if (!mddev_is_dm(mddev) || READ_ONCE(mddev->dm_suspending)) {
+ bi->bi_status = BLK_STS_RESOURCE;
+ ret = STRIPE_WAIT_RESHAPE;
+ }
pr_err_ratelimited("dm-raid456: io across reshape position while reshape can't make progress");
}
return ret;
--
2.50.1
^ permalink raw reply related
* Re: [PATCH] md/raid5: Don't set bi_status on STRIPE_WAIT_RESHAPE
From: Yang Xiuwei @ 2026-04-15 1:28 UTC (permalink / raw)
To: Benjamin Marzinski
Cc: Yu Kuai, Li Nan, Song Liu, Xiao Ni, Nigel Croxon, linux-raid,
dm-devel
In-Reply-To: <ad6FNUtSCS_zTrHW@redhat.com>
Hi Ben,
On Tue, Apr 14, 2026 at 02:19:33PM -0400, Benjamin Marzinski wrote:
> Yang Xiuwei, have you verified that this fix actually solves your
> problems? If a dm map() function completes with DM_MAPIO_REQUEUE, and
> the device is in a noflush suspend, it shouldn't set the error on the
> original bio, regardless of the clone bio. It should requeue the bio. If
> a dm map() function completes with DM_MAPIO_REQUEUE, and the device
> isn't in a noflush suspend, the original bio will always be completed
> with an error.
>
> To me, it seems more likely that what you are seeing is
> make_stripe_request() returning STRIPE_WAIT_RESHAPE when the dm device
> isn't actually in a noflush suspend. I have seen this myself.
>
> -Ben
I tested the version that removes setting bi->bi_status to BLK_STS_RESOURCE
in the STRIPE_WAIT_RESHAPE path you described. In my environment it did
not fix the failure below.
Sorry for the slow response. The earlier fix still did not solve the
problem in my testing. I am not very familiar with this area yet and wanted
to learn more before continuing the analysis, but other work meant I have
not had time to pick it up again until now.
I have not yet tested the dm-raid RFC patch from your follow-up message,
but I plan to try it when I have time.
The failure was observed while running the LVM2 shell test
lvconvert-raid-reshape-stripes-load-fail.sh. Below is the test log (kernel
messages and harness output), followed by the script contents.
Test log:
| [ 0:10.630] WARNING: This metadata update is NOT backed up.
| [ 0:10.632] aux disable_dev $dev1
| [ 0:10.748] #lvconvert-raid-reshape-stripes-load-fail.sh:68+ aux disable_dev /dev/mapper/LVMTEST1351568pv1
| [ 0:10.748] Disabling device /dev/mapper/LVMTEST1351568pv1 (252:5)
| [ 0:10.868] [73439.222696] <6> 2026-01-20 13:59:47 md: reshape of RAID array mdX
| [ 0:10.868] aux delay_dev "$dev2" 0 50
| [ 0:10.871] #lvconvert-raid-reshape-stripes-load-fail.sh:69+ aux delay_dev /dev/mapper/LVMTEST1351568pv2 0 50
| [ 0:10.871] check lv_first_seg_field $vg/$lv1 segtype "raid5_ls"
| [ 0:10.886] [73439.231558] <3> 2026-01-20 13:59:47 Buffer I/O error on dev dm-5, logical block 0, async page read
| [ 0:10.886] #lvconvert-raid-reshape-stripes-load-fail.sh:70+ check lv_first_seg_field LVMTEST1351568vg/LV1 segtype raid5_ls
| [ 0:10.886] WARNING: Couldn't find device with uuid Xprpyw-NTcw-RDRr-HzMg-LDZN-ZDIL-0Q2LoQ.
| [ 0:10.910] WARNING: VG LVMTEST1351568vg is missing PV Xprpyw-NTcw-RDRr-HzMg-LDZN-ZDIL-0Q2LoQ (last written to /dev/mapper/LVMTEST1351568pv1).
| [ 0:10.910] WARNING: Couldn't find all devices for LV LVMTEST1351568vg/LV1_rimage_0 while checking used and assumed devices.
| [ 0:10.910] WARNING: Couldn't find all devices for LV LVMTEST1351568vg/LV1_rmeta_0 while checking used and assumed devices.
| [ 0:10.910] check lv_first_seg_field $vg/$lv1 stripesize "64.00k"
| [ 0:10.912] #lvconvert-raid-reshape-stripes-load-fail.sh:71+ check lv_first_seg_field LVMTEST1351568vg/LV1 stripesize 64.00k
| [ 0:10.912] WARNING: Couldn't find device with uuid Xprpyw-NTcw-RDRr-HzMg-LDZN-ZDIL-0Q2LoQ.
| [ 0:10.933] WARNING: VG LVMTEST1351568vg is missing PV Xprpyw-NTcw-RDRr-HzMg-LDZN-ZDIL-0Q2LoQ (last written to /dev/mapper/LVMTEST1351568pv1).
| [ 0:10.933] WARNING: Couldn't find all devices for LV LVMTEST1351568vg/LV1_rimage_0 while checking used and assumed devices.
| [ 0:10.933] WARNING: Couldn't find all devices for LV LVMTEST1351568vg/LV1_rmeta_0 while checking used and assumed devices.
| [ 0:10.933] check lv_first_seg_field $vg/$lv1 data_stripes 15
| [ 0:10.935] #lvconvert-raid-reshape-stripes-load-fail.sh:72+ check lv_first_seg_field LVMTEST1351568vg/LV1 data_stripes 15
| [ 0:10.935] WARNING: Couldn't find device with uuid Xprpyw-NTcw-RDRr-HzMg-LDZN-ZDIL-0Q2LoQ.
| [ 0:10.956] [73439.292632] <3> 2026-01-20 13:59:47 md: super_written gets error=-5
| [ 0:10.956] [73439.297679] <2> 2026-01-20 13:59:47 md/raid:mdX: Disk failure on dm-22, disabling device.
| [ 0:10.956] [73439.304626] <2> 2026-01-20 13:59:47 md/raid:mdX: Operation continuing on 15 devices.
| [ 0:10.956] WARNING: VG LVMTEST1351568vg is missing PV Xprpyw-NTcw-RDRr-HzMg-LDZN-ZDIL-0Q2LoQ (last written to /dev/mapper/LVMTEST1351568pv1).
| [ 0:10.956] WARNING: Couldn't find all devices for LV LVMTEST1351568vg/LV1_rimage_0 while checking used and assumed devices.
| [ 0:10.956] WARNING: Couldn't find all devices for LV LVMTEST1351568vg/LV1_rmeta_0 while checking used and assumed devices.
| [ 0:10.956] check lv_first_seg_field $vg/$lv1 stripes 16
| [ 0:10.958] #lvconvert-raid-reshape-stripes-load-fail.sh:73+ check lv_first_seg_field LVMTEST1351568vg/LV1 stripes 16
| [ 0:10.958] WARNING: Couldn't find device with uuid Xprpyw-NTcw-RDRr-HzMg-LDZN-ZDIL-0Q2LoQ.
| [ 0:10.979] WARNING: VG LVMTEST1351568vg is missing PV Xprpyw-NTcw-RDRr-HzMg-LDZN-ZDIL-0Q2LoQ (last written to /dev/mapper/LVMTEST1351568pv1).
| [ 0:10.979] WARNING: Couldn't find all devices for LV LVMTEST1351568vg/LV1_rimage_0 while checking used and assumed devices.
| [ 0:10.979] WARNING: Couldn't find all devices for LV LVMTEST1351568vg/LV1_rmeta_0 while checking used and assumed devices.
| [ 0:10.979]
| [ 0:10.981] kill -9 %%
| [ 0:10.981] #lvconvert-raid-reshape-stripes-load-fail.sh:75+ kill -9 %%
| [ 0:10.981] wait
| [ 0:10.981] #lvconvert-raid-reshape-stripes-load-fail.sh:76+ wait
| [ 0:10.981] rm -fr "$mount_dir/[12]"
| [ 0:11.787] [73439.674065] <4> 2026-01-20 13:59:48 make_stripe_request: 24 callbacks suppressed
| [ 0:11.787] [73439.674074] <3> 2026-01-20 13:59:48 dm-raid456: io across reshape position while reshape can't make progress
| [ 0:11.787] [73439.674086] <3> 2026-01-20 13:59:48 Buffer I/O error on dev dm-43, logical block 1074, lost sync page write
| [ 0:11.787] [73439.681096] <6> 2026-01-20 13:59:48 md: mdX: reshape interrupted.
| [ 0:11.787] [73439.682723] <3> 2026-01-20 13:59:48 dm-raid456: io across reshape position while reshape can't make progress
| [ 0:11.787] [73439.691180] <3> 2026-01-20 13:59:48 dm-raid456: io across reshape position while reshape can't make progress
| [ 0:11.787] [73439.699766] <3> 2026-01-20 13:59:48 dm-raid456: io across reshape position while reshape can't make progress
| [ 0:11.787] [73439.708347] <3> 2026-01-20 13:59:48 dm-raid456: io across reshape position while reshape can't make progress
| [ 0:11.787] [73439.716934] <3> 2026-01-20 13:59:48 dm-raid456: io across reshape position while reshape can't make progress
| [ 0:11.787] [73439.725519] <3> 2026-01-20 13:59:48 dm-raid456: io across reshape position while reshape can't make progress
| [ 0:11.787] [73439.734099] <3> 2026-01-20 13:59:48 dm-raid456: io across reshape position while reshape can't make progress
| [ 0:11.787] [73439.734574] <2> 2026-01-20 13:59:48 EXT4-fs error (device dm-43): ext4_check_bdev_write_error:225: comm kworker/u388:2: Error while async write back metadata
| [ 0:11.787] [73439.742682] <3> 2026-01-20 13:59:48 dm-raid456: io across reshape position while reshape can't make progress
| [ 0:11.787] [73439.764081] <3> 2026-01-20 13:59:48 Aborting journal on device dm-43-8.
| [ 0:11.787] [73439.778040] <3> 2026-01-20 13:59:48 dm-raid456: io across reshape position while reshape can't make progress
| [ 0:11.788] [73439.778043] <3> 2026-01-20 13:59:48 Buffer I/O error on dev dm-43, logical block 740, lost sync page write
| [ 0:11.788] [73439.795025] <3> 2026-01-20 13:59:48 JBD2: I/O error when updating journal superblock for dm-43-8.
| [ 0:11.788] [73439.802674] <2> 2026-01-20 13:59:48 EXT4-fs error (device dm-43): ext4_journal_check_start:85: comm cp: Detected aborted journal
| [ 0:11.788] [73439.802673] <2> 2026-01-20 13:59:48 EXT4-fs error (device dm-43): ext4_journal_check_start:85: comm cp: Detected aborted journal
| [ 0:11.788] [73440.032568] <3> 2026-01-20 13:59:48 Buffer I/O error on dev dm-43, logical block 1, lost sync page write
| [ 0:11.788] [73440.040800] <3> 2026-01-20 13:59:48 EXT4-fs (dm-43): I/O error while writing superblock
| [ 0:11.788] [73440.040813] <3> 2026-01-20 13:59:48 EXT4-fs (dm-43): previous I/O error to superblock detected
| [ 0:11.788] [73440.047569] <2> 2026-01-20 13:59:48 EXT4-fs (dm-43): Remounting filesystem read-only
| [ 0:11.788] [73440.054948] <3> 2026-01-20 13:59:48 Buffer I/O error on dev dm-43, logical block 1, lost sync page write
| [ 0:11.788] [73440.069663] <3> 2026-01-20 13:59:48 EXT4-fs (dm-43): I/O error while writing superblock
| [ 0:11.788] [73440.076428] <2> 2026-01-20 13:59:48 EXT4-fs (dm-43): Remounting filesystem read-only
| [ 0:11.788] #lvconvert-raid-reshape-stripes-load-fail.sh:77+ rm -fr 'mnt/[12]'
| [ 0:11.788]
| [ 0:11.789] sync
| [ 0:11.789] #lvconvert-raid-reshape-stripes-load-fail.sh:79+ sync
| [ 0:11.789] umount "$mount_dir"
| [ 0:11.798] [73440.145596] <3> 2026-01-20 13:59:48 Buffer I/O error on dev dm-43, logical block 82, lost async page write
| [ 0:11.798] #lvconvert-raid-reshape-stripes-load-fail.sh:80+ umount mnt
| [ 0:11.798]
| [ 0:11.814] fsck -fn "$DM_DEV_DIR/$vg/$lv1"
| [ 0:11.814] [73440.162114] <6> 2026-01-20 13:59:48 EXT4-fs (dm-43): unmounting filesystem 86548d8e-e409-4ae8-b7d5-8b78a9b5fb50.
| [ 0:11.814] [73440.162336] <3> 2026-01-20 13:59:48 EXT4-fs (dm-43): I/O error while writing superblock
| [ 0:11.814] #lvconvert-raid-reshape-stripes-load-fail.sh:82+ fsck -fn /dev/LVMTEST1351568vg/LV1
| [ 0:11.814] fsck from util-linux 2.39.1
| [ 0:11.816] e2fsck 1.47.0 (5-Feb-2023)
| [ 0:11.821] fsck.ext2: Input/output error while trying to open /dev/mapper/LVMTEST1351568vg-LV1
| [ 0:11.821]
| [ 0:11.821] The superblock could not be read or does not describe a valid ext2/ext3/ext4
| [ 0:11.821] filesystem. If the device is valid and it really contains an ext2/ext3/ext4
| [ 0:11.821] filesystem (and not swap or ufs or something else), then the superblock
| [ 0:11.821] is corrupt, and you might try running e2fsck with an alternate superblock:
| [ 0:11.821] e2fsck -b 8193 <device>
| [ 0:11.821] or
| [ 0:11.821] e2fsck -b 32768 <device>
| [ 0:11.821]
| [ 0:11.821] set +vx; STACKTRACE; set -vx
| [ 0:11.822] ##lvconvert-raid-reshape-stripes-load-fail.sh:82+ set +vx
| [ 0:11.822] ## - /opt/K2CI_agent_tool/lvm2/test/shell/lvconvert-raid-reshape-stripes-load-fail.sh:82
| [ 0:11.822] ## 1 STACKTRACE() called from /opt/K2CI_agent_tool/lvm2/test/shell/lvconvert-raid-reshape-stripes-load-fail.sh:82
lvconvert-raid-reshape-stripes-load-fail.sh:
#!/usr/bin/env bash
# Copyright (C) 2017 Red Hat, Inc. All rights reserved.
#
# This copyrighted material is made available to anyone wishing to use,
# modify, copy, or redistribute it subject to the terms and conditions
# of the GNU General Public License v.2.
#
# You should have received a copy of the GNU General Public License
# along with this program; if not, write to the Free Software Foundation,
# Inc., 51 Franklin Street, Fifth Floor, Boston, MA2110-1301 USA
SKIP_WITH_LVMPOLLD=1
. lib/inittest
# Test reshaping under io load
case "$(uname -r)" in
3.10.0-862*) skip "Cannot run this test on unfixed kernel." ;;
esac
which mkfs.ext4 || skip
aux have_raid 1 13 2 || skip
mount_dir="mnt"
cleanup_mounted_and_teardown()
{
umount "$mount_dir" || true
aux teardown
}
aux prepare_pvs 16 32
get_devs
vgcreate $SHARED -s 1M "$vg" "${DEVICES[@]}"
trap 'cleanup_mounted_and_teardown' EXIT
# Create 10-way striped raid5 (11 legs total)
lvcreate --yes --type raid5_ls --stripesize 64K --stripes 10 -L4 -n$lv1 $vg
check lv_first_seg_field $vg/$lv1 segtype "raid5_ls"
check lv_first_seg_field $vg/$lv1 stripesize "64.00k"
check lv_first_seg_field $vg/$lv1 data_stripes 10
check lv_first_seg_field $vg/$lv1 stripes 11
wipefs -a "$DM_DEV_DIR/$vg/$lv1"
mkfs -t ext4 "$DM_DEV_DIR/$vg/$lv1"
fsck -fn "$DM_DEV_DIR/$vg/$lv1"
mkdir -p "$mount_dir"
mount "$DM_DEV_DIR/$vg/$lv1" "$mount_dir"
mkdir -p "$mount_dir/1" "$mount_dir/2"
echo 3 >/proc/sys/vm/drop_caches
cp -r /usr/bin "$mount_dir/1" &>/dev/null &
cp -r /usr/bin "$mount_dir/2" &>/dev/null &
sync &
aux wait_for_sync $vg $lv1
aux delay_dev "$dev2" 0 100
# Reshape it to 15 data stripes
lvconvert --yes --stripes 15 $vg/$lv1
aux disable_dev $dev1
aux delay_dev "$dev2" 0 50
check lv_first_seg_field $vg/$lv1 segtype "raid5_ls"
check lv_first_seg_field $vg/$lv1 stripesize "64.00k"
check lv_first_seg_field $vg/$lv1 data_stripes 15
check lv_first_seg_field $vg/$lv1 stripes 16
kill -9 %%
wait
rm -fr "$mount_dir/[12]"
sync
umount "$mount_dir"
fsck -fn "$DM_DEV_DIR/$vg/$lv1"
vgremove -ff $vg
Thanks,
Yang Xiuwei
^ permalink raw reply
* Re: [PATCH] md: fix kobject reference leak in md_import_device()
From: Li Nan @ 2026-04-15 1:40 UTC (permalink / raw)
To: Guangshuo Li, Song Liu, Yu Kuai, Greg Kroah-Hartman, linux-raid,
linux-kernel
Cc: stable
In-Reply-To: <20260412154219.2560732-1-lgs201920130244@gmail.com>
在 2026/4/12 23:42, Guangshuo Li 写道:
> md_import_device() initializes rdev->kobj with kobject_init() before
> checking the device size and loading the superblock.
>
> When one of the later checks fails, the error path still frees rdev
> directly with kfree(). This bypasses the kobject release path and leaves
> the kobject reference unbalanced.
>
> After kobject_init(), release rdev through kobject_put() instead of
> kfree().
>
> Fixes: f9cb074bff8e ("Kobject: rename kobject_init_ng() to kobject_init()")
> Cc: stable@vger.kernel.org
> Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
> ---
> drivers/md/md.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index 6d73f6e196a9..4ce7512dc834 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -3871,6 +3871,9 @@ static struct md_rdev *md_import_device(dev_t newdev, int super_format, int supe
>
> out_blkdev_put:
> fput(rdev->bdev_file);
> + md_rdev_clear(rdev);
> + kobject_put(&rdev->kobj);
> + return ERR_PTR(err);
> out_clear_rdev:
> md_rdev_clear(rdev);
> out_free_rdev:
Multiple return points in error handling are strange. Can we move
kobject_init() before return rdev? It would be simpler.
--
Thanks,
Nan
^ permalink raw reply
* Re: [PATCH] md: factor out cloned bio cleanup into md_free_bio()
From: Li Nan @ 2026-04-15 2:27 UTC (permalink / raw)
To: Abd-Alrhman Masalkhi, song, yukuai
Cc: linux-raid, linux-kernel, yangerkun@huawei.com
In-Reply-To: <20260414103813.307601-1-abd.masalkhi@gmail.com>
在 2026/4/14 18:38, Abd-Alrhman Masalkhi 写道:
> Refactor duplicated cloned bio completion and cleanup logic into
> a new helper, md_free_bio().
>
> md_end_clone_io() and md_free_cloned_bio() previously shared nearly
> identical teardown code, differing only in whether the original
> bio’s endio callback was invoked. Introduce a boolean parameter
> orig_endio to control this behavior and consolidate the logic.
>
> Signed-off-by: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
> ---
> drivers/md/md.c | 26 +++++++++-----------------
> 1 file changed, 9 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index ac71640ff3a8..707d605fee61 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -9208,7 +9208,7 @@ static void md_bitmap_end(struct mddev *mddev, struct md_io_clone *md_io_clone)
> fn(mddev, md_io_clone->offset, md_io_clone->sectors);
> }
>
> -static void md_end_clone_io(struct bio *bio)
> +static void md_free_bio(struct bio *bio, bool orig_endio)
> {
> struct md_io_clone *md_io_clone = bio->bi_private;
> struct bio *orig_bio = md_io_clone->orig_bio;
> @@ -9224,10 +9224,16 @@ static void md_end_clone_io(struct bio *bio)
> bio_end_io_acct(orig_bio, md_io_clone->start_time);
>
> bio_put(bio);
> - bio_endio(orig_bio);
> + if (orig_endio)
> + bio_endio(orig_bio);
> percpu_ref_put(&mddev->active_io);
> }
>
> +static void md_end_clone_io(struct bio *bio)
> +{
> + md_free_bio(bio, true);
> +}
> +
> static void md_clone_bio(struct mddev *mddev, struct bio **bio)
> {
> struct block_device *bdev = (*bio)->bi_bdev;
> @@ -9262,21 +9268,7 @@ EXPORT_SYMBOL_GPL(md_account_bio);
>
> void md_free_cloned_bio(struct bio *bio)
> {
> - struct md_io_clone *md_io_clone = bio->bi_private;
> - struct bio *orig_bio = md_io_clone->orig_bio;
> - struct mddev *mddev = md_io_clone->mddev;
> -
> - if (bio_data_dir(orig_bio) == WRITE && md_bitmap_enabled(mddev, false))
> - md_bitmap_end(mddev, md_io_clone);
> -
> - if (bio->bi_status && !orig_bio->bi_status)
> - orig_bio->bi_status = bio->bi_status;
> -
> - if (md_io_clone->start_time)
> - bio_end_io_acct(orig_bio, md_io_clone->start_time);
> -
> - bio_put(bio);
> - percpu_ref_put(&mddev->active_io);
> + md_free_bio(bio, false);
> }
> EXPORT_SYMBOL_GPL(md_free_cloned_bio);
>
md_free_bio() is redundant. Can md_end_clone_io() directly call
md_free_cloned_bio()?
We do not need to consider the compatibility of the EXPORT_SYMBOL in
mainline.
--
Thanks,
Nan
^ permalink raw reply
* 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
* [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 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
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