Re: [PATCH v6 4/7] libcxl: Add extent functionality to DC regions

[Anisa Su <anisa.su887@gmail.com>]

On Mon, Jun 08, 2026 at 05:05:58PM -0700, Dave Jiang wrote:
> 
> 
> On 5/23/26 2:50 AM, Anisa Su wrote:
> > From: Ira Weiny <ira.weiny@intel.com>
> > 
> > DCD regions have 0 or more extents.  The ability to list those and their
> > properties is useful to end users.
> > 
> > Add extent scanning and reporting functionality to libcxl.
> > 
> > Signed-off-by: Ira Weiny <ira.weiny@intel.com>
> 
> Missing Anisa sign-off
> 
Added
> 
> > 
> > ---
> > Changes:
> > [alison: s/tag/uuid/ for extents]
> > ---
> >  Documentation/cxl/lib/libcxl.txt |  27 ++++++
> >  cxl/lib/libcxl.c                 | 138 +++++++++++++++++++++++++++++++
> >  cxl/lib/libcxl.sym               |   5 ++
> >  cxl/lib/private.h                |  11 +++
> >  cxl/libcxl.h                     |  11 +++
> >  5 files changed, 192 insertions(+)
> > 
> > diff --git a/Documentation/cxl/lib/libcxl.txt b/Documentation/cxl/lib/libcxl.txt
> > index 9921ac1..0ad294c 100644
> > --- a/Documentation/cxl/lib/libcxl.txt
> > +++ b/Documentation/cxl/lib/libcxl.txt
> > @@ -635,6 +635,33 @@ where its properties can be interrogated by daxctl. The helper
> >  cxl_region_get_daxctl_region() returns an 'struct daxctl_region *' that
> >  can be used with other libdaxctl APIs.
> >  
> > +EXTENTS
> > +-------
> > +
> > +=== EXTENT: Enumeration
> > +----
> > +struct cxl_region_extent;
> > +struct cxl_region_extent *cxl_extent_get_first(struct cxl_region *region);
> > +struct cxl_region_extent *cxl_extent_get_next(struct cxl_region_extent *extent);
> > +#define cxl_extent_foreach(region, extent) \
> > +        for (extent = cxl_extent_get_first(region); \
> > +             extent != NULL; \
> > +             extent = cxl_extent_get_next(extent))
> > +
> > +----
> > +
> > +=== EXTENT: Attributes
> > +----
> > +unsigned long long cxl_extent_get_offset(struct cxl_region_extent *extent);
> > +unsigned long long cxl_extent_get_length(struct cxl_region_extent *extent);
> > +void cxl_extent_get_uuid(struct cxl_region_extent *extent, uuid_t uuid);
> > +----
> > +
> > +Extents represent available memory within a dynamic capacity region.  Extent
> > +objects are available for informational purposes to aid in allocation of
> > +memory.
> > +
> > +
> >  include::../../copyright.txt[]
> >  
> >  SEE ALSO
> > diff --git a/cxl/lib/libcxl.c b/cxl/lib/libcxl.c
> > index be0bc03..c096666 100644
> > --- a/cxl/lib/libcxl.c
> > +++ b/cxl/lib/libcxl.c
> > @@ -635,6 +635,7 @@ static void *add_cxl_region(void *parent, int id, const char *cxlregion_base)
> >  	region->ctx = ctx;
> >  	region->decoder = decoder;
> >  	list_head_init(&region->mappings);
> > +	list_head_init(&region->extents);
> >  
> >  	region->dev_path = strdup(cxlregion_base);
> >  	if (!region->dev_path)
> > @@ -1257,6 +1258,143 @@ cxl_mapping_get_next(struct cxl_memdev_mapping *mapping)
> >  	return list_next(&region->mappings, mapping, list);
> >  }
> >  
> > +static void cxl_extents_init(struct cxl_region *region)
> > +{
> > +	const char *devname = cxl_region_get_devname(region);
> > +	struct cxl_ctx *ctx = cxl_region_get_ctx(region);
> > +	char *extent_path, *dax_region_path;
> > +	struct dirent *de;
> > +	DIR *dir = NULL;
> > +
> > +	if (region->extents_init)
> > +		return;
> > +	region->extents_init = 1;
> > +
> > +	dax_region_path = calloc(1, strlen(region->dev_path) + 64);
> > +	if (!dax_region_path) {
> > +		err(ctx, "%s: allocation failure\n", devname);
> > +		return;
> > +	}
> > +
> > +	extent_path = calloc(1, strlen(region->dev_path) + 100);
> > +	if (!extent_path) {
> > +		err(ctx, "%s: allocation failure\n", devname);
> > +		free(dax_region_path);
> > +		return;
> > +	}
> > +
> > +	sprintf(dax_region_path, "%s/dax_region%d",
> > +		region->dev_path, region->id);
> > +	dir = opendir(dax_region_path);
> > +	if (!dir) {
> > +		err(ctx, "no extents found (%s): %s\n",
> > +			strerror(errno), dax_region_path);
> > +		free(extent_path);
> > +		free(dax_region_path);
> > +		return;
> > +	}
> > +
> > +	while ((de = readdir(dir)) != NULL) {
> > +		struct cxl_region_extent *extent;
> > +		char buf[SYSFS_ATTR_SIZE];
> > +		u64 offset, length;
> > +		int id, region_id;
> > +
> > +		if (sscanf(de->d_name, "extent%d.%d", &region_id, &id) != 2)
> > +			continue;
> > +
> > +		sprintf(extent_path, "%s/extent%d.%d/offset",
> > +			dax_region_path, region_id, id);
> > +		if (sysfs_read_attr(ctx, extent_path, buf) < 0) {
> > +			err(ctx, "%s: failed to read extent%d.%d/offset\n",
> > +				devname, region_id, id);
> > +			continue;
> > +		}
> > +
> > +		offset = strtoull(buf, NULL, 0);
> > +		if (offset == ULLONG_MAX) {
> > +			err(ctx, "%s extent%d.%d: failed to read offset\n",
> > +				devname, region_id, id);
> > +			continue;
> > +		}
> > +
> > +		sprintf(extent_path, "%s/extent%d.%d/length",
> > +			dax_region_path, region_id, id);
> > +		if (sysfs_read_attr(ctx, extent_path, buf) < 0) {
> > +			err(ctx, "%s: failed to read extent%d.%d/length\n",
> > +				devname, region_id, id);
> > +			continue;
> > +		}
> > +
> > +		length = strtoull(buf, NULL, 0);
> > +		if (length == ULLONG_MAX) {
> > +			err(ctx, "%s extent%d.%d: failed to read length\n",
> > +				devname, region_id, id);
> > +			continue;
> > +		}
> > +
> > +		sprintf(extent_path, "%s/extent%d.%d/tag",
> > +			dax_region_path, region_id, id);
> > +		buf[0] = '\0';
> > +		if (sysfs_read_attr(ctx, extent_path, buf) != 0)
> > +			dbg(ctx, "%s extent%d.%d: failed to read uuid\n",
> > +				devname, region_id, id);
> > +
> > +		extent = calloc(1, sizeof(*extent));
> > +		if (!extent) {
> > +			err(ctx, "%s extent%d.%d: allocation failure\n",
> > +				devname, region_id, id);
> > +			continue;
> > +		}
> > +		if (strlen(buf) && uuid_parse(buf, extent->uuid) < 0)
> > +			err(ctx, "%s:%s\n", extent_path, buf);
> > +		extent->region = region;
> > +		extent->offset = offset;
> > +		extent->length = length;
> > +
> > +		list_node_init(&extent->list);
> > +		list_add(&region->extents, &extent->list);
> 
> free_region() never frees any of the extents allocated and added here and thus leak the memory when region is freed.
> 
added loop to free extents in free_region():

list_for_each_safe(&region->extents, extent, _e, list) {
	list_del_from(&region->extents, &extent->list);
	free(extent);
}
> 
> > +		dbg(ctx, "%s added extent%d.%d\n", devname, region_id, id);
> > +	}
> > +	free(dax_region_path);
> > +	free(extent_path);
> > +	closedir(dir);
> > +}
> > +
> > +CXL_EXPORT struct cxl_region_extent *
> > +cxl_extent_get_first(struct cxl_region *region)
> > +{
> > +	cxl_extents_init(region);
> > +
> > +	return list_top(&region->extents, struct cxl_region_extent, list);
> > +}
> > +
> > +CXL_EXPORT struct cxl_region_extent *
> > +cxl_extent_get_next(struct cxl_region_extent *extent)
> > +{
> > +	struct cxl_region *region = extent->region;
> > +
> > +	return list_next(&region->extents, extent, list);
> > +}
> > +
> > +CXL_EXPORT unsigned long long
> > +cxl_extent_get_offset(struct cxl_region_extent *extent)
> > +{
> > +	return extent->offset;
> > +}
> > +
> > +CXL_EXPORT unsigned long long
> > +cxl_extent_get_length(struct cxl_region_extent *extent)
> > +{
> > +	return extent->length;
> > +}
> > +
> > +CXL_EXPORT void
> > +cxl_extent_get_uuid(struct cxl_region_extent *extent, uuid_t uuid)
> > +{
> > +	memcpy(uuid, extent->uuid, sizeof(uuid_t));
> > +}
> > +
> >  CXL_EXPORT struct cxl_decoder *
> >  cxl_mapping_get_decoder(struct cxl_memdev_mapping *mapping)
> >  {
> > diff --git a/cxl/lib/libcxl.sym b/cxl/lib/libcxl.sym
> > index 258bdd3..dcfe242 100644
> > --- a/cxl/lib/libcxl.sym
> > +++ b/cxl/lib/libcxl.sym
> > @@ -298,6 +298,11 @@ global:
> >  	cxl_memdev_get_dynamic_ram_a_qos_class;
> >  	cxl_decoder_is_dynamic_ram_a_capable;
> >  	cxl_decoder_create_dynamic_ram_a_region;
> > +	cxl_extent_get_first;
> > +	cxl_extent_get_next;
> > +	cxl_extent_get_offset;
> > +	cxl_extent_get_length;
> > +	cxl_extent_get_uuid;
> >  } LIBECXL_8;

