On Wed, 30 Jul 2025 17:41:51 +0530
Neeraj Kumar <s.nee...@samsung.com> wrote:
> 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
>
> As both namespace and region labels are of same size of 256 bytes.
> Thus renamed "struct nd_namespace_label" to "struct nd_lsa_label",
> where both namespace label and region label can stay as union.
Maybe add something on why it makes sense to use a union rather than
new handling.
>
> No functional change introduced.
>
> Signed-off-by: Neeraj Kumar <s.nee...@samsung.com>
> ---
A few minor comments inline.
> diff --git a/drivers/nvdimm/namespace_devs.c b/drivers/nvdimm/namespace_devs.c
> index 55cfbf1e0a95..bdf1ed6f23d8 100644
> --- a/drivers/nvdimm/namespace_devs.c
> +++ b/drivers/nvdimm/namespace_devs.c
> @@ -1615,17 +1619,21 @@ 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_lsa_label *lsa_label = NULL;
Why not pull this into the scope below.
> struct nd_namespace_label *nd_label = NULL;
> u64 hw_start, hw_end, pmem_start, pmem_end;
> struct nd_label_ent *label_ent;
>
> lockdep_assert_held(&nd_mapping->lock);
> list_for_each_entry(label_ent, &nd_mapping->labels, list) {
e.g.
struct nd_lsa_label *lsa_label = label_ent->label;
then no need to set it to NULL later.
> - nd_label = label_ent->label;
> - if (!nd_label)
> + lsa_label = label_ent->label;
> + if (!lsa_label)
> continue;
> +
> + nd_label = &lsa_label->ns_label;
> if (nsl_uuid_equal(ndd, nd_label, pmem_id))
> break;
> + lsa_label = NULL;
> nd_label = NULL;
> }
>
> @@ -1746,19 +1754,21 @@ static struct device *create_namespace_pmem(struct
> nd_region *nd_region,
>
> /* Calculate total size and populate namespace properties from label0 */
> for (i = 0; i < nd_region->ndr_mappings; i++) {
> + struct nd_lsa_label *lsa_label;
> struct nd_namespace_label *label0;
> struct nvdimm_drvdata *ndd;
>
> nd_mapping = &nd_region->mapping[i];
> label_ent = list_first_entry_or_null(&nd_mapping->labels,
> typeof(*label_ent), list);
> - label0 = label_ent ? label_ent->label : NULL;
> + lsa_label = label_ent ? label_ent->label : NULL;
>
> - if (!label0) {
> + if (!lsa_label) {
> WARN_ON(1);
> continue;
> }
>
> + label0 = &lsa_label->ns_label;
> ndd = to_ndd(nd_mapping);
> size += nsl_get_rawsize(ndd, label0);
> if (nsl_get_position(ndd, label0) != 0)
>
