| Message ID | 169881493839.1770063.6493881963554580869.stgit@bgt-140510-bm03.eng.stellus.in |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] cxl/region: Refactor logic around check_last_peer() | expand |
On Wed, Nov 01, 2023 at 05:02:18AM +0000, Jim Harris wrote: > 'distance' is equivalent to the product of the interleave_ways of the > "ancestor" decoders of the port's decoder we are setting up in > cxl_port_setup_targets(). > > So use the term "ancestral_ways" instead of "distance" to better > clarify the meaning of this value. Also move all logic around this > value directly into check_last_peer() so that all necessary calculations > are in one place. It also allows eliminating a parameter to that > function. Reviewed-by: Alison Schofield <alison.schofield@intel.com> A couple of housekeeping notes: - Use get_maintainers script to find the To: List - It's customary to send revised patches as a new thread, not as a reply to previous version. Thanks, Alison > > Signed-off-by: Jim Harris <jim.harris@samsung.com> > --- > drivers/cxl/core/region.c | 33 +++++++++++++++++++-------------- > 1 file changed, 19 insertions(+), 14 deletions(-) > > diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c > index a1eac592c66a..2edfc02bb766 100644 > --- a/drivers/cxl/core/region.c > +++ b/drivers/cxl/core/region.c > @@ -1037,8 +1037,7 @@ static void cxl_port_detach_region(struct cxl_port *port, > } > > static int check_last_peer(struct cxl_endpoint_decoder *cxled, > - struct cxl_ep *ep, struct cxl_region_ref *cxl_rr, > - int distance) > + struct cxl_ep *ep, struct cxl_region_ref *cxl_rr) > { > struct cxl_memdev *cxlmd = cxled_to_memdev(cxled); > struct cxl_region *cxlr = cxl_rr->region; > @@ -1048,19 +1047,30 @@ static int check_last_peer(struct cxl_endpoint_decoder *cxled, > struct cxl_memdev *cxlmd_peer; > struct cxl_ep *ep_peer; > int pos = cxled->pos; > + int ancestral_ways; > + > + if (cxl_rr->nr_targets == 1) { > + /* > + * Passthrough decoders impose no positioning requirements > + * between peers. > + */ > + return 0; > + } > + > + ancestral_ways = p->nr_targets / cxl_rr->nr_targets; > > /* > * If this position wants to share a dport with the last endpoint mapped > - * then that endpoint, at index 'position - distance', must also be > + * then that endpoint, at index 'position - ancestral_ways', must also be > * mapped by this dport. > */ > - if (pos < distance) { > + if (pos < ancestral_ways) { > dev_dbg(&cxlr->dev, "%s:%s: cannot host %s:%s at %d\n", > dev_name(port->uport_dev), dev_name(&port->dev), > dev_name(&cxlmd->dev), dev_name(&cxled->cxld.dev), pos); > return -ENXIO; > } > - cxled_peer = p->targets[pos - distance]; > + cxled_peer = p->targets[pos - ancestral_ways]; > cxlmd_peer = cxled_to_memdev(cxled_peer); > ep_peer = cxl_ep_load(port, cxlmd_peer); > if (ep->dport != ep_peer->dport) { > @@ -1105,20 +1115,15 @@ static int cxl_port_setup_targets(struct cxl_port *port, > > cxlsd = to_cxl_switch_decoder(&cxld->dev); > if (cxl_rr->nr_targets_set) { > - int i, distance; > + int i; > > /* > - * Passthrough decoders impose no distance requirements between > - * peers > + * Check if this ep is already in the switch target list and > + * if so ensure it meets relative positioning requirements. > */ > - if (cxl_rr->nr_targets == 1) > - distance = 0; > - else > - distance = p->nr_targets / cxl_rr->nr_targets; > for (i = 0; i < cxl_rr->nr_targets_set; i++) > if (ep->dport == cxlsd->target[i]) { > - rc = check_last_peer(cxled, ep, cxl_rr, > - distance); > + rc = check_last_peer(cxled, ep, cxl_rr); > if (rc) > return rc; > goto out_target_set; >
diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c index a1eac592c66a..2edfc02bb766 100644 --- a/drivers/cxl/core/region.c +++ b/drivers/cxl/core/region.c @@ -1037,8 +1037,7 @@ static void cxl_port_detach_region(struct cxl_port *port, } static int check_last_peer(struct cxl_endpoint_decoder *cxled, - struct cxl_ep *ep, struct cxl_region_ref *cxl_rr, - int distance) + struct cxl_ep *ep, struct cxl_region_ref *cxl_rr) { struct cxl_memdev *cxlmd = cxled_to_memdev(cxled); struct cxl_region *cxlr = cxl_rr->region; @@ -1048,19 +1047,30 @@ static int check_last_peer(struct cxl_endpoint_decoder *cxled, struct cxl_memdev *cxlmd_peer; struct cxl_ep *ep_peer; int pos = cxled->pos; + int ancestral_ways; + + if (cxl_rr->nr_targets == 1) { + /* + * Passthrough decoders impose no positioning requirements + * between peers. + */ + return 0; + } + + ancestral_ways = p->nr_targets / cxl_rr->nr_targets; /* * If this position wants to share a dport with the last endpoint mapped - * then that endpoint, at index 'position - distance', must also be + * then that endpoint, at index 'position - ancestral_ways', must also be * mapped by this dport. */ - if (pos < distance) { + if (pos < ancestral_ways) { dev_dbg(&cxlr->dev, "%s:%s: cannot host %s:%s at %d\n", dev_name(port->uport_dev), dev_name(&port->dev), dev_name(&cxlmd->dev), dev_name(&cxled->cxld.dev), pos); return -ENXIO; } - cxled_peer = p->targets[pos - distance]; + cxled_peer = p->targets[pos - ancestral_ways]; cxlmd_peer = cxled_to_memdev(cxled_peer); ep_peer = cxl_ep_load(port, cxlmd_peer); if (ep->dport != ep_peer->dport) { @@ -1105,20 +1115,15 @@ static int cxl_port_setup_targets(struct cxl_port *port, cxlsd = to_cxl_switch_decoder(&cxld->dev); if (cxl_rr->nr_targets_set) { - int i, distance; + int i; /* - * Passthrough decoders impose no distance requirements between - * peers + * Check if this ep is already in the switch target list and + * if so ensure it meets relative positioning requirements. */ - if (cxl_rr->nr_targets == 1) - distance = 0; - else - distance = p->nr_targets / cxl_rr->nr_targets; for (i = 0; i < cxl_rr->nr_targets_set; i++) if (ep->dport == cxlsd->target[i]) { - rc = check_last_peer(cxled, ep, cxl_rr, - distance); + rc = check_last_peer(cxled, ep, cxl_rr); if (rc) return rc; goto out_target_set;
'distance' is equivalent to the product of the interleave_ways of the "ancestor" decoders of the port's decoder we are setting up in cxl_port_setup_targets(). So use the term "ancestral_ways" instead of "distance" to better clarify the meaning of this value. Also move all logic around this value directly into check_last_peer() so that all necessary calculations are in one place. It also allows eliminating a parameter to that function. Signed-off-by: Jim Harris <jim.harris@samsung.com> --- drivers/cxl/core/region.c | 33 +++++++++++++++++++-------------- 1 file changed, 19 insertions(+), 14 deletions(-)