All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] nvme.h: expend lbaf on nvme_id_ns, lbafe on nvme_zns_id_ns
@ 2021-09-26  4:34 Steven Seungcheol Lee
  2021-09-27 10:24 ` Niklas Cassel
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Steven Seungcheol Lee @ 2021-09-26  4:34 UTC (permalink / raw)
  To: kbusch, axboe, hch, sagi; +Cc: linux-nvme, sc108.lee

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

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 {
  __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

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH] nvme.h: expend lbaf on nvme_id_ns, lbafe on nvme_zns_id_ns
  2021-09-26  4:34 [PATCH] nvme.h: expend lbaf on nvme_id_ns, lbafe on nvme_zns_id_ns Steven Seungcheol Lee
@ 2021-09-27 10:24 ` Niklas Cassel
  2021-09-27 19:23 ` Chaitanya Kulkarni
  2021-09-27 20:03 ` Keith Busch
  2 siblings, 0 replies; 10+ messages in thread
From: Niklas Cassel @ 2021-09-27 10:24 UTC (permalink / raw)
  To: Steven Seungcheol Lee; +Cc: kbusch, axboe, hch, sagi, linux-nvme

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] nvme.h: expend lbaf on nvme_id_ns, lbafe on nvme_zns_id_ns
  2021-09-26  4:34 [PATCH] nvme.h: expend lbaf on nvme_id_ns, lbafe on nvme_zns_id_ns Steven Seungcheol Lee
  2021-09-27 10:24 ` Niklas Cassel
@ 2021-09-27 19:23 ` Chaitanya Kulkarni
  2021-09-27 20:03 ` Keith Busch
  2 siblings, 0 replies; 10+ messages in thread
From: Chaitanya Kulkarni @ 2021-09-27 19:23 UTC (permalink / raw)
  To: linux-nvme

On 9/25/21 9:34 PM, 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
> 
> Signed-off-by: Steven Seungcheol Lee <sc108.lee@@gmail.com>


In its current state Nackd.
_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] nvme.h: expend lbaf on nvme_id_ns, lbafe on nvme_zns_id_ns
  2021-09-26  4:34 [PATCH] nvme.h: expend lbaf on nvme_id_ns, lbafe on nvme_zns_id_ns Steven Seungcheol Lee
  2021-09-27 10:24 ` Niklas Cassel
  2021-09-27 19:23 ` Chaitanya Kulkarni
@ 2021-09-27 20:03 ` Keith Busch
  2021-09-28  2:19   ` Steven Seungcheol Lee
  2 siblings, 1 reply; 10+ messages in thread
From: Keith Busch @ 2021-09-27 20:03 UTC (permalink / raw)
  To: Steven Seungcheol Lee; +Cc: axboe, hch, sagi, linux-nvme

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

In order to ease the driver maintenance burden, we typically don't
update spec fields unless the driver has a real use for them. If this
patch is part of a larger series that eventually does make use of these
new formats, please send this update when that happens.

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] nvme.h: expend lbaf on nvme_id_ns, lbafe on nvme_zns_id_ns
  2021-09-27 20:03 ` Keith Busch
@ 2021-09-28  2:19   ` Steven Seungcheol Lee
  2021-09-28  2:43     ` Keith Busch
  0 siblings, 1 reply; 10+ messages in thread
From: Steven Seungcheol Lee @ 2021-09-28  2:19 UTC (permalink / raw)
  To: Keith Busch; +Cc: axboe, hch, sagi, linux-nvme

@Keith Busch
As I mentioned on commit, Its already been used from kernel source.

@Niklas Cassel
Thanks for the chekcing details, I made mistake for the structure name
since I brought those changes from nvme-cli.

Added more details on commit message about both of you guy's question.


=========== modified commit below=============
modification for supporting more LBA format from the device
if the nvme device does support over 16 LBA format,
current nvme driver will occur error.

lbaf, lbafe currently used on nvme driver
lbaf:
    function nvme_configure_metadata
    function nvme_update_ns_info
    function nvmet_passthru_override_id_ns

lbafe:
    function nvme_update_zone_info

The structure chages based on spec:
    NVMe-NVM-Command-Set-Specification-1.0a-2021.07.26-Ratified
    NVMe-Zoned-Namespace-Command-Set-Specification-1.1a-2021.07.26-Ratified

Signed-off-by: Steven Seungcheol Lee <sc108.lee@gmail.com>
---
 include/linux/nvme.h | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/include/linux/nvme.h b/include/linux/nvme.h
index b7c4c4130b65..2d03c7322ac9 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];
 };

@@ -394,9 +396,14 @@ struct nvme_id_ns_zns {
  __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.25.1

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

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH] nvme.h: expend lbaf on nvme_id_ns, lbafe on nvme_zns_id_ns
  2021-09-28  2:19   ` Steven Seungcheol Lee
@ 2021-09-28  2:43     ` Keith Busch
  2021-09-28  2:51       ` Steven Seungcheol Lee
  0 siblings, 1 reply; 10+ messages in thread
From: Keith Busch @ 2021-09-28  2:43 UTC (permalink / raw)
  To: Steven Seungcheol Lee; +Cc: axboe, hch, sagi, linux-nvme

On Tue, Sep 28, 2021 at 11:19:40AM +0900, Steven Seungcheol Lee wrote:
> @Keith Busch
> As I mentioned on commit, Its already been used from kernel source.

You are introducing new things with his patch. How is the kernel already
using them before they exist?

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] nvme.h: expend lbaf on nvme_id_ns, lbafe on nvme_zns_id_ns
  2021-09-28  2:43     ` Keith Busch
