Re: [PATCH V3 18/20] cxl/pmem_region: Prep patch to accommodate pmem_region attributes

[Neeraj Kumar <s.neeraj@samsung.com>]

[-- Attachment #1: Type: text/plain, Size: 4385 bytes --]

On 06/10/25 09:06AM, Dave Jiang wrote:
>
>
>On 9/29/25 6:57 AM, Neeraj Kumar wrote:
>> On 24/09/25 11:53AM, Dave Jiang wrote:
>>>
>>>
>>> On 9/17/25 6:41 AM, Neeraj Kumar wrote:
>>>> Created a separate file core/pmem_region.c along with CONFIG_PMEM_REGION
>>>> Moved pmem_region related code from core/region.c to core/pmem_region.c
>>>> For region label update, need to create device attribute, which calls
>>>> nvdimm exported function thus making pmem_region dependent on libnvdimm.
>>>> Because of this dependency of pmem region on libnvdimm, segregated pmem
>>>> region related code from core/region.c
>>>
>>> We can minimize the churn in this patch by introduce the new core/pmem_region.c and related bits in the beginning instead of introduce new functions and then move them over from region.c.
>>
>> Hi Dave,
>>
>> As per LSA 2.1, during region creation we need to intract with nvdimmm
>> driver to write region label into LSA.
>> This dependency of libnvdimm is only for PMEM region, therefore I have
>> created a seperate file core/pmem_region.c and copied pmem related functions
>> present in core/region.c into core/pmem_region.c.
>> Because of this movemement of code we have churn introduced in this patch.
>> Can you please suggest optimized way to handle dependency on libnvdimm
>> with minimum code changes.
>
>Hmm....maybe relegate the introduction of core/pmem_region.c new file and only the moving of the existing bits into the new file to a patch. And then your patch will be rid of the delete/add bits of the old code? Would that work?
>
>DJ

Hi Dave,

As per LSA 2.1, during region creation we need to intract with nvdimmm
driver to write region label into LSA.

This dependency of libnvdimm is only for PMEM region, therefore I have
created a seperate file core/pmem_region.c and copied pmem related functions
present in core/region.c into core/pmem_region.c

I have moved following 7 pmem related functions from core/region.c to core/pmem_region.c
  - cxl_pmem_region_release()
  - cxl_pmem_region_alloc()
  - cxlr_release_nvdimm()
  - cxlr_pmem_unregister()
  - devm_cxl_add_pmem_region()
  - is_cxl_pmem_region()
  - to_cxl_pmem_region()

I have created region_label_update_show/store() and region_label_delete_store() which
internally calls following libnvdimm exported function
  - region_label_update_show/store()
  - region_label_delete_store()

I have added above attributes as following
    {{{
	static struct attribute *cxl_pmem_region_attrs[] = {
         	&dev_attr_region_label_update.attr,
         	&dev_attr_region_label_delete.attr,
         	NULL
	};
	static struct attribute_group cxl_pmem_region_group = {
	        .attrs = cxl_pmem_region_attrs,
	};
	static const struct attribute_group *cxl_pmem_region_attribute_groups[] = {
	        &cxl_base_attribute_group,
	        &cxl_pmem_region_group,      ------> New addition in this patch
	        NULL
	};
	const struct device_type cxl_pmem_region_type = {
	        .name = "cxl_pmem_region",
	        .release = cxl_pmem_region_release,
	        .groups = cxl_pmem_region_attribute_groups,
	};

	static int cxl_pmem_region_alloc(struct cxl_region *cxlr)
	{
	        <snip>
	        dev = &cxlr_pmem->dev;
	        dev->parent = &cxlr->dev;
	        dev->bus = &cxl_bus_type;
	        dev->type = &cxl_pmem_region_type;
		<snip>
	}
    }}}

So I mean to say all above mentioned functions are inter-connected and dependent on libnvdimm
Keeping any of them in core/region.c to avoid churn, throws following linking error
    {{{
	ERROR: modpost: "nd_region_label_delete" [drivers/cxl/core/cxl_core.ko] undefined!
	ERROR: modpost: "nd_region_label_update" [drivers/cxl/core/cxl_core.ko] undefined!
	make[2]: *** [scripts/Makefile.modpost:147: Module.symvers] Error 1
    }}}

Keeping these functions in core/region.c (CONFIG_REGION)) and manually enabling CONFIG_LIBNVDIMM=y
will make it pass.

Even if we can put these functions in core/region.c and forcefully make
libnvdimm (CONFIG_LIBNVDIMM) dependent using Kconfig. But I find it little improper as
this new dependency is not for all type of cxl devices (vmem and pmem) but only for cxl pmem
device region.

Therefore I have seperated it out in core/pmem_region.c under Kconfig control
making libnvdimm forcefully enable if CONFIG_CXL_PMEM_REGION == y

So, I believe this prep patch is required for this LSA 2.1 support.


Regards,
Neeraj

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]