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

[Nilay Shroff <nilay@linux.ibm.com>]

On 8/13/26 4:02 AM, wenxiong@linux.ibm.com wrote:
> From: Wen Xiong <wenxiong@linux.ibm.com>
> 
> Add proper module reference counting to prevent premature unloading of
> NVMe transport modules while multipath namespaces are still active.
> 
> When a namespace is added to a multipath device via nvme_mpath_add_disk(),
> the underlying transport module (PCIe, FC, RDMA, TCP, etc.) must remain
> loaded as long as the multipath device references that namespace. Without
> proper reference counting, the transport module could be unloaded while
> the multipath device is still using resources from that module, leading
> to the potential system crashes.
> 
> This ensures the transport module remains loaded for the entire lifetime
> of the multipath namespace association.
> 
> Signed-off-by: Wen Xiong <wenxiong@linux.ibm.com>
> ---
>   drivers/nvme/host/multipath.c | 5 +++++
>   1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipath.c
> index 9b9a657fa330..707b8f95727d 100644
> --- a/drivers/nvme/host/multipath.c
> +++ b/drivers/nvme/host/multipath.c
> @@ -1348,6 +1348,8 @@ void nvme_mpath_remove_sysfs_link(struct nvme_ns *ns)
>   	sysfs_remove_link_from_group(kobj, nvme_ns_mpath_attr_group.name,
>   			dev_name(target));
>   	clear_bit(NVME_NS_SYSFS_ATTR_LINK, &ns->flags);
> +
> +	module_put(ns->ctrl->ops->module);
>   }
>   
>   void nvme_mpath_add_disk(struct nvme_ns *ns, __le32 anagrpid)
> @@ -1379,6 +1381,9 @@ void nvme_mpath_add_disk(struct nvme_ns *ns, __le32 anagrpid)
>   	if (blk_queue_is_zoned(ns->queue) && ns->head->disk)
>   		ns->head->disk->nr_zones = ns->disk->nr_zones;
>   #endif
> +	if (!try_module_get(ns->ctrl->ops->module))
> +		dev_err(disk_to_dev(ns->disk),
> +				"Failed to get module reference\n");
>   }
>   
>   void nvme_mpath_remove_disk(struct nvme_ns_head *head)

I know I am late to the party here, but I don't think adding the
module refcounting in nvme_mpath_{add|remove}_sysfs_link() is the
right approach. That would increment the transport module reference
for every namespace/path, which means the transport module could
not be unloaded until all namespaces using it are deleted, even
when none of those namespaces are actually in use. So for instance,
with this change now, I can't rmmod nvme.ko until I delete
all namespaces/paths created under PCIe nvme subsystem, even though
no one is using the namespace/path under that subsystem.

Instead, I think the module reference should track the lifetime of an
active user of the multipath namespace. When the namespace is in use,
the corresponding transport module(s) need to remain loaded and once
the namespace is no longer in use, those references should be released.

Looking at the current code, nvme_ns_head_{open|release}() seems to be
a more appropriate place for this. When a multipath namespace is opened,
nvme_ns_head_open() could take the required transport module references,
and nvme_ns_head_release() could drop them when the last user releases
the namespace.

Also, since a multipath head can have paths through different transports
, we should not take a reference only to the transport of one namespace/path
found through nvme_find_path(), as was done in v1. Instead, when opening the
head, take a module reference iterating through each controller reachable
from the corresponding NVMe subsystem. This ensures that every transport
that can service I/O for the active multipath namespace remains loaded for
the duration of its use.

Thanks,
--Nilay