lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20250904132743.qhagt7jonhyjggmn@test-PowerEdge-R740xd>
Date: Thu, 4 Sep 2025 18:57:43 +0530
From: Neeraj Kumar <s.neeraj@...sung.com>
To: Jonathan Cameron <Jonathan.Cameron@...wei.com>
Cc: linux-cxl@...r.kernel.org, nvdimm@...ts.linux.dev,
	linux-kernel@...r.kernel.org, gost.dev@...sung.com,
	a.manzanares@...sung.com, vishak.g@...sung.com, neeraj.kernel@...il.com,
	cpgs@...sung.com
Subject: Re: [PATCH V2 02/20] nvdimm/label: Prep patch to accommodate cxl
 lsa 2.1 support

On 13/08/25 02:23PM, Jonathan Cameron wrote:
>On Wed, 30 Jul 2025 17:41:51 +0530
>Neeraj Kumar <s.neeraj@...sung.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.

Currently we have support of LSA v1.1 and v1.2 in Linux, Where LSA can
only accommodate only one type of labels, which is namespace label.

But as per LSA 2.1, LSA can accommodate both namespace and region
labels.

As v1.1 and v1.2 only namespace label therefore we have "struct
nd_namespace_label"

As this patch-set supports LSA 2.1, where an LSA can have any of
namespace or region label.
It is therefore, introduced "struct nd_lsa_label" in-place of "struct
nd_namespace_label"

I understand that this renaming has created some extra noise in existing
code. May be I will revisit this change and try using region label handling
separately instead of using union.

>
>>
>> No functional change introduced.
>>
>> Signed-off-by: Neeraj Kumar <s.neeraj@...sung.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.

Thanks Jonathan, I will fix it in next patch-set

Regards,
Neeraj


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