On 23/06/25 11:53AM, Jonathan Cameron wrote:
On Tue, 17 Jun 2025 18:09:26 +0530
Neeraj Kumar <s.nee...@samsung.com> wrote:
In order to accommodate cxl lsa 2.1 format region label, renamed
nd_namespace_label to nd_lsa_label.
I would add some more information on why. I've no idea from this
description whether the issue is a naming clash or a need for a broader
name or something entirely different.
Most readers aren't going to be particular familiar with the lsa spec
version changes, so help them out with a little more detail.
The comment you have with the union is probably the right info
to duplicate here.
Thanks Jonathan, I will elaborate commit message with more information.
No functional change introduced.
Signed-off-by: Neeraj Kumar <s.nee...@samsung.com>
This is quite a lot of churn which, just looking at this patch, could
be avoided by just setting local variables to point at a particular
member of the union rather than the containing struct.
You already do this in a few places like nd_label_reserve_dpa().
Perhaps it will be come clearer why that doesn't make sense as
I ready later patches.
Sure, I will try to minimize wherever its possible.
diff --git a/drivers/nvdimm/label.h b/drivers/nvdimm/label.h
index 0650fb4b9821..4883b3a1320f 100644
--- a/drivers/nvdimm/label.h
+++ b/drivers/nvdimm/label.h
@@ -183,6 +183,16 @@ struct nd_namespace_label {
};
};
+/*
+ * LSA 2.1 format introduces region label, which can also reside
+ * into LSA along with only namespace label as per v1.1 and v1.2
+ */
+struct nd_lsa_label {
+ union {
+ struct nd_namespace_label ns_label;
+ };
+};
diff --git a/drivers/nvdimm/namespace_devs.c b/drivers/nvdimm/namespace_devs.c
index 55cfbf1e0a95..f180f0068c15 100644
--- a/drivers/nvdimm/namespace_devs.c
+++ b/drivers/nvdimm/namespace_devs.c
@@ -1595,7 +1596,8 @@ static bool has_uuid_at_pos(struct nd_region *nd_region,
const uuid_t *uuid,
return false;
}
found_uuid = true;
- if (!nsl_validate_nlabel(nd_region, ndd, nd_label))
+ if (!nsl_validate_nlabel(nd_region,
+ ndd, &nd_label->ns_label))
Strange wrap. I'd move ndd up a line.
Sure, I will fix it in V1
continue;
if (position != pos)
continue;
@@ -1615,7 +1617,7 @@ static int select_pmem_id(struct nd_region *nd_region,
const uuid_t *pmem_id)
for (i = 0; i < nd_region->ndr_mappings; i++) {
struct nd_mapping *nd_mapping = &nd_region->mapping[i];
struct nvdimm_drvdata *ndd = to_ndd(nd_mapping);
- struct nd_namespace_label *nd_label = NULL;
+ struct nd_lsa_label *nd_label = NULL;
u64 hw_start, hw_end, pmem_start, pmem_end;
struct nd_label_ent *label_ent;
@@ -1739,14 +1741,14 @@ static struct device *create_namespace_pmem(struct
nd_region *nd_region,
* that position at labels[0], and NULL at labels[1]. In the process,
* check that the namespace aligns with interleave-set.
*/
- nsl_get_uuid(ndd, nd_label, &uuid);
+ nsl_get_uuid(ndd, &nd_label->ns_label, &uuid);
rc = select_pmem_id(nd_region, &uuid);
if (rc)
goto err;
/* Calculate total size and populate namespace properties from label0 */
for (i = 0; i < nd_region->ndr_mappings; i++) {
- struct nd_namespace_label *label0;
+ struct nd_lsa_label *label0;
If you are only ever going to use one part of the union in a given
bit of code, why not use a struct of that type so you only need to change
the point where it is assigned rather that lots of places.
So keep this as struct nd_namespace_label *label0;
If that makes sense, check for other places where doing that will reduce the
churn
and complexity of the code.
Sure, Let me revisit these. Thanks
struct nvdimm_drvdata *ndd;
nd_mapping = &nd_region->mapping[i];
@@ -1760,17 +1762,17 @@ static struct device *create_namespace_pmem(struct
nd_region *nd_region,
}
ndd = to_ndd(nd_mapping);
- size += nsl_get_rawsize(ndd, label0);
- if (nsl_get_position(ndd, label0) != 0)
+ size += nsl_get_rawsize(ndd, &label0->ns_label);
+ if (nsl_get_position(ndd, &label0->ns_label) != 0)
continue;
WARN_ON(nspm->alt_name || nspm->uuid);
- nspm->alt_name = kmemdup(nsl_ref_name(ndd, label0),
+ nspm->alt_name = kmemdup(nsl_ref_name(ndd, &label0->ns_label),
NSLABEL_NAME_LEN, GFP_KERNEL);
- nsl_get_uuid(ndd, label0, &uuid);
+ nsl_get_uuid(ndd, &label0->ns_label, &uuid);
nspm->uuid = kmemdup(&uuid, sizeof(uuid_t), GFP_KERNEL);
- nspm->lbasize = nsl_get_lbasize(ndd, label0);
+ nspm->lbasize = nsl_get_lbasize(ndd, &label0->ns_label);
nspm->nsio.common.claim_class =
- nsl_get_claim_class(ndd, label0);
+ nsl_get_claim_class(ndd, &label0->ns_label);
}
