linux-kernel - Re: [PATCH] nvme: remove multipath module parameter

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <b37a4f6c-95a9-430d-89f9-141b72fd6be8@redhat.com>
Date: Tue, 18 Feb 2025 09:26:46 -0500
From: John Meneghini <jmeneghi@...hat.com>
To: Bryan Gurney <bgurney@...hat.com>, linux-kernel@...r.kernel.org,
 linux-nvme@...ts.infradead.org, kbusch@...nel.org, hch@....de,
 sagi@...mberg.me, axboe@...nel.dk
Cc: bmarzins@...hat.com
Subject: Re: [PATCH] nvme: remove multipath module parameter

Bryan, since we are re-spinning this patch please make the changes below.

On 2/4/25 4:11 PM, Bryan Gurney wrote:
> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> index 2bcd9f710cb6..b07cd482fbc1 100644
> --- a/drivers/nvme/host/core.c
> +++ b/drivers/nvme/host/core.c
> @@ -3809,14 +3809,6 @@ static int nvme_init_ns_head(struct nvme_ns *ns, struct nvme_ns_info *info)
>   					info->nsid);
>   			goto out_put_ns_head;
>   		}
> -
> -		if (!multipath) {
> -			dev_warn(ctrl->device,
> -				"Found shared namespace %d, but multipathing not supported.\n",
> -				info->nsid);
> -			dev_warn_once(ctrl->device,
> -				"Support for shared namespaces without CONFIG_NVME_MULTIPATH is deprecated and will be removed in Linux 6.0.\n");
> -		}

I think we want to keep a similar warning like the above, only move it down below.

Let's wait for Sagi's approval of the exact message.

>   	}
>   
>   	list_add_tail_rcu(&ns->siblings, &head->list);
> @@ -3915,12 +3907,14 @@ static void nvme_alloc_ns(struct nvme_ctrl *ctrl, struct nvme_ns_info *info)
>   		sprintf(disk->disk_name, "nvme%dc%dn%d", ctrl->subsys->instance,
>   			ctrl->instance, ns->head->instance);
>   		disk->flags |= GENHD_FL_HIDDEN;
> -	} else if (multipath) {
> +	} else {
> +#ifdef CONFIG_NVME_MULTIPATH
>   		sprintf(disk->disk_name, "nvme%dn%d", ctrl->subsys->instance,
>   			ns->head->instance);
> -	} else {
 > +#else

You don't need the this #ifdef conditional statement because the nvme_ns_head_multipath() function checks CONFIG_NVME_MULTIPATH
for you and the sprintf(disk->disk_name, "nvme%dn%d" statement is kind of redundant.


>   		sprintf(disk->disk_name, "nvme%dn%d", ctrl->instance,
>   			ns->head->instance);
> +#endif
>   	}
>   
>   	if (nvme_update_ns_info(ns, info))

Thanks,

/John