Re: [PATCH 3/4] cxl/region: Consolidate region performance completion

[Alison Schofield <alison.schofield@intel.com>]

On Fri, Nov 06, 2026 at 02:15:47PM +0800, Li Ming wrote:
> 
> 在 2026/11/5 11:28, Alison Schofield 写道:
> > Both user-created and autodiscovered regions perform the same three
> > operations when all targets are attached: calculate performance data,
> > account for shared upstream bandwidth, and update the visibility of the
> > region access attributes.
> > 
> > Consolidate these operations in cxl_region_perf_data_complete() to keep
> > the completion paths consistent and preserve their ordering.
> > 
> > No functional change.
> > 
> > Signed-off-by: Alison Schofield <alison.schofield@intel.com>
> > ---
> >   drivers/cxl/core/region.c | 15 +++++++++------
> >   1 file changed, 9 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> > index 50cd483d710d..f9435e65c695 100644
> > --- a/drivers/cxl/core/region.c
> > +++ b/drivers/cxl/core/region.c
> > @@ -157,6 +157,13 @@ static void cxl_region_update_access_groups(struct cxl_region *cxlr)
> >   		dev_dbg(&cxlr->dev, "Failed to update access1 group\n");
> >   }
> > +static void cxl_region_perf_data_complete(struct cxl_region *cxlr)
> > +{
> > +	cxl_region_perf_data_update(cxlr);
> > +	cxl_region_shared_upstream_bandwidth_update(cxlr);
> > +	cxl_region_update_access_groups(cxlr);
> > +}
> > +
> >   static ssize_t uuid_show(struct device *dev, struct device_attribute *attr,
> >   			 char *buf)
> >   {
> > @@ -2182,9 +2189,7 @@ static int cxl_region_attach(struct cxl_region *cxlr,
> >   		 * then the region is already committed.
> >   		 */
> >   		p->state = CXL_CONFIG_COMMIT;
> > -		cxl_region_perf_data_update(cxlr);
> > -		cxl_region_shared_upstream_bandwidth_update(cxlr);
> > -		cxl_region_update_access_groups(cxlr);
> > +		cxl_region_perf_data_complete(cxlr);
> >   		return 0;
> >   	}
> > @@ -2206,9 +2211,7 @@ static int cxl_region_attach(struct cxl_region *cxlr,
> >   		if (rc)
> >   			return rc;
> >   		p->state = CXL_CONFIG_ACTIVE;
> > -		cxl_region_perf_data_update(cxlr);
> > -		cxl_region_shared_upstream_bandwidth_update(cxlr);
> > -		cxl_region_update_access_groups(cxlr);
> > +		cxl_region_perf_data_complete(cxlr);
> >   	}
> Is it possible to call cxl_region_perf_data_complete() inside
> cxl_region_setup_targets()? So that we don't need to call it separately for
> user-created and autodiscovered regions.

Thanks for the reviews and feedback Ming -

That would work, yet I prefer to keep the two calls explicit. That makes
it clear when the region has reached its completed state and perf data is
finalized.

Note that cxl_region_setup_targets() configures port decoder targets, while
cxl_region_perf_data_complete() handles performance data and sysfs visiblity.
I think those operations are better kept separate.

-- Alison


> >   	cxled->cxld.interleave_ways = p->interleave_ways;