Re: [PATCH] nvme.h: expend lbaf on nvme_id_ns, lbafe on nvme_zns_id_ns

From: Niklas Cassel <Niklas.Cassel@wdc.com>
To: Steven Seungcheol Lee <sc108.lee@gmail.com>
Cc: "kbusch@kernel.org" <kbusch@kernel.org>,
	"axboe@fb.com" <axboe@fb.com>, "hch@lst.de" <hch@lst.de>,
	"sagi@grimberg.me" <sagi@grimberg.me>,
	"linux-nvme@lists.infradead.org" <linux-nvme@lists.infradead.org>
Subject: Re: [PATCH] nvme.h: expend lbaf on nvme_id_ns, lbafe on nvme_zns_id_ns
Date: Mon, 27 Sep 2021 10:24:32 +0000	[thread overview]
Message-ID: <YVGberwHSmN/HSjR@x1-carbon> (raw)
In-Reply-To: <CAGrZSEyrdJaZkHX9fPse0bv3d1x-HU7vvPQeg5JyR++_hOs8bQ@mail.gmail.com>

On Sun, Sep 26, 2021 at 01:34:10PM +0900, Steven Seungcheol Lee wrote:
> lbaf, lbafe used on nvme driver
> Base spec:
>     NVMe-NVM-Command-Set-Specification-1.0a-2021.07.26-Ratified
>     NVMe-Zoned-Namespace-Command-Set-Specification-1.1a-2021.07.26-Ratified

Please read:
https://www.kernel.org/doc/html/latest/process/submitting-patches.html#describe-your-changes

You should explain what the patch changes, and why the patch is necessary.

> 
> Signed-off-by: Steven Seungcheol Lee <sc108.lee@@gmail.com>
> ---
>  include/linux/nvme.h | 21 ++++++++++++++-------
>  1 file changed, 14 insertions(+), 7 deletions(-)
> 
> diff --git a/include/linux/nvme.h b/include/linux/nvme.h
> index b7c4c4130b65..c11eda4be426 100644
> --- a/include/linux/nvme.h
> +++ b/include/linux/nvme.h
> @@ -368,7 +368,10 @@ struct nvme_id_ns {
>   __le16 npdg;
>   __le16 npda;
>   __le16 nows;
> - __u8 rsvd74[18];
> + __le16 mssrl;
> + __le32 mcl;
> + __u8 msrc;
> + __u8 rsvd81[11];
>   __le32 anagrpid;
>   __u8 rsvd96[3];
>   __u8 nsattr;
> @@ -376,8 +379,7 @@ struct nvme_id_ns {
>   __le16 endgid;
>   __u8 nguid[16];
>   __u8 eui64[8];
> - struct nvme_lbaf lbaf[16];
> - __u8 rsvd192[192];
> + struct nvme_lbaf lbaf[64];
>   __u8 vs[3712];
>  };
> 
> @@ -387,16 +389,21 @@ struct nvme_zns_lbafe {
>   __u8 rsvd9[7];
>  };
> 
> -struct nvme_id_ns_zns {
> +struct nvme_zns_id_ns {

You cannot simply rename the struct like this.

You didn't provide any reason for renaming it in the changelog..

..and even if you provided a really good reason for renaming it,
you didn't change the variables that uses this struct, so patch
should not compile.

>   __le16 zoc;
>   __le16 ozcs;
>   __le32 mar;
>   __le32 mor;
>   __le32 rrl;
>   __le32 frl;
> - __u8 rsvd20[2796];
> - struct nvme_zns_lbafe lbafe[16];
> - __u8 rsvd3072[768];
> + __le32 rrl1;
> + __le32 rrl2;
> + __le32 rrl3;
> + __le32 frl1;
> + __le32 frl2;
> + __le32 frl3;
> + __u8 rsvd44[2772];
> + struct nvme_zns_lbafe lbafe[64];
>   __u8 vs[256];
>  };
> 
> --
> 2.17.0.windows.1
> 
> _______________________________________________
> Linux-nvme mailing list
> Linux-nvme@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-nvme
_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme