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 <[email protected]> > Signed-off-by: Dan Williams <[email protected]> > --- > 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) { >
