[PATCH] nvme: Add module reference counting for multipath nvme device

[PATCH] nvme: Add module reference counting for multipath nvme device

[wenxiong]

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

Re: [PATCH] nvme: Add module reference counting for multipath nvme device

[John Garry]

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
> 

Re: [PATCH] nvme: Add module reference counting for multipath nvme device

[Wen Xiong]

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

Re: [PATCH] nvme: Add module reference counting for multipath nvme device

[John Garry]

On 11/08/2026 19:54, Wen Xiong 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.
> 

Can't the tester also just unbind the device from the driver, which does 
the same (as unloading the driver)?

Example:
echo 0000:00:04.0  > /sys/bus/pci/drivers/nvme/unbind

For this PCI case, we call nvme_remove() -> nvme_remove_namespaces() -> 
nvme_ns_remove(), which removes the NS path and deletes the gendisk if 
the last path. Same effect, as I see.

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

Re: [PATCH] nvme: Add module reference counting for multipath nvme device

[Wen Xiong]

On 2026-08-12 03:43, John Garry wrote:

> Can't the tester also just unbind the device from the driver, which
> does the same (as unloading the driver)?
> 
> Example:
> echo 0000:00:04.0  > /sys/bus/pci/drivers/nvme/unbind
> 
> For this PCI case, we call nvme_remove() -> nvme_remove_namespaces()
> -> nvme_ns_remove(), which removes the NS path and deletes the gendisk
> if the last path. Same effect, as I see.
> 
Yes. Both rmmod nvme and unbind are test cases for the tester.

rmmod nvme should not be allowed when the module reference count is 
non-zero. However, unbind is allowed even when the reference count is 
non-zero; it should decrease the reference count.

Re: [PATCH] nvme: Add module reference counting for multipath nvme device

[Keith Busch]

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.

Re: [PATCH] nvme: Add module reference counting for multipath nvme device

[Wen Xiong]

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

Re: [PATCH] nvme: Add module reference counting for multipath nvme device

[Keith Busch]

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().

Re: [PATCH] nvme: Add module reference counting for multipath nvme device

[Wen Xiong]

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