On 9/17/25 6:41 AM, Neeraj Kumar wrote:
> Using these attributes region label is added/deleted into LSA. These
> attributes are called from userspace (ndctl) after region creation.
>
> Signed-off-by: Neeraj Kumar <[email protected]>
> ---
> Documentation/ABI/testing/sysfs-bus-cxl | 22 ++++++
> drivers/cxl/core/pmem_region.c | 91 ++++++++++++++++++++++++-
> drivers/cxl/cxl.h | 1 +
> 3 files changed, 113 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/ABI/testing/sysfs-bus-cxl
> b/Documentation/ABI/testing/sysfs-bus-cxl
> index 6b4e8c7a963d..d6080fcf843a 100644
> --- a/Documentation/ABI/testing/sysfs-bus-cxl
> +++ b/Documentation/ABI/testing/sysfs-bus-cxl
> @@ -615,3 +615,25 @@ Description:
> The count is persistent across power loss and wraps back to 0
> upon overflow. If this file is not present, the device does not
> have the necessary support for dirty tracking.
> +
> +
> +What:
> /sys/bus/cxl/devices/regionZ/pmem_regionZ/region_label_update
> +Date: Sept, 2025
> +KernelVersion: v6.17
> +Contact: [email protected]
> +Description:
> + (RW) Write a boolean 'true' string value to this attribute to
> + update cxl region information into LSA as region label. It
> + uses nvdimm nd_region_label_update() to update cxl region
> + information saved during cxl region creation into LSA. This
> + attribute must be called at last during cxl region creation.
> +
> +
> +What:
> /sys/bus/cxl/devices/regionZ/pmem_regionZ/region_label_delete
> +Date: Sept, 2025
> +KernelVersion: v6.17
> +Contact: [email protected]
> +Description:
> + (WO) When a boolean 'true' is written to this attribute then
> + pmem_region driver deletes cxl region label from LSA using
> + nvdimm nd_region_label_delete()
> diff --git a/drivers/cxl/core/pmem_region.c b/drivers/cxl/core/pmem_region.c
> index 55b80d587403..665b603c907b 100644
> --- a/drivers/cxl/core/pmem_region.c
> +++ b/drivers/cxl/core/pmem_region.c
> @@ -45,9 +45,98 @@ static void cxl_pmem_region_release(struct device *dev)
> kfree(cxlr_pmem);
> }
>
> +static ssize_t region_label_update_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t len)
> +{
> + struct cxl_pmem_region *cxlr_pmem = to_cxl_pmem_region(dev);
> + struct cxl_region *cxlr = cxlr_pmem->cxlr;
> + ssize_t rc;
> + bool update;
> +
> + rc = kstrtobool(buf, &update);
> + if (rc)
> + return rc;
> +
> + ACQUIRE(rwsem_write_kill, rwsem)(&cxl_rwsem.region);
> + rc = ACQUIRE_ERR(rwsem_write_kill, &rwsem);
> + if (rc)
> + return rc;
> +
> + /* Region not yet committed */
> + if (update && cxlr && cxlr->params.state != CXL_CONFIG_COMMIT) {
> + dev_dbg(dev, "region not committed, can't update into LSA\n");
> + return -ENXIO;
> + }
> +
> + if (cxlr && cxlr->cxlr_pmem && cxlr->cxlr_pmem->nd_region) {
> + rc = nd_region_label_update(cxlr->cxlr_pmem->nd_region);
> + if (!rc)
> + cxlr->params.region_label_state = 1;
> + }
> +
> + if (rc)
> + return rc;
Feels like this segment should look like
if (!cxlr || !cxlr->cxlr_pmem || ! cxlr->cxlr_pmem->nd_region)
return 0;
rc = nd_region_label_update(..);
if (rc)
return rc;
cxlr->params.region_label_state = 1;
> +
> + return len;
> +}
> +
> +static ssize_t region_label_update_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + struct cxl_pmem_region *cxlr_pmem = to_cxl_pmem_region(dev);
> + struct cxl_region *cxlr = cxlr_pmem->cxlr;
> + struct cxl_region_params *p = &cxlr->params;
> + ssize_t rc;
> +
> + ACQUIRE(rwsem_read_intr, rwsem)(&cxl_rwsem.region);
> + rc = ACQUIRE_ERR(rwsem_read_intr, &rwsem);
> + if (rc)
> + return rc;
> +
> + return sysfs_emit(buf, "%d\n", p->region_label_state);
> +}
> +static DEVICE_ATTR_RW(region_label_update);
> +
> +static ssize_t region_label_delete_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t len)
> +{
> + struct cxl_pmem_region *cxlr_pmem = to_cxl_pmem_region(dev);
> + struct cxl_region *cxlr = cxlr_pmem->cxlr;
> + ssize_t rc;
> +
> + ACQUIRE(rwsem_write_kill, rwsem)(&cxl_rwsem.region);
> + rc = ACQUIRE_ERR(rwsem_write_kill, &rwsem);
> + if (rc)
> + return rc;
> +
> + if (cxlr && cxlr->cxlr_pmem && cxlr->cxlr_pmem->nd_region) {
> + rc = nd_region_label_delete(cxlr->cxlr_pmem->nd_region);
> + if (rc)
> + return rc;
> + cxlr->params.region_label_state = 0;
> + }
Similar to above. You can exit early and not have to indent.
> +
> + return len;
> +}
> +static DEVICE_ATTR_WO(region_label_delete);
> +
> +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,
> - NULL,
> + &cxl_pmem_region_group,
> + NULL
> };
>
> const struct device_type cxl_pmem_region_type = {
> diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
> index 0d576b359de6..f01f8c942fdf 100644
> --- a/drivers/cxl/cxl.h
> +++ b/drivers/cxl/cxl.h
> @@ -484,6 +484,7 @@ enum cxl_config_state {
> */
> struct cxl_region_params {
> enum cxl_config_state state;
> + int region_label_state;
Maybe a enum?
> uuid_t uuid;
> int interleave_ways;
> int interleave_granularity;
