| Message ID | 167053497261.582963.1274754281124548404.stgit@dwillia2-xfh.jf.intel.com |
|---|---|
| State | Accepted |
| Commit | 5cefc0f59bea9130ef466b10d5a377bae152d206 |
| Headers | show |
| Series | cxl-cli test and usability updates | expand |
On Thu, Dec 08, 2022 at 01:29:32PM -0800, Dan Williams wrote: > Now that parse_create_region() uses cxl_filter_walk() to gather memdevs > use that as the target list in case no target list is provided. In other > words the result of "cxl list -M -d $decoder" returns all the potential > memdevs that can comprise a region under $decoder, so just go ahead and > try to use that as the target list by default. > > Note though that the order of devices returned by cxl_filter_walk() may > not be a suitable region creation order. So this porcelain helps for > simple topologies, but needs a follow-on patch to sort the memdevs by > valid region order, and/or discover cases where deviceA or deviceB can > be in the region, but not both. > > Outside of those cases: > > cxl create-region -d decoderX.Y > > ...is sufficient to create a region. > Reviewed-by: Alison Schofield <alison.schofield@intel.com> > Signed-off-by: Dan Williams <dan.j.williams@intel.com> > --- > Documentation/cxl/cxl-create-region.txt | 10 ++++++---- > cxl/region.c | 16 ++++++++-------- > 2 files changed, 14 insertions(+), 12 deletions(-) > > diff --git a/Documentation/cxl/cxl-create-region.txt b/Documentation/cxl/cxl-create-region.txt > index e0e6818cfdd1..286779eff9ed 100644 > --- a/Documentation/cxl/cxl-create-region.txt > +++ b/Documentation/cxl/cxl-create-region.txt > @@ -53,16 +53,18 @@ OPTIONS > ------- > <target(s)>:: > The CXL targets that should be used to form the region. The number of > -'target' arguments must match the '--ways' option (if provided). The > -targets are memdev names such as 'mem0', 'mem1' etc. > +'target' arguments must match the '--ways' option (if provided). > > include::bus-option.txt[] > > -m:: > --memdevs:: > Indicate that the non-option arguments for 'target(s)' refer to memdev > - names. Currently this is the only option supported, and must be > - specified. > + device names. If this option is omitted and no targets are specified > + then create-region uses the equivalent of 'cxl list -M -d $decoder' > + internally as the target list. Note that depending on the topology, for > + example with switches, the automatic target list ordering may not be > + valid and manual specification of the target list is required. > > -s:: > --size=:: > diff --git a/cxl/region.c b/cxl/region.c > index 286c358f1a34..15cac64a158c 100644 > --- a/cxl/region.c > +++ b/cxl/region.c > @@ -269,10 +269,13 @@ static int parse_create_options(struct cxl_ctx *ctx, int count, > } > > /* > - * For all practical purposes, -m is the default target type, but > - * hold off on actively making that decision until a second target > - * option is available. > + * For all practical purposes, -m is the default target type, but hold > + * off on actively making that decision until a second target option is > + * available. Unless there are no arguments then just assume memdevs. > */ > + if (!count) > + param.memdevs = true; > + > if (!param.memdevs) { > log_err(&rl, > "must specify option for target object types (-m)\n"); > @@ -314,11 +317,8 @@ static int parse_create_options(struct cxl_ctx *ctx, int count, > p->ways = count; > if (!validate_ways(p, count)) > return -EINVAL; > - } else { > - log_err(&rl, > - "couldn't determine interleave ways from options or arguments\n"); > - return -EINVAL; > - } > + } else > + p->ways = p->num_memdevs; > > if (param.granularity < INT_MAX) { > if (param.granularity <= 0) { >
diff --git a/Documentation/cxl/cxl-create-region.txt b/Documentation/cxl/cxl-create-region.txt index e0e6818cfdd1..286779eff9ed 100644 --- a/Documentation/cxl/cxl-create-region.txt +++ b/Documentation/cxl/cxl-create-region.txt @@ -53,16 +53,18 @@ OPTIONS ------- <target(s)>:: The CXL targets that should be used to form the region. The number of -'target' arguments must match the '--ways' option (if provided). The -targets are memdev names such as 'mem0', 'mem1' etc. +'target' arguments must match the '--ways' option (if provided). include::bus-option.txt[] -m:: --memdevs:: Indicate that the non-option arguments for 'target(s)' refer to memdev - names. Currently this is the only option supported, and must be - specified. + device names. If this option is omitted and no targets are specified + then create-region uses the equivalent of 'cxl list -M -d $decoder' + internally as the target list. Note that depending on the topology, for + example with switches, the automatic target list ordering may not be + valid and manual specification of the target list is required. -s:: --size=:: diff --git a/cxl/region.c b/cxl/region.c index 286c358f1a34..15cac64a158c 100644 --- a/cxl/region.c +++ b/cxl/region.c @@ -269,10 +269,13 @@ static int parse_create_options(struct cxl_ctx *ctx, int count, } /* - * For all practical purposes, -m is the default target type, but - * hold off on actively making that decision until a second target - * option is available. + * For all practical purposes, -m is the default target type, but hold + * off on actively making that decision until a second target option is + * available. Unless there are no arguments then just assume memdevs. */ + if (!count) + param.memdevs = true; + if (!param.memdevs) { log_err(&rl, "must specify option for target object types (-m)\n"); @@ -314,11 +317,8 @@ static int parse_create_options(struct cxl_ctx *ctx, int count, p->ways = count; if (!validate_ways(p, count)) return -EINVAL; - } else { - log_err(&rl, - "couldn't determine interleave ways from options or arguments\n"); - return -EINVAL; - } + } else + p->ways = p->num_memdevs; if (param.granularity < INT_MAX) { if (param.granularity <= 0) {
Now that parse_create_region() uses cxl_filter_walk() to gather memdevs use that as the target list in case no target list is provided. In other words the result of "cxl list -M -d $decoder" returns all the potential memdevs that can comprise a region under $decoder, so just go ahead and try to use that as the target list by default. Note though that the order of devices returned by cxl_filter_walk() may not be a suitable region creation order. So this porcelain helps for simple topologies, but needs a follow-on patch to sort the memdevs by valid region order, and/or discover cases where deviceA or deviceB can be in the region, but not both. Outside of those cases: cxl create-region -d decoderX.Y ...is sufficient to create a region. Signed-off-by: Dan Williams <dan.j.williams@intel.com> --- Documentation/cxl/cxl-create-region.txt | 10 ++++++---- cxl/region.c | 16 ++++++++-------- 2 files changed, 14 insertions(+), 12 deletions(-)