Re: [PATCH V5 04/17] nvdimm/label: Include region label in slot validation

[Neeraj Kumar]

On 14/01/26 03:20PM, Ira Weiny wrote:
Neeraj Kumar wrote:
Prior to LSA 2.1 Support, label in slot means only namespace
label. But with LSA 2.1 a label can be either namespace or
region label.
Slot validation routine validates label slot by calculating
label checksum. It was only validating namespace label.
This changeset also validates region label if present.

In previous patch to_lsa_label() was introduced along with
to_label(). to_label() returns only namespace label whereas
to_lsa_label() returns union nd_lsa_label*

In this patch We have converted all usage of to_label()

NIT: don't use 'We'

Fixed it in V6


to to_lsa_label()

Reviewed-by: Jonathan Cameron <jonathan.came...@huawei.com>
Reviewed-by: Dave Jiang <dave.ji...@intel.com>
Signed-off-by: Neeraj Kumar <s.nee...@samsung.com>
---
 drivers/nvdimm/label.c | 94 ++++++++++++++++++++++++++++--------------
 1 file changed, 64 insertions(+), 30 deletions(-)

diff --git a/drivers/nvdimm/label.c b/drivers/nvdimm/label.c
index 17e2a1f5a6da..9854cb45fb62 100644
--- a/drivers/nvdimm/label.c
+++ b/drivers/nvdimm/label.c
@@ -312,16 +312,6 @@ static union nd_lsa_label *to_lsa_label(struct 
nvdimm_drvdata *ndd, int slot)
        return (union nd_lsa_label *) label;
 }

-static struct nd_namespace_label *to_label(struct nvdimm_drvdata *ndd, int 
slot)
-{
-       unsigned long label, base;
-
-       base = (unsigned long) nd_label_base(ndd);
-       label = base + sizeof_namespace_label(ndd) * slot;
-
-       return (struct nd_namespace_label *) label;
-}
-
 #define for_each_clear_bit_le(bit, addr, size) \
        for ((bit) = find_next_zero_bit_le((addr), (size), 0);  \
             (bit) < (size);                                    \
@@ -382,7 +372,7 @@ static bool nsl_validate_checksum(struct nvdimm_drvdata 
*ndd,
 {
        u64 sum, sum_save;

-       if (!ndd->cxl && !efi_namespace_label_has(ndd, checksum))
+       if (!efi_namespace_label_has(ndd, checksum))

What does this change have to do with region label validation during slot
validation?

                return true;

        sum_save = nsl_get_checksum(ndd, nd_label);
@@ -397,13 +387,25 @@ static void nsl_calculate_checksum(struct nvdimm_drvdata 
*ndd,
 {
        u64 sum;

-       if (!ndd->cxl && !efi_namespace_label_has(ndd, checksum))
+       if (!efi_namespace_label_has(ndd, checksum))

This and the above seem like cleanups because efi_namespace_label_has()
already checks !ndd->cxl?  Was that the intent?  Perhaps as a separate
cleanup?

Hi Ira,

Actually above is required changes and not the cleanup. 
Earlier condition (!ndd->cxl && !efi_namespace_label_has(ndd, checksum))
was getting true (no further processing) in first case only if its a
region label which we don't want in current case.

And this condition (!efi_namespace_label_has(ndd, checksum)) returns false
(means proceed further) in case if its a region label (!!ndd->cxl).



Regards,
Neeraj