Re: [PATCH] Revert "nvme: verify MNAN value if ANA is enabled"

From: Daniel Wagner <dwagner@suse.de>
To: Chaitanya Kulkarni <Chaitanya.Kulkarni@wdc.com>
Cc: "linux-nvme@lists.infradead.org" <linux-nvme@lists.infradead.org>
Subject: Re: [PATCH] Revert "nvme: verify MNAN value if ANA is enabled"
Date: Thu, 10 Jun 2021 09:45:46 +0200	[thread overview]
Message-ID: <20210610074546.jyqjalpldii6reg6@beryllium.lan> (raw)
In-Reply-To: <BYAPR04MB49655A9B821FEE29D7A89E3986359@BYAPR04MB4965.namprd04.prod.outlook.com>

Hi Chaitanya,

On Thu, Jun 10, 2021 at 02:55:08AM +0000, Chaitanya Kulkarni wrote:
> On 6/9/21 19:45, Chaitanya Kulkarni wrote:
> > This reverts commit 8872c159c7a83daf633768cee7a7ef7154010341. This is
> > needed to move forward with the blktests for now, without this patch
> > all the testcases result in the error :-
> >
> > [ 3502.072798] nvme nvme1: Invalid MNAN value 1024
> 
> Thinking about the code again I think following should work :-
> 
> diff --git a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipath.c
> index 23573fe3fc7d..4277f1554bd5 100644
> --- a/drivers/nvme/host/multipath.c
> +++ b/drivers/nvme/host/multipath.c
> @@ -813,7 +813,7 @@ int nvme_mpath_init_identify(struct nvme_ctrl *ctrl,
> struct nvme_id_ctrl *id)
>             !(ctrl->subsys->cmic & NVME_CTRL_CMIC_ANA))
>                 return 0;
>  
> -       if (!ctrl->max_namespaces ||
> +       if (ctrl->max_namespaces &&
>             ctrl->max_namespaces > le32_to_cpu(id->nn)) {
>                 dev_err(ctrl->device,
>                         "Invalid MNAN value %u\n", ctrl->max_namespaces);

'!ctrl->max_namespace' could also be written as
'ctrl->max_namespace != 0' which makes it more obvious what the intend
is here:

  If the controller supports Asymmetric Namespace Access Reporting, then
  this field shall be set to a non-zero value that is less than or equal
  to the NN value.

> diff --git a/drivers/nvme/target/admin-cmd.c
> b/drivers/nvme/target/admin-cmd.c
> index cd60a8184d04..a8ec377bb68d 100644
> --- a/drivers/nvme/target/admin-cmd.c
> +++ b/drivers/nvme/target/admin-cmd.c
> @@ -394,7 +394,7 @@ static void nvmet_execute_identify_ctrl(struct
> nvmet_req *req)
>         id->maxcmd = cpu_to_le16(NVMET_MAX_CMD);
>  
>         id->nn = cpu_to_le32(ctrl->subsys->max_nsid);
> -       id->mnan = cpu_to_le32(NVMET_MAX_NAMESPACES);
> +       id->mnan = cpu_to_le32(ctrl->subsys->max_nsid);
>         id->oncs = cpu_to_le16(NVME_CTRL_ONCS_DSM |
>                         NVME_CTRL_ONCS_WRITE_ZEROES);

This looks like the right fix for the upper limit problem.
I can see the tests are still failing with

  nvme nvme0: Invalid MNAN value 0

which indicates there is another problem in the nvmet core.

I try to find it.x

Thanks,
Daniel

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme