Re: [ndctl PATCH v2 5/6] ndctl/cxl: Add extent output to region query

[Alison Schofield]

On Mon, Nov 04, 2024 at 08:10:49PM -0600, Ira Weiny wrote:
> DCD regions have 0 or more extents.  The ability to list those and their
> properties is useful to end users.
> 

Should we describe those new useful properties in the man page...see
below...

> Add extent output to region queries.
> 
> Signed-off-by: Ira Weiny <ira.we...@intel.com>
> ---
>  Documentation/cxl/cxl-list.txt |   4 ++
>  cxl/filter.h                   |   3 +
>  cxl/json.c                     |  47 ++++++++++++++
>  cxl/json.h                     |   3 +
>  cxl/lib/libcxl.c               | 138 
> +++++++++++++++++++++++++++++++++++++++++
>  cxl/lib/libcxl.sym             |   5 ++
>  cxl/lib/private.h              |  11 ++++
>  cxl/libcxl.h                   |  11 ++++
>  cxl/list.c                     |   3 +
>  util/json.h                    |   1 +
>  10 files changed, 226 insertions(+)
> 
> diff --git a/Documentation/cxl/cxl-list.txt b/Documentation/cxl/cxl-list.txt
> index 
> 9a9911e7dd9bba561c6202784017db1bb4b9f4bd..71fd313cfec2509c79f8ad1e0f64857d0d804c13
>  100644
> --- a/Documentation/cxl/cxl-list.txt
> +++ b/Documentation/cxl/cxl-list.txt
> @@ -411,6 +411,10 @@ OPTIONS
>  }
>  ----
>  
> +-N::
> +--extents::
> +     Extend Dynamic Capacity region listings extent information.
> +

a sample perhaps?  or some verbage on  what to expect.


snip

>  
> +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;
> +
> +     dbg(ctx, "Checking extents: %s\n", region->dev_path);

Rather than emit the above which makes me assume success if
no err message follows, how about emitting the success debug
msg when all is done below.

> +
> +     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\n", 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 == ERANGE) {
> +                     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 == ERANGE) {
> +                     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 tag\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->tag) < 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);
> +     }

        Here - dbg the success message

> +     free(dax_region_path);
> +     free(extent_path);
> +     closedir(dir);
> +}
>