@ 2021-09-28  2:51       ` Steven Seungcheol Lee
  2021-09-28  2:59         ` Keith Busch
  0 siblings, 1 reply; 10+ messages in thread
From: Steven Seungcheol Lee @ 2021-09-28  2:51 UTC (permalink / raw)
  To: Keith Busch; +Cc: axboe, hch, sagi, linux-nvme

I only mean lbaf, lbafe field.
other than lbaf, lbafe fields are not used.
I thought It would be nice to add other fields as well, since the
structure will be changed with lbaf, lbafe.
should I comment about other fields instead of just mentioning spec ?


>
> On Tue, Sep 28, 2021 at 11:19:40AM +0900, Steven Seungcheol Lee wrote:
> > @Keith Busch
> > As I mentioned on commit, Its already been used from kernel source.
>
> You are introducing new things with his patch. How is the kernel already
> using them before they exist?

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] nvme.h: expend lbaf on nvme_id_ns, lbafe on nvme_zns_id_ns
  2021-09-28  2:51       ` Steven Seungcheol Lee
@ 2021-09-28  2:59         ` Keith Busch
  2021-09-28  3:44           ` Steven Seungcheol Lee
  0 siblings, 1 reply; 10+ messages in thread
From: Keith Busch @ 2021-09-28  2:59 UTC (permalink / raw)
  To: Steven Seungcheol Lee; +Cc: axboe, hch, sagi, linux-nvme

On Tue, Sep 28, 2021 at 11:51:30AM +0900, Steven Seungcheol Lee wrote:
> I only mean lbaf, lbafe field.
> other than lbaf, lbafe fields are not used.
> I thought It would be nice to add other fields as well, since the
> structure will be changed with lbaf, lbafe.
> should I comment about other fields instead of just mentioning spec ?

The driver currently can't access the new elements, though, so what's
the point of defining them? If you want to enable these formats, then
this change needs to be part of a larger patch that properly enables the
feature.

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] nvme.h: expend lbaf on nvme_id_ns, lbafe on nvme_zns_id_ns
  2021-09-28  2:59         ` Keith Busch
@ 2021-09-28  3:44           ` Steven Seungcheol Lee
  2021-09-28  8:45             ` Steven Seungcheol Lee
  0 siblings, 1 reply; 10+ messages in thread
From: Steven Seungcheol Lee @ 2021-09-28  3:44 UTC (permalink / raw)
  To: Keith Busch; +Cc: axboe, hch, sagi, linux-nvme

That means leave other elements and fix lbf only?

The changes for lbaf, lbafe happens.
so I thought same structure should be updated as same version of spec.

If not recommanded, I wll change as below.

id_ns
mssrl, mcl, msrc => remove
rsvd area -> lbaf[64]

id_zns
rrl1~3, frl1~3 => remove
rsvd area -> lbafe[64]

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] nvme.h: expend lbaf on nvme_id_ns, lbafe on nvme_zns_id_ns
  2021-09-28  3:44           ` Steven Seungcheol Lee
@ 2021-09-28  8:45             ` Steven Seungcheol Lee
  0 siblings, 0 replies; 10+ messages in thread
From: Steven Seungcheol Lee @ 2021-09-28  8:45 UTC (permalink / raw)
  To: Keith Busch; +Cc: axboe, hch, sagi, linux-nvme

Sorry guys misunderstood, I just realized that changes need other
functionality that you guys mention.
Thanks for the chekcing :)
Just ignore this patch ~

2021년 9월 28일 (화) 오후 12:44, Steven Seungcheol Lee <sc108.lee@gmail.com>님이 작성:
>
> That means leave other elements and fix lbf only?
>
> The changes for lbaf, lbafe happens.
> so I thought same structure should be updated as same version of spec.
>
> If not recommanded, I wll change as below.
>
> id_ns
> mssrl, mcl, msrc => remove
> rsvd area -> lbaf[64]
>
> id_zns
> rrl1~3, frl1~3 => remove
> rsvd area -> lbafe[64]

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2021-09-28  8:45 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-26  4:34 [PATCH] nvme.h: expend lbaf on nvme_id_ns, lbafe on nvme_zns_id_ns Steven Seungcheol Lee
2021-09-27 10:24 ` Niklas Cassel
2021-09-27 19:23 ` Chaitanya Kulkarni
2021-09-27 20:03 ` Keith Busch
2021-09-28  2:19   ` Steven Seungcheol Lee
2021-09-28  2:43     ` Keith Busch
2021-09-28  2:51       ` Steven Seungcheol Lee
2021-09-28  2:59         ` Keith Busch
2021-09-28  3:44           ` Steven Seungcheol Lee
2021-09-28  8:45             ` Steven Seungcheol Lee

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.