Re: [ndctl PATCH 4/6] cxl/region: Add creation of Dynamic capacity regions

[Ira Weiny <ira.weiny@intel.com>]

Fan Ni wrote:
> On Wed, Oct 30, 2024 at 04:54:47PM -0500, ira.weiny@intel.com wrote:
> > From: Navneet Singh <navneet.singh@intel.com>
> > 
> > CXL Dynamic Capacity Devices (DCDs) optionally support dynamic capacity
> > with up to eight partitions (Regions) (dc0-dc7).  CXL regions can now be
> > spare and defined as dynamic capacity (dc).
> > 
> > Add support for DCD devices.  Query for DCD capabilities.  Add the
> > ability to add DC partitions to a CXL DC region.
> > 
> > Signed-off-by: Navneet Singh <navneet.singh@intel.com>
> > Co-authored-by: Sushant1 Kumar <sushant1.kumar@intel.com>
> > Signed-off-by: Sushant1 Kumar <sushant1.kumar@intel.com>
> > Co-authored-by: Ira Weiny <ira.weiny@intel.com>
> > Signed-off-by: Ira Weiny <ira.weiny@intel.com>
> > 
> > ---
> > Changes:
> > [iweiny: adjust to new sysfs interface.]
> > [iweiny: Rebase to latest pending]
> > [iweiny: Adjust DCD region code to new upstream sysfs entries]
> > [iweiny: Ensure backwards compatibility for non-DC kernels]
> > [iweiny: fixup help message to show DC type]
> > [iweiny: don't double declare decoder mode is dc]
> > [iweiny: simplify __reserve_dpa() with decoder mode to index]
> > [iweiny: Adjust to the new region mode]
> > ---
> >  cxl/json.c         | 26 +++++++++++++++
> >  cxl/lib/libcxl.c   | 95 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
> >  cxl/lib/libcxl.sym |  3 ++
> >  cxl/lib/private.h  |  6 +++-
> >  cxl/libcxl.h       | 55 +++++++++++++++++++++++++++++--
> >  cxl/memdev.c       |  7 +++-
> >  cxl/region.c       | 49 ++++++++++++++++++++++++++--
> >  7 files changed, 234 insertions(+), 7 deletions(-)
> > 
> > diff --git a/cxl/json.c b/cxl/json.c
> > index dcd3cc28393faf7e8adf299a857531ecdeaac50a..4276b9678d7e03eaf2aec581a08450f2a0b857f2 100644
> > --- a/cxl/json.c
> > +++ b/cxl/json.c
> > @@ -754,10 +754,12 @@ err_free:
> >  	return jpoison;
> >  }
> >  
> > +#define DC_SIZE_NAME_LEN 64
> >  struct json_object *util_cxl_memdev_to_json(struct cxl_memdev *memdev,
> >  		unsigned long flags)
> >  {
> >  	const char *devname = cxl_memdev_get_devname(memdev);
> > +	char size_name[DC_SIZE_NAME_LEN];
> >  	struct json_object *jdev, *jobj;
> >  	unsigned long long serial, size;
> >  	const char *fw_version;
> > @@ -800,6 +802,17 @@ struct json_object *util_cxl_memdev_to_json(struct cxl_memdev *memdev,
> >  		}
> >  	}
> >  
> > +	for (int index; index < MAX_NUM_DC_REGIONS; index++) {
> 
> index is not initialized.
> Should be index = 0;

Thanks for the review!

Good catch.  I'll fix up.

> 
> Also, the "cxl list" looks like below, the size of each DC region is
> attached to each DCD device, that seems not quite aligned with what
> "_size" means for pmem/ram. Should we have a separate option for "cxl
> list" to show DC region info??

I'm not sure I follow.  The pmem/ram sizes show the size of the partitions on
the memdev.  This is the same for each DC partition.

Are you looking for the available size after some extents are available?

In that case I think you are looking for the dax information details which
comes after creating a region and using the -X option.

17:02:42 > ./build/cxl/cxl list -r 8 -X
[
  {
    "region":"region8",
    "resource":1031597457408,
    "size":536870912,
    "type":"dc",
    "interleave_ways":1,
    "interleave_granularity":256,
    "decode_state":"commit",
    "daxregion":{
      "id":8,
      "size":536870912,
      "available_size":134217728,
      "align":2097152
    }
  }
]


This shows an available size which can further be dissected with the new
--extents (-N) option added in patch 5/6.

17:04:32 > ./build/cxl/cxl list -r 8 -X -N
[
  {
    "region":"region8",
    "resource":1031597457408,
    "size":536870912,
    "type":"dc",
    "interleave_ways":1,
    "interleave_granularity":256,
    "decode_state":"commit",
    "daxregion":{
      "id":8,
      "size":536870912,
      "available_size":134217728,
      "align":2097152
    },
    "extents":[
      {
        "offset":268435456,
        "length":67108864,
        "tag":"00000000-0000-0000-0000-000000000000"
      },
      {
        "offset":134217728,
        "length":67108864,
        "tag":"00000000-0000-0000-0000-000000000000"
      }
    ]
  }
]


Does this give you the information you are looking for?  Or am I missing
something in your question?

Ira

> 
> Fan
> 
> ----------
>   {
>         "memdev":"mem1",
>         "dc0_size":"2.00 GiB (2.15 GB)",
>         "dc1_size":"2.00 GiB (2.15 GB)",
>         "serial":"0xf02",
>         "host":"0000:11:00.0",
>         "firmware_version":"BWFW VERSION 00"
>       },
>       {
>         "memdev":"mem3",
>         "dc0_size":"2.00 GiB (2.15 GB)",
>         "dc1_size":"2.00 GiB (2.15 GB)",
>         "serial":"0xf03",
>         "host":"0000:12:00.0",
>         "firmware_version":"BWFW VERSION 00"
>       },
> ----------
> 

[snip]