Now in LIBCXL_13

> >  
> >  LIBCXL_10 {
> > diff --git a/cxl/lib/private.h b/cxl/lib/private.h
> > index 37b7b06..c5f3bed 100644
> > --- a/cxl/lib/private.h
> > +++ b/cxl/lib/private.h
> > @@ -183,6 +183,7 @@ struct cxl_region {
> >  	struct cxl_decoder *decoder;
> >  	struct list_node list;
> >  	int mappings_init;
> > +	int extents_init;
> >  	struct cxl_ctx *ctx;
> >  	void *dev_buf;
> >  	size_t buf_len;
> > @@ -200,6 +201,7 @@ struct cxl_region {
> >  	struct daxctl_region *dax_region;
> >  	struct kmod_module *module;
> >  	struct list_head mappings;
> > +	struct list_head extents;
> >  };
> >  
> >  struct cxl_memdev_mapping {
> > @@ -209,6 +211,15 @@ struct cxl_memdev_mapping {
> >  	struct list_node list;
> >  };
> >  
> > +#define CXL_REGION_EXTENT_TAG 0x10
> 
> defined but never used
> 
thanks for the catch! deleted

> DJ
> 
Thanks,
Anisa
> > +struct cxl_region_extent {
> > +	struct cxl_region *region;
> > +	u64 offset;
> > +	u64 length;
> > +	uuid_t uuid;
> > +	struct list_node list;
> > +};
> > +
> >  enum cxl_cmd_query_status {
> >  	CXL_CMD_QUERY_NOT_RUN = 0,
> >  	CXL_CMD_QUERY_OK,
> > diff --git a/cxl/libcxl.h b/cxl/libcxl.h
> > index fd41122..a60509f 100644
> > --- a/cxl/libcxl.h
> > +++ b/cxl/libcxl.h
> > @@ -394,6 +394,17 @@ unsigned int cxl_mapping_get_position(struct cxl_memdev_mapping *mapping);
> >               mapping != NULL; \
> >               mapping = cxl_mapping_get_next(mapping))
> >  
> > +struct cxl_region_extent;
> > +struct cxl_region_extent *cxl_extent_get_first(struct cxl_region *region);
> > +struct cxl_region_extent *cxl_extent_get_next(struct cxl_region_extent *extent);
> > +#define cxl_extent_foreach(region, extent) \
> > +        for (extent = cxl_extent_get_first(region); \
> > +             extent != NULL; \
> > +             extent = cxl_extent_get_next(extent))
> > +unsigned long long cxl_extent_get_offset(struct cxl_region_extent *extent);
> > +unsigned long long cxl_extent_get_length(struct cxl_region_extent *extent);
> > +void cxl_extent_get_uuid(struct cxl_region_extent *extent, uuid_t uuid);
> > +
> >  struct cxl_cmd;
> >  const char *cxl_cmd_get_devname(struct cxl_cmd *cmd);
> >  struct cxl_cmd *cxl_cmd_new_raw(struct cxl_memdev *memdev, int opcode);
>