[PATCH 1/1] nvme/host: Rescan namespace if ns_id is changed

[Hannes Reinecke]

On 11/14/23 22:57, wenxiong at linux.ibm.com wrote:
> From: Wen Xiong <wenxiong at linux.ibm.com>
> 
> Hi all,
> 
> Namespace is not visible if we detach/delete/create/attach a namespace
> on a new nvme device because of nguid is changed.
> 
> Like Keith's suggestion, we can do the 2nd ns-attach to get new
> namespace available. Test team likes to fix it in the driver.
> 
> The following patch has been tested by test team and works fine.
> 
> Appreciate your suggestion for difference approach!
> 
> Thanks,
> Wendy
> 
> Signed-off-by: Wen Xiong <wenxiong at linux.ibm.com>
> Tested-by: tjo at us.ibm.com
> 
> ---
>   drivers/nvme/host/core.c | 23 ++++++++++++++++-------
>   1 file changed, 16 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> index 88b54cdcbd68..a608a63d3108 100644
> --- a/drivers/nvme/host/core.c
> +++ b/drivers/nvme/host/core.c
> @@ -3721,13 +3721,15 @@ static void nvme_ns_remove_by_nsid(struct nvme_ctrl *ctrl, u32 nsid)
>   	}
>   }
>   
> -static void nvme_validate_ns(struct nvme_ns *ns, struct nvme_ns_info *info)
> +static bool nvme_validate_ns(struct nvme_ns *ns, struct nvme_ns_info *info)
>   {
>   	int ret = NVME_SC_INVALID_NS | NVME_SC_DNR;
> +	bool ns_id_changed = false;
>   
>   	if (!nvme_ns_ids_equal(&ns->head->ids, &info->ids)) {
>   		dev_err(ns->ctrl->device,
>   			"identifiers changed for nsid %d\n", ns->head->ns_id);
> +		ns_id_changed = true;
>   		goto out;
>   	}
>   
> @@ -3741,21 +3743,24 @@ static void nvme_validate_ns(struct nvme_ns *ns, struct nvme_ns_info *info)
>   	 */
>   	if (ret > 0 && (ret & NVME_SC_DNR))
>   		nvme_ns_remove(ns);
> +
> +	return ns_id_changed;
>   }
>   
> -static void nvme_scan_ns(struct nvme_ctrl *ctrl, unsigned nsid)
> +static bool nvme_scan_ns(struct nvme_ctrl *ctrl, unsigned nsid)
>   {
>   	struct nvme_ns_info info = { .nsid = nsid };
>   	struct nvme_ns *ns;
>   	int ret;
> +	bool ns_rescan = false;
>   
>   	if (nvme_identify_ns_descs(ctrl, &info))
> -		return;
> +		return false;
>   
>   	if (info.ids.csi != NVME_CSI_NVM && !nvme_multi_css(ctrl)) {
>   		dev_warn(ctrl->device,
>   			"command set not reported for nsid: %d\n", nsid);
> -		return;
> +		return false;
>   	}
>   
>   	/*
> @@ -3777,15 +3782,17 @@ static void nvme_scan_ns(struct nvme_ctrl *ctrl, unsigned nsid)
>   	 * becomes ready and restart the scan.
>   	 */
>   	if (ret || !info.is_ready)
> -		return;
> +		return false;
>   
>   	ns = nvme_find_get_ns(ctrl, nsid);
>   	if (ns) {
> -		nvme_validate_ns(ns, &info);
> +		ns_rescan = nvme_validate_ns(ns, &info);
>   		nvme_put_ns(ns);
>   	} else {
>   		nvme_alloc_ns(ctrl, &info);
>   	}
> +
> +	return ns_rescan;
>   }
>   
>   static void nvme_remove_invalid_namespaces(struct nvme_ctrl *ctrl,
> @@ -3837,7 +3844,9 @@ static int nvme_scan_ns_list(struct nvme_ctrl *ctrl)
>   
>   			if (!nsid)	/* end of the list? */
>   				goto out;
> -			nvme_scan_ns(ctrl, nsid);
> +			if (nvme_scan_ns(ctrl, nsid))
> +				nvme_scan_ns(ctrl, nsid);
> +
>   			while (++prev < nsid)
>   				nvme_ns_remove_by_nsid(ctrl, prev);
>   		}
No. If the namespace has changed we should abort scanning altogether, as 
there is no guarantee that the _previous_ namespaces in the list have 
not changed.
We should rather abort and wait for the NS changed AEN to be signalled, 
which will trigger a rescan anyway.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		           Kernel Storage Architect
hare at suse.de			                  +49 911 74053 688
SUSE Software Solutions Germany GmbH, Frankenstr. 146, 90461 Nürnberg
Managing Directors: I. Totev, A. Myers, A. McDonald, M. B. Moerman
(HRB 36809, AG Nürnberg)