Re: [ndctl PATCH 09/11] cxl/memdev: Add {reserve,free}-dpa commands

From: "Verma, Vishal L" <vishal.l.verma@intel.com>
To: "Williams, Dan J" <dan.j.williams@intel.com>
Cc: "Schofield, Alison" <alison.schofield@intel.com>,
	"linux-cxl@vger.kernel.org" <linux-cxl@vger.kernel.org>,
	"nvdimm@lists.linux.dev" <nvdimm@lists.linux.dev>
Subject: Re: [ndctl PATCH 09/11] cxl/memdev: Add {reserve,free}-dpa commands
Date: Wed, 13 Jul 2022 08:04:58 +0000	[thread overview]
Message-ID: <cac1d3a7a7e6515b2db0ce7ee73db812686d3407.camel@intel.com> (raw)
In-Reply-To: <165765289554.435671.7351049564074120659.stgit@dwillia2-xfh>

On Tue, 2022-07-12 at 12:08 -0700, Dan Williams wrote:
> Add helper commands for managing allocations of DPA (device physical
> address) capacity on a set of CXL memory devices.
> 
> The main convenience this command affords is automatically picking the next
> decoder to allocate per-memdev.
> 
> For example, to allocate 256MiB from all endpoints that are covered by a
> given root decoder, and collect those resulting endpoint-decoders into an
> array:
> 
>   readarray -t mem < <(cxl list -M -d $decoder | jq -r ".[].memdev")
>   readarray -t endpoint < <(cxl reserve-dpa -t pmem ${mem[*]} -s $((256<<20)) |
>                             jq -r ".[] | .decoder.decoder")
> 
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> ---
>  .clang-format                    |    1 
>  Documentation/cxl/lib/libcxl.txt |    2 

I guess the new commands are mostly for debug only - but should we add
man pages for them?

>  cxl/builtin.h                    |    2 
>  cxl/cxl.c                        |    2 
>  cxl/filter.c                     |    4 -
>  cxl/filter.h                     |    2 
>  cxl/lib/libcxl.c                 |   86 ++++++++++++
>  cxl/lib/libcxl.sym               |    4 +
>  cxl/libcxl.h                     |    9 +
>  cxl/memdev.c                     |  276 ++++++++++++++++++++++++++++++++++++++
>  10 files changed, 385 insertions(+), 3 deletions(-)
> 

<snip>

> 
> +
> +       if (cxl_decoder_get_mode(target) != mode) {
> +               rc = cxl_decoder_set_dpa_size(target, 0);
> +               if (rc) {
> +                       log_err(&ml,
> +                               "%s: %s: failed to clear allocation to set mode\n",
> +                               devname, cxl_decoder_get_devname(target));
> +                       return rc;
> +               }
> +               rc = cxl_decoder_set_mode(target, mode);
> +               if (rc) {
> +                       log_err(&ml, "%s: %s: failed to set %s mode\n", devname,
> +                               cxl_decoder_get_devname(target),
> +                               mode == CXL_DECODER_MODE_PMEM ? "pmem" : "ram");
> +                       return rc;
> +               }
> +       }
> +
> +       rc = cxl_decoder_set_dpa_size(target, size);
> +       if (rc)
> +               log_err(&ml, "%s: %s: failed to set dpa allocation\n", devname,

This patch adds some >80 col lines, which is fine by me - maybe we
should update .clang-format to 100 to make it official?