[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <1296674576.21762749302024.JavaMail.epsvc@epcpadp1new>
Date: Fri, 7 Nov 2025 18:19:09 +0530
From: Neeraj Kumar <s.neeraj@...sung.com>
To: Dave Jiang <dave.jiang@...el.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 V3 18/20] cxl/pmem_region: Prep patch to accommodate
pmem_region attributes
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
Powered by blists - more mailing lists