[PATCH 1/4] nvme: simplify error logic in nvme_validate_ns()

[Sagi Grimberg]

On 2/25/21 11:17 PM, Hannes Reinecke wrote:
> We only should remove namespaces when we get fatal error back from
> the device or when the namespace IDs have changed.
> So instead of painfully masking out error numbers which might indicate
> that the error should be ignored we could use an NVME status code
> to indicated when the namespace should be removed.
> That simplifies the final logic and makes it less error-prone.
> 
> Signed-off-by: Hannes Reinecke <hare at suse.de>
> Reviewed-by: Keith Busch <kbusch at kernel.org>
> ---
>   drivers/nvme/host/core.c | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> index d77f3f26d8d3..bde1b22ca7f2 100644
> --- a/drivers/nvme/host/core.c
> +++ b/drivers/nvme/host/core.c
> @@ -1439,7 +1439,7 @@ static int nvme_identify_ns(struct nvme_ctrl *ctrl, unsigned nsid,
>   		goto out_free_id;
>   	}
>   
> -	error = -ENODEV;
> +	error = NVME_SC_INVALID_NS | NVME_SC_DNR;
>   	if ((*id)->ncap == 0) /* namespace not allocated or attached */
>   		goto out_free_id;
>   
> @@ -4036,7 +4036,7 @@ 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_ids *ids)
>   {
>   	struct nvme_id_ns *id;
> -	int ret = -ENODEV;
> +	int ret = NVME_SC_INVALID_NS | NVME_SC_DNR;
>   
>   	if (test_bit(NVME_NS_DEAD, &ns->flags))
>   		goto out;
> @@ -4045,7 +4045,7 @@ static void nvme_validate_ns(struct nvme_ns *ns, struct nvme_ns_ids *ids)
>   	if (ret)
>   		goto out;
>   
> -	ret = -ENODEV;
> +	ret = NVME_SC_INVALID_NS | NVME_SC_DNR;
>   	if (!nvme_ns_ids_equal(&ns->head->ids, ids)) {
>   		dev_err(ns->ctrl->device,
>   			"identifiers changed for nsid %d\n", ns->head->ns_id);
> @@ -4063,7 +4063,7 @@ static void nvme_validate_ns(struct nvme_ns *ns, struct nvme_ns_ids *ids)
>   	 *
>   	 * TODO: we should probably schedule a delayed retry here.
>   	 */
> -	if (ret && ret != -ENOMEM && !(ret > 0 && !(ret & NVME_SC_DNR)))
> +	if (ret > 0 && (ret & NVME_SC_DNR))
>   		nvme_ns_remove(ns);
>   }

This looks ok to me,

Reviewed-by: Sagi Grimberg <sagi at grimberg.me>