[PATCH 0/2] minor changes regarding multipath-head assertion

[PATCH 0/2] minor changes regarding multipath-head assertion

[Irvin Cote]

Irvin Cote (2):
  nvme-core: remove redundant condition in nvme_ns_head_multipath
  nvme-core: use nvme_ns_head_multipath instead of ns->head->disk

 drivers/nvme/host/core.c | 2 +-
 drivers/nvme/host/nvme.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

-- 
2.39.2

[PATCH 1/2] nvme-core: remove redundant condition in nvme_ns_head_multipath

[Irvin Cote]

head->disk implies IS_ENABLED(CONFIG_NVME_MULTIPATH) (c.f
nvme_mpath_alloc_disk) hence the condition is redundant.

Signed-off-by: Irvin Cote <irvincoteg@gmail.com>
---
 drivers/nvme/host/nvme.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index bf46f122e9e1..7820e40cdce4 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -453,7 +453,7 @@ struct nvme_ns_head {
 
 static inline bool nvme_ns_head_multipath(struct nvme_ns_head *head)
 {
-	return IS_ENABLED(CONFIG_NVME_MULTIPATH) && head->disk;
+	return head->disk;
 }
 
 enum nvme_ns_features {
-- 
2.39.2

[PATCH 2/2] nvme-core: use nvme_ns_head_multipath instead of ns->head->disk

[Irvin Cote]

Change the way we check for a multipath nshead so as
to consistently use the same check to assert the same condition.

Signed-off-by: Irvin Cote <irvincoteg@gmail.com>
---
 drivers/nvme/host/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index ccb6eb1282f8..c6889cf4439d 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -4310,7 +4310,7 @@ static void nvme_alloc_ns(struct nvme_ctrl *ctrl, struct nvme_ns_info *info)
 	 * instance as shared namespaces will show up as multiple block
 	 * devices.
 	 */
-	if (ns->head->disk) {
+	if (nvme_ns_head_multipath(ns->head)) {
 		sprintf(disk->disk_name, "nvme%dc%dn%d", ctrl->subsys->instance,
 			ctrl->instance, ns->head->instance);
 		disk->flags |= GENHD_FL_HIDDEN;
-- 
2.39.2

Re: [PATCH 1/2] nvme-core: remove redundant condition in nvme_ns_head_multipath

[Jens Axboe]

On 5/18/23 4:10?PM, Irvin Cote wrote:
> head->disk implies IS_ENABLED(CONFIG_NVME_MULTIPATH) (c.f
> nvme_mpath_alloc_disk) hence the condition is redundant.
> 
> Signed-off-by: Irvin Cote <irvincoteg@gmail.com>
> ---
>  drivers/nvme/host/nvme.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
> index bf46f122e9e1..7820e40cdce4 100644
> --- a/drivers/nvme/host/nvme.h
> +++ b/drivers/nvme/host/nvme.h
> @@ -453,7 +453,7 @@ struct nvme_ns_head {
>  
>  static inline bool nvme_ns_head_multipath(struct nvme_ns_head *head)
>  {
> -	return IS_ENABLED(CONFIG_NVME_MULTIPATH) && head->disk;
> +	return head->disk;
>  }

Might not matter for this case, but this changes something that'd
compile away for !CONFIG_NVME_MULTIPATH to now needing to read
disk->head instead. That doesn't seem like a good change.

-- 
Jens Axboe

Re: [PATCH 1/2] nvme-core: remove redundant condition in nvme_ns_head_multipath

[Christoph Hellwig]

On Thu, May 18, 2023 at 08:12:23PM -0600, Jens Axboe wrote:
> >  static inline bool nvme_ns_head_multipath(struct nvme_ns_head *head)
> >  {
> > -	return IS_ENABLED(CONFIG_NVME_MULTIPATH) && head->disk;
> > +	return head->disk;
> >  }
> 
> Might not matter for this case, but this changes something that'd
> compile away for !CONFIG_NVME_MULTIPATH to now needing to read
> disk->head instead. That doesn't seem like a good change.

Exactly.  That's the reason why this helper and check exist.

Re: [PATCH 2/2] nvme-core: use nvme_ns_head_multipath instead of ns->head->disk

[Christoph Hellwig]

On Thu, May 18, 2023 at 07:10:54PM -0300, Irvin Cote wrote:
> Change the way we check for a multipath nshead so as
> to consistently use the same check to assert the same condition.
> 
> Signed-off-by: Irvin Cote <irvincoteg@gmail.com>

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 2/2] nvme-core: use nvme_ns_head_multipath instead of ns->head->disk

[Keith Busch]

Thanks, applied to nvme-6.5, skipping patch 1/2.