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

[John Garry <john.g.garry@oracle.com>]

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
>