* [PATCH] nvme: Add module reference counting for multipath nvme device
@ 2026-08-10 23:50 wenxiong
2026-08-11 13:18 ` John Garry
2026-08-11 19:12 ` Keith Busch
0 siblings, 2 replies; 7+ messages in thread
From: wenxiong @ 2026-08-10 23:50 UTC (permalink / raw)
To: linux-nvme, kbusch; +Cc: gjoyce, wenxiong, Wen Xiong
From: Wen Xiong <wenxiong@linux.ibm.com>
Ensure proper module reference counting for NVMe multipath head devices
on open, preventing the controller module from being unloaded while
the multipath head device is still open or in use.
This patch acquires a module reference in nvme_ns_head_open() and
releases it in nvme_ns_head_release()
Signed-off-by: Wen Xiong <wenxiong@linux.ibm.com>
---
drivers/nvme/host/multipath.c | 29 +++++++++++++++++++++++++++--
1 file changed, 27 insertions(+), 2 deletions(-)
diff --git a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipath.c
index 9b9a657fa330..74c3710bf5f7 100644
--- a/drivers/nvme/host/multipath.c
+++ b/drivers/nvme/host/multipath.c
@@ -554,14 +554,39 @@ static void nvme_ns_head_submit_bio(struct bio *bio)
static int nvme_ns_head_open(struct gendisk *disk, blk_mode_t mode)
{
- if (!nvme_tryget_ns_head(disk->private_data))
+ struct nvme_ns_head *head = disk->private_data;
+ struct nvme_ns *ns;
+ int srcu_idx;
+
+ if (!nvme_tryget_ns_head(head))
return -ENXIO;
+
+ /* Get module reference from any available path */
+ srcu_idx = srcu_read_lock(&head->srcu);
+ ns = nvme_find_path(head);
+ if (ns && !try_module_get(ns->ctrl->ops->module)) {
+ srcu_read_unlock(&head->srcu, srcu_idx);
+ nvme_put_ns_head(head);
+ return -ENXIO;
+ }
+ srcu_read_unlock(&head->srcu, srcu_idx);
+
return 0;
}
static void nvme_ns_head_release(struct gendisk *disk)
{
- nvme_put_ns_head(disk->private_data);
+ struct nvme_ns_head *head = disk->private_data;
+ struct nvme_ns *ns;
+ int srcu_idx;
+
+ srcu_idx = srcu_read_lock(&head->srcu);
+ ns = nvme_find_path(head);
+ if (ns)
+ module_put(ns->ctrl->ops->module);
+ srcu_read_unlock(&head->srcu, srcu_idx);
+
+ nvme_put_ns_head(head);
}
static int nvme_ns_head_get_unique_id(struct gendisk *disk, u8 id[16],
--
2.52.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] nvme: Add module reference counting for multipath nvme device
2026-08-10 23:50 [PATCH] nvme: Add module reference counting for multipath nvme device wenxiong
@ 2026-08-11 13:18 ` John Garry
2026-08-11 18:54 ` Wen Xiong
2026-08-11 19:12 ` Keith Busch
1 sibling, 1 reply; 7+ messages in thread
From: John Garry @ 2026-08-11 13:18 UTC (permalink / raw)
To: wenxiong, linux-nvme, kbusch; +Cc: gjoyce, wenxiong
On 11/08/2026 00:50, wenxiong@linux.ibm.com wrote:
> From: Wen Xiong<wenxiong@linux.ibm.com>
>
> Ensure proper module reference counting for NVMe multipath head devices
> on open, preventing the controller module from being unloaded while
> the multipath head device is still open or in use.
>
> This patch acquires a module reference in nvme_ns_head_open() and
> releases it in nvme_ns_head_release()
Can you demonstrate that this is a problem?
Can we remove the controller driver while the NS head bdev is open? If
we could, then any NSes must disappear as we could not unload the
controller driver module while the controller is registered and has
NSes, right? Then with no NSes we cannot find a path from
nvme_ns_head_submit_bio() -> nvme_find_path() or similar and would
error. Or if we have a NS reference from nvme_ns_head_submit_bio() ->
nvme_find_path() and then remove the controller and its module, the NS
removal must be part of the controller teardown which does a scru sync
per NS, which must wait for nvme_ns_head_submit_bio() to complete.
That's the way I see it ... but maybe I am missing something.
>
> Signed-off-by: Wen Xiong<wenxiong@linux.ibm.com>
> ---
> drivers/nvme/host/multipath.c | 29 +++++++++++++++++++++++++++--
> 1 file changed, 27 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipath.c
> index 9b9a657fa330..74c3710bf5f7 100644
> --- a/drivers/nvme/host/multipath.c
> +++ b/drivers/nvme/host/multipath.c
> @@ -554,14 +554,39 @@ static void nvme_ns_head_submit_bio(struct bio *bio)
>
> static int nvme_ns_head_open(struct gendisk *disk, blk_mode_t mode)
> {
> - if (!nvme_tryget_ns_head(disk->private_data))
> + struct nvme_ns_head *head = disk->private_data;
> + struct nvme_ns *ns;
> + int srcu_idx;
> +
> + if (!nvme_tryget_ns_head(head))
> return -ENXIO;
> +
> + /* Get module reference from any available path */
> + srcu_idx = srcu_read_lock(&head->srcu);
> + ns = nvme_find_path(head);
> + if (ns && !try_module_get(ns->ctrl->ops->module)) {
If you cannot find a path then why even allow the nvme_ns_head_open() to
succeed?
> + srcu_read_unlock(&head->srcu, srcu_idx);
> + nvme_put_ns_head(head);
> + return -ENXIO;
> + }
> + srcu_read_unlock(&head->srcu, srcu_idx);
> +
> return 0;
> }
>
> static void nvme_ns_head_release(struct gendisk *disk)
> {
> - nvme_put_ns_head(disk->private_data);
> + struct nvme_ns_head *head = disk->private_data;
> + struct nvme_ns *ns;
> + int srcu_idx;
> +
> + srcu_idx = srcu_read_lock(&head->srcu);
> + ns = nvme_find_path(head);
> + if (ns)
> + module_put(ns->ctrl->ops->module);
> + srcu_read_unlock(&head->srcu, srcu_idx);
> +
> + nvme_put_ns_head(head);
> }
>
> static int nvme_ns_head_get_unique_id(struct gendisk *disk, u8 id[16],
> --
> 2.52.0
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] nvme: Add module reference counting for multipath nvme device
2026-08-11 13:18 ` John Garry
@ 2026-08-11 18:54 ` Wen Xiong
0 siblings, 0 replies; 7+ messages in thread
From: Wen Xiong @ 2026-08-11 18:54 UTC (permalink / raw)
To: John Garry; +Cc: linux-nvme, kbusch, gjoyce, wenxiong
On 2026-08-11 08:18, John Garry wrote:
> Can you demonstrate that this is a problem?
>
For example, the Linux root filesystem is located on a multipath NVMe
device. During system boot, the reference count of the nvme module is
zero. As a result, the tester can run rmmod nvme, which causes the
system to become unstable or crash.
linux was installed on /dev/nvme4n2.
#lsmod|grep nvme
nvme_fabrics 262144 0
nvme 262144 0 ----> reference count = 0 for nvme
module.
nvme_core 458752 4 nvme,nvme_fabrics
nvme_keyring 262144 2 nvme_core,nvme_fabrics
nvme_auth 262144 1 nvme_core
# rmmod nvme
[11312.100786][ T937] BTRFS error (device nvme4n1p2): bdev
/dev/nvme4n1p2 errs: wr 1, rd 0, flush 0, corrupt 0, gen 0
[11312.100855][ T937] BTRFS error (device nvme4n1p2): bdev
/dev/nvme4n1p2 errs: wr 2, rd 0, flush 0, corrupt 0, gen 0
[11338.724061][ T624] BTRFS error (device nvme4n1p2 state A):
Transaction aborted (error -5)
[11338.724075][ T624] BTRFS: error (device nvme4n1p2 state A) in
__btrfs_update_delayed_inode:1096: errno=-5 IO failure
# ls
ls: reading directory '.': Input/output error
In the current nvme module reference count increase/decrease logic, the
operation is skipped when the device is a multipath device. As a result,
the NVMe module reference count remains zero even though the NVMe device
is actively being used by the multipath stack.
static int nvme_ns_open(struct nvme_ns *ns)
{
/* should never be called due to GENHD_FL_HIDDEN */
if (WARN_ON_ONCE(nvme_ns_head_multipath(ns->head)))
goto fail;
if (!nvme_get_ns(ns))
goto fail;
if (!try_module_get(ns->ctrl->ops->module))
goto fail_put_ns;
This patch adds module reference count increase/decrease logic in
nvme_ns_head_open() and nvme_ns_head_release() for the multipath head
device. This mirrors the behavior of nvme_ns_open() for non-multipath
devices.
Thanks,
Wendy
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] nvme: Add module reference counting for multipath nvme device
2026-08-10 23:50 [PATCH] nvme: Add module reference counting for multipath nvme device wenxiong
2026-08-11 13:18 ` John Garry
@ 2026-08-11 19:12 ` Keith Busch
2026-08-11 20:33 ` Wen Xiong
1 sibling, 1 reply; 7+ messages in thread
From: Keith Busch @ 2026-08-11 19:12 UTC (permalink / raw)
To: wenxiong; +Cc: linux-nvme, gjoyce, wenxiong
On Mon, Aug 10, 2026 at 07:50:34PM -0400, wenxiong@linux.ibm.com wrote:
> static int nvme_ns_head_open(struct gendisk *disk, blk_mode_t mode)
> {
> - if (!nvme_tryget_ns_head(disk->private_data))
> + struct nvme_ns_head *head = disk->private_data;
> + struct nvme_ns *ns;
> + int srcu_idx;
> +
> + if (!nvme_tryget_ns_head(head))
> return -ENXIO;
> +
> + /* Get module reference from any available path */
> + srcu_idx = srcu_read_lock(&head->srcu);
> + ns = nvme_find_path(head);
> + if (ns && !try_module_get(ns->ctrl->ops->module)) {
> + srcu_read_unlock(&head->srcu, srcu_idx);
> + nvme_put_ns_head(head);
> + return -ENXIO;
> + }
> + srcu_read_unlock(&head->srcu, srcu_idx);
> +
> return 0;
> }
>
> static void nvme_ns_head_release(struct gendisk *disk)
> {
> - nvme_put_ns_head(disk->private_data);
> + struct nvme_ns_head *head = disk->private_data;
> + struct nvme_ns *ns;
> + int srcu_idx;
> +
> + srcu_idx = srcu_read_lock(&head->srcu);
> + ns = nvme_find_path(head);
> + if (ns)
> + module_put(ns->ctrl->ops->module);
> + srcu_read_unlock(&head->srcu, srcu_idx);
> +
> + nvme_put_ns_head(head);
> }
The result of nvme_find_path() is just the current path at the moment,
so I don't think this is right.
I think you need the head to hold a module reference on every path the
head has, so the module_get/put should be in nvme_mpath_add_disk and
nvme_mpath_remove_disk.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] nvme: Add module reference counting for multipath nvme device
2026-08-11 19:12 ` Keith Busch
@ 2026-08-11 20:33 ` Wen Xiong
2026-08-11 20:49 ` Keith Busch
0 siblings, 1 reply; 7+ messages in thread
From: Wen Xiong @ 2026-08-11 20:33 UTC (permalink / raw)
To: Keith Busch; +Cc: linux-nvme, gjoyce, wenxiong
On 2026-08-11 14:12, Keith Busch wrote:
> The result of nvme_find_path() is just the current path at the moment,
> so I don't think this is right.
>
I thought:
- if policy is NUMA, returns current caches path if optimized.
- Otherwise scans other paths to find a new optimal path.
> I think you need the head to hold a module reference on every path the
> head has, so the module_get/put should be in nvme_mpath_add_disk and
> nvme_mpath_remove_disk.
I will check this.
Thanks for your reviewing!
Wendy
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] nvme: Add module reference counting for multipath nvme device
2026-08-11 20:33 ` Wen Xiong
@ 2026-08-11 20:49 ` Keith Busch
2026-08-12 3:59 ` Wen Xiong
0 siblings, 1 reply; 7+ messages in thread
From: Keith Busch @ 2026-08-11 20:49 UTC (permalink / raw)
To: Wen Xiong; +Cc: linux-nvme, gjoyce, wenxiong
On Tue, Aug 11, 2026 at 03:33:19PM -0500, Wen Xiong wrote:
> On 2026-08-11 14:12, Keith Busch wrote:
>
>
> > The result of nvme_find_path() is just the current path at the moment,
> > so I don't think this is right.
> >
> I thought:
> - if policy is NUMA, returns current caches path if optimized.
> - Otherwise scans other paths to find a new optimal path.
Let's say one path is rdma and the other is tcp. At the time you open,
the RDMA is the current optimal path, so you take a reference on the
nvme-rdma module. But while using this multipath device, the RDMA
connection is lost, so we failover to the TCP path. When you close the
multipath device, you find the TCP path, and then drop a reference on
the nvme-tcp module, leaking the RDMA reference, and underflowing TCP.
> > I think you need the head to hold a module reference on every path the
> > head has, so the module_get/put should be in nvme_mpath_add_disk and
> > nvme_mpath_remove_disk.
> I will check this.
Just a note, unlike I initially thought, nvme_mpath_remove_disk() is not
the mirror teardown function to nvme_mpath_add_disk(), so the path's
module reference put will have to be somewhere else, like
nvme_mpath_remove_sysfs_link().
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] nvme: Add module reference counting for multipath nvme device
2026-08-11 20:49 ` Keith Busch
@ 2026-08-12 3:59 ` Wen Xiong
0 siblings, 0 replies; 7+ messages in thread
From: Wen Xiong @ 2026-08-12 3:59 UTC (permalink / raw)
To: Keith Busch; +Cc: linux-nvme, gjoyce, wenxiong
On 2026-08-11 15:49, Keith Busch wrote:
> Let's say one path is rdma and the other is tcp. At the time you open,
> the RDMA is the current optimal path, so you take a reference on the
> nvme-rdma module. But while using this multipath device, the RDMA
> connection is lost, so we failover to the TCP path. When you close the
> multipath device, you find the TCP path, and then drop a reference on
> the nvme-tcp module, leaking the RDMA reference, and underflowing TCP.
Thanks for explaining this!
> Just a note, unlike I initially thought, nvme_mpath_remove_disk() is
> not
> the mirror teardown function to nvme_mpath_add_disk(), so the path's
> module reference put will have to be somewhere else, like
> nvme_mpath_remove_sysfs_link().
Thanks! I can regenerate the patch as v2 and test it soon.
Can I add module reference get in nvme_mpath_add_sysfs_link() instead of
nvme_mpath_add_disk()?
Thanks,
Wendy
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2026-08-12 3:59 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-10 23:50 [PATCH] nvme: Add module reference counting for multipath nvme device wenxiong
2026-08-11 13:18 ` John Garry
2026-08-11 18:54 ` Wen Xiong
2026-08-11 19:12 ` Keith Busch
2026-08-11 20:33 ` Wen Xiong
2026-08-11 20:49 ` Keith Busch
2026-08-12 3:59 ` Wen Xiong
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.