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
