| Message ID | 20210310095347.682395-10-its@irrelevant.dk |
|---|---|
| State | New |
| Headers | show |
| Series | hw/block/nvme: metadata and end-to-end data protection support | expand |
On 21-03-10 10:53:43, Klaus Jensen wrote: > From: Klaus Jensen <k.jensen@samsung.com> > > Provide a more flexible nlbas helper. > > Signed-off-by: Klaus Jensen <k.jensen@samsung.com> > --- > hw/block/nvme-ns.h | 14 ++++++++++---- > 1 file changed, 10 insertions(+), 4 deletions(-) > > diff --git a/hw/block/nvme-ns.h b/hw/block/nvme-ns.h > index 07e16880801d..34f9474a1cd1 100644 > --- a/hw/block/nvme-ns.h > +++ b/hw/block/nvme-ns.h > @@ -136,12 +136,18 @@ static inline bool nvme_ns_ext(NvmeNamespace *ns) > } > > /* calculate the number of LBAs that the namespace can accomodate */ > +static inline uint64_t __nvme_nlbas(size_t size, uint8_t lbads, uint16_t ms) > +{ > + if (ms) { > + return size / ((1 << lbads) + ms); > + } > + > + return size >> lbads; > +} > + > static inline uint64_t nvme_ns_nlbas(NvmeNamespace *ns) > { > - if (nvme_msize(ns)) { > - return ns->size / (nvme_lsize(ns) + nvme_msize(ns)); > - } > - return ns->size >> nvme_ns_lbads(ns); > + return __nvme_nlbas(ns->size, nvme_ns_lbads(ns), nvme_msize(ns)); > } Hmm.. I think it looks like __nvme_nlbas does the same with the nvme_ns_nlbas, but flexible argument attributes. But I think those three attributes are all for ns-specific fields which is not that generic so that I don't think we are going to take the helper from much more general perspective with __nvme_nlbas.
On Mar 16 15:53, Minwoo Im wrote: > On 21-03-10 10:53:43, Klaus Jensen wrote: > > From: Klaus Jensen <k.jensen@samsung.com> > > > > Provide a more flexible nlbas helper. > > > > Signed-off-by: Klaus Jensen <k.jensen@samsung.com> > > --- > > hw/block/nvme-ns.h | 14 ++++++++++---- > > 1 file changed, 10 insertions(+), 4 deletions(-) > > > > diff --git a/hw/block/nvme-ns.h b/hw/block/nvme-ns.h > > index 07e16880801d..34f9474a1cd1 100644 > > --- a/hw/block/nvme-ns.h > > +++ b/hw/block/nvme-ns.h > > @@ -136,12 +136,18 @@ static inline bool nvme_ns_ext(NvmeNamespace *ns) > > } > > > > /* calculate the number of LBAs that the namespace can accomodate */ > > +static inline uint64_t __nvme_nlbas(size_t size, uint8_t lbads, uint16_t ms) > > +{ > > + if (ms) { > > + return size / ((1 << lbads) + ms); > > + } > > + > > + return size >> lbads; > > +} > > + > > static inline uint64_t nvme_ns_nlbas(NvmeNamespace *ns) > > { > > - if (nvme_msize(ns)) { > > - return ns->size / (nvme_lsize(ns) + nvme_msize(ns)); > > - } > > - return ns->size >> nvme_ns_lbads(ns); > > + return __nvme_nlbas(ns->size, nvme_ns_lbads(ns), nvme_msize(ns)); > > } > > Hmm.. I think it looks like __nvme_nlbas does the same with the > nvme_ns_nlbas, but flexible argument attributes. But I think those > three attributes are all for ns-specific fields which is not that > generic so that I don't think we are going to take the helper from much > more general perspective with __nvme_nlbas. > This patch should be moved two patches forward in the series - it is used in [12/13] to check the zone geometry before the values are set on the namespace proper. This is also used in Format NVM to verify the format before formatting ("commiting" the values on the NvmeNamespace structure).
On 21-03-16 08:19:08, Klaus Jensen wrote: > On Mar 16 15:53, Minwoo Im wrote: > > On 21-03-10 10:53:43, Klaus Jensen wrote: > > > From: Klaus Jensen <k.jensen@samsung.com> > > > > > > Provide a more flexible nlbas helper. > > > > > > Signed-off-by: Klaus Jensen <k.jensen@samsung.com> > > > --- > > > hw/block/nvme-ns.h | 14 ++++++++++---- > > > 1 file changed, 10 insertions(+), 4 deletions(-) > > > > > > diff --git a/hw/block/nvme-ns.h b/hw/block/nvme-ns.h > > > index 07e16880801d..34f9474a1cd1 100644 > > > --- a/hw/block/nvme-ns.h > > > +++ b/hw/block/nvme-ns.h > > > @@ -136,12 +136,18 @@ static inline bool nvme_ns_ext(NvmeNamespace *ns) > > > } > > > > > > /* calculate the number of LBAs that the namespace can accomodate */ > > > +static inline uint64_t __nvme_nlbas(size_t size, uint8_t lbads, uint16_t ms) > > > +{ > > > + if (ms) { > > > + return size / ((1 << lbads) + ms); > > > + } > > > + > > > + return size >> lbads; > > > +} > > > + > > > static inline uint64_t nvme_ns_nlbas(NvmeNamespace *ns) > > > { > > > - if (nvme_msize(ns)) { > > > - return ns->size / (nvme_lsize(ns) + nvme_msize(ns)); > > > - } > > > - return ns->size >> nvme_ns_lbads(ns); > > > + return __nvme_nlbas(ns->size, nvme_ns_lbads(ns), nvme_msize(ns)); > > > } > > > > Hmm.. I think it looks like __nvme_nlbas does the same with the > > nvme_ns_nlbas, but flexible argument attributes. But I think those > > three attributes are all for ns-specific fields which is not that > > generic so that I don't think we are going to take the helper from much > > more general perspective with __nvme_nlbas. > > > > This patch should be moved two patches forward in the series - it is > used in [12/13] to check the zone geometry before the values are set on > the namespace proper. This is also used in Format NVM to verify the > format before formatting ("commiting" the values on the NvmeNamespace > structure). Checked [12/13] right before. Thanks for pointing that out! Reviewed-by: Minwoo Im <minwoo.im.dev@gmail.com>
diff --git a/hw/block/nvme-ns.h b/hw/block/nvme-ns.h index 07e16880801d..34f9474a1cd1 100644 --- a/hw/block/nvme-ns.h +++ b/hw/block/nvme-ns.h @@ -136,12 +136,18 @@ static inline bool nvme_ns_ext(NvmeNamespace *ns) } /* calculate the number of LBAs that the namespace can accomodate */ +static inline uint64_t __nvme_nlbas(size_t size, uint8_t lbads, uint16_t ms) +{ + if (ms) { + return size / ((1 << lbads) + ms); + } + + return size >> lbads; +} + static inline uint64_t nvme_ns_nlbas(NvmeNamespace *ns) { - if (nvme_msize(ns)) { - return ns->size / (nvme_lsize(ns) + nvme_msize(ns)); - } - return ns->size >> nvme_ns_lbads(ns); + return __nvme_nlbas(ns->size, nvme_ns_lbads(ns), nvme_msize(ns)); } typedef struct NvmeCtrl NvmeCtrl;