[PATCH 0/4] cxl/region: Update and test performance calculations

[PATCH 0/4] cxl/region: Update and test performance calculations

[Alison Schofield]

CXL regions expose latency and bandwidth through their access0 and access1
attributes. The same performance data also contributes to NUMA node access
coordinates and memory tier placement.

This series updates region performance calculation and reporting,
consolidates the completion-time operations, and adds cxl_test support
for validating the results.

This work was prompted by AI-assisted patch reviews that identified ways
stale contributions from failed or removed targets could affect region
performance data.

Patch 1 addresses stale contributions to region performance data by
recalculating from the attached targets only when the region completes.

Patch 2 makes region access attributes visible as soon as performance
data is available.

Patch 3 consolidates the three completion-time operations into one helper
shared by user-created and autodiscovered regions.

Patch 4 assigns distinct latency and bandwidth values to cxl_test memdevs
so that region performance calculations can be tested.

A new CXL unit test will be posted separately.
Testing with cxl_test:
- With all four patches, the new test and existing ndctl CXL tests pass.
- Without patch 1, rejected, cleared, and rebuilt target cases report
  incorrect latency or bandwidth.
- Without patch 2, the region access0 and access1 directories remain empty.

Based on v7.4-rc1.


Alison Schofield (4):
  cxl/region: Calculate performance data once all targets are attached
  cxl/region: Show access attributes once performance data is calculated
  cxl/region: Consolidate region performance completion
  cxl/test: Give each mock memdev its own performance data

 drivers/cxl/core/cdat.c      | 19 +++++++++++++++--
 drivers/cxl/core/region.c    | 40 +++++++++++++++++++++---------------
 drivers/cxl/cxl.h            |  3 +--
 tools/testing/cxl/test/cxl.c | 21 ++++++++++++++-----
 4 files changed, 57 insertions(+), 26 deletions(-)


base-commit: 6628e7d4c6b25c8d18f1345e3a20d21829aae7e4
-- 
2.37.3

[PATCH 1/4] cxl/region: Calculate performance data once all targets are attached

[Alison Schofield]

Region performance data is accumulated as endpoint decoders attempt to
join a region. Failed attachments and subsequently removed targets can
leave stale contributions in the region's performance coordinates.

Latency reflects the worst of all attempted targets, potentially
misreporting the region's access coordinates and affecting NUMA memory
tier placement. Bandwidth is normally recalculated when the region
completes, but if that calculation cannot be performed, the accumulated
bandwidth remains. Rebuilding a region can inflate it further.

Calculate performance data from the attached targets when the region
completes, for both user-created and autodiscovered regions. Reset the
coordinates before each calculation so only current members contribute.
Perform this calculation before the shared-upstream bandwidth update,
preserving the sum of member bandwidths if that update cannot complete.

Fixes: 3d9f4a197230 ("cxl/region: Calculate performance data for a region")
Signed-off-by: Alison Schofield <alison.schofield@intel.com>
---
 drivers/cxl/core/cdat.c   | 19 +++++++++++++++++--
 drivers/cxl/core/region.c |  4 ++--
 drivers/cxl/cxl.h         |  3 +--
 3 files changed, 20 insertions(+), 6 deletions(-)

diff --git a/drivers/cxl/core/cdat.c b/drivers/cxl/core/cdat.c
index f5f86b69c86d..7acc516ae9fa 100644
--- a/drivers/cxl/core/cdat.c
+++ b/drivers/cxl/core/cdat.c
@@ -1124,8 +1124,8 @@ void cxl_region_shared_upstream_bandwidth_update(struct cxl_region *cxlr)
 	cxl_region_update_bandwidth(cxlr, usp_xa);
 }
 
-void cxl_region_perf_data_calculate(struct cxl_region *cxlr,
-				    struct cxl_endpoint_decoder *cxled)
+static void cxl_region_perf_data_add(struct cxl_region *cxlr,
+				     struct cxl_endpoint_decoder *cxled)
 {
 	struct cxl_dpa_perf *perf;
 
@@ -1147,3 +1147,18 @@ void cxl_region_perf_data_calculate(struct cxl_region *cxlr,
 		cxlr->coord[i].write_bandwidth += perf->coord[i].write_bandwidth;
 	}
 }
+
+/**
+ * cxl_region_perf_data_update - Recalculate region performance data
+ * @cxlr: region with all targets attached
+ *
+ * Reset the access coordinates and calculate them from the current targets.
+ */
+void cxl_region_perf_data_update(struct cxl_region *cxlr)
+{
+	struct cxl_region_params *p = &cxlr->params;
+
+	memset(cxlr->coord, 0, sizeof(cxlr->coord));
+	for (int i = 0; i < p->nr_targets; i++)
+		cxl_region_perf_data_add(cxlr, p->targets[i]);
+}
diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
index a048bd5c06cb..34d42559e33e 100644
--- a/drivers/cxl/core/region.c
+++ b/drivers/cxl/core/region.c
@@ -2130,8 +2130,6 @@ static int cxl_region_attach(struct cxl_region *cxlr,
 		return -EINVAL;
 	}
 
-	cxl_region_perf_data_calculate(cxlr, cxled);
-
 	if (test_bit(CXL_REGION_F_AUTO, &cxlr->flags)) {
 		int i;
 
@@ -2171,6 +2169,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);
 
 		return 0;
@@ -2193,6 +2192,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);
 	}
 
diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
index baa6df2d362a..2cd1842ce1ad 100644
--- a/drivers/cxl/cxl.h
+++ b/drivers/cxl/cxl.h
@@ -932,8 +932,7 @@ void cxl_switch_parse_cdat(struct cxl_dport *dport);
 
 int cxl_endpoint_get_perf_coordinates(struct cxl_port *port,
 				      struct access_coordinate *coord);
-void cxl_region_perf_data_calculate(struct cxl_region *cxlr,
-				    struct cxl_endpoint_decoder *cxled);
+void cxl_region_perf_data_update(struct cxl_region *cxlr);
 void cxl_region_shared_upstream_bandwidth_update(struct cxl_region *cxlr);
 
 void cxl_memdev_update_perf(struct cxl_memdev *cxlmd);
-- 
2.37.3

[PATCH 2/4] cxl/region: Show access attributes once performance data is calculated

[Alison Schofield]

Region access attributes expose calculated latency and bandwidth to
userspace, allowing the performance of individual CXL regions to be
inspected independently of their use as system RAM.

Currently, attribute visibility is refreshed only when performance data is
propagated to a NUMA node. Regions whose memory is not onlined as system
RAM, such as pmem and devdax regions, can have valid performance data that
userspace cannot inspect.

Refresh attribute visibility when the region completes and its performance
data has been calculated. The existing visibility checks continue to hide
attributes without valid values, and NUMA node performance updates remain
unchanged.

Fixes: c20eaf44113e ("cxl/region: Add sysfs attribute for locality attributes of CXL regions")
Signed-off-by: Alison Schofield <alison.schofield@intel.com>
---
 drivers/cxl/core/region.c | 29 ++++++++++++++++-------------
 1 file changed, 16 insertions(+), 13 deletions(-)

diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
index 34d42559e33e..50cd483d710d 100644
--- a/drivers/cxl/core/region.c
+++ b/drivers/cxl/core/region.c
@@ -144,6 +144,19 @@ static const struct attribute_group *get_cxl_region_access1_group(void)
 	return &cxl_region_access1_coordinate_group;
 }
 
+/*
+ * Region access attributes depend on calculated performance data, not
+ * NUMA node onlining. Re-evaluate visibility after region completion.
+ */
+static void cxl_region_update_access_groups(struct cxl_region *cxlr)
+{
+	if (sysfs_update_group(&cxlr->dev.kobj, get_cxl_region_access0_group()))
+		dev_dbg(&cxlr->dev, "Failed to update access0 group\n");
+
+	if (sysfs_update_group(&cxlr->dev.kobj, get_cxl_region_access1_group()))
+		dev_dbg(&cxlr->dev, "Failed to update access1 group\n");
+}
+
 static ssize_t uuid_show(struct device *dev, struct device_attribute *attr,
 			 char *buf)
 {
@@ -2171,6 +2184,7 @@ static int cxl_region_attach(struct cxl_region *cxlr,
 		p->state = CXL_CONFIG_COMMIT;
 		cxl_region_perf_data_update(cxlr);
 		cxl_region_shared_upstream_bandwidth_update(cxlr);
+		cxl_region_update_access_groups(cxlr);
 
 		return 0;
 	}
@@ -2194,6 +2208,7 @@ static int cxl_region_attach(struct cxl_region *cxlr,
 		p->state = CXL_CONFIG_ACTIVE;
 		cxl_region_perf_data_update(cxlr);
 		cxl_region_shared_upstream_bandwidth_update(cxlr);
+		cxl_region_update_access_groups(cxlr);
 	}
 
 	cxled->cxld.interleave_ways = p->interleave_ways;
@@ -2629,7 +2644,6 @@ static struct cxl_region *cxl_region_alloc(struct cxl_root_decoder *cxlrd, int i
 static bool cxl_region_update_coordinates(struct cxl_region *cxlr, int nid)
 {
 	int cset = 0;
-	int rc;
 
 	for (int i = 0; i < ACCESS_COORDINATE_MAX; i++) {
 		if (cxlr->coord[i].read_bandwidth) {
@@ -2650,18 +2664,7 @@ static bool cxl_region_update_coordinates(struct cxl_region *cxlr, int nid)
 		}
 	}
 
-	if (!cset)
-		return false;
-
-	rc = sysfs_update_group(&cxlr->dev.kobj, get_cxl_region_access0_group());
-	if (rc)
-		dev_dbg(&cxlr->dev, "Failed to update access0 group\n");
-
-	rc = sysfs_update_group(&cxlr->dev.kobj, get_cxl_region_access1_group());
-	if (rc)
-		dev_dbg(&cxlr->dev, "Failed to update access1 group\n");
-
-	return true;
+	return cset > 0;
 }
 
 static int cxl_region_perf_attrs_callback(struct notifier_block *nb,
-- 
2.37.3

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

[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);
 	}
 
 	cxled->cxld.interleave_ways = p->interleave_ways;
-- 
2.37.3

[PATCH 4/4] cxl/test: Give each mock memdev its own performance data

[Alison Schofield]

Assign distinct and predictable latency and bandwidth values to mock
memdevs so region performance calculations can be tested against the
expected contributions of individual targets.

Derive each memdev's performance data from its memdev id:
  latency   = 100 + 10 * id     (nanoseconds)
  bandwidth = 1000 + 100 * id   (MB/s)

These values allow tests to detect stale or duplicate contributions from
targets that are not members of the completed region. Since mock memdevs
are not PCI devices, the shared-upstream bandwidth calculation is skipped.
Region bandwidth in cxl_test will be the sum of the individual memdev
bandwidths, providing a predictable result for testing.

Signed-off-by: Alison Schofield <alison.schofield@intel.com>
---
 tools/testing/cxl/test/cxl.c | 21 ++++++++++++++++-----
 1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/tools/testing/cxl/test/cxl.c b/tools/testing/cxl/test/cxl.c
index 3eb1051b1ee8..762c8b9ed4dd 100644
--- a/tools/testing/cxl/test/cxl.c
+++ b/tools/testing/cxl/test/cxl.c
@@ -1629,18 +1629,29 @@ static struct cxl_dport *mock_cxl_add_dport_by_dev(struct cxl_port *port,
 }
 
 /*
- * Faking the cxl_dpa_perf for the memdev when appropriate.
+ * Derive mock performance data from the memdev id so tests can identify
+ * each memdev's contribution to region performance.
+ *
+ *   latency   = 100 + 10 * id     (nanoseconds)
+ *   bandwidth = 1000 + 100 * id   (MB/s)
+ *
+ * Keep the values nonzero so the region access attributes are visible.
+ * The ndctl cxl-perf-data.sh test relies on these formulas.
  */
 static void dpa_perf_setup(struct cxl_port *endpoint, struct range *range,
 			   struct cxl_dpa_perf *dpa_perf)
 {
+	struct cxl_memdev *cxlmd = to_cxl_memdev(endpoint->uport_dev);
+	unsigned int bandwidth = 1000 + 100 * cxlmd->id;
+	unsigned int latency = 100 + 10 * cxlmd->id;
+
 	dpa_perf->qos_class = FAKE_QTG_ID;
 	dpa_perf->dpa_range = *range;
 	for (int i = 0; i < ACCESS_COORDINATE_MAX; i++) {
-		dpa_perf->coord[i].read_latency = 500;
-		dpa_perf->coord[i].write_latency = 500;
-		dpa_perf->coord[i].read_bandwidth = 1000;
-		dpa_perf->coord[i].write_bandwidth = 1000;
+		dpa_perf->coord[i].read_latency = latency;
+		dpa_perf->coord[i].write_latency = latency;
+		dpa_perf->coord[i].read_bandwidth = bandwidth;
+		dpa_perf->coord[i].write_bandwidth = bandwidth;
 	}
 }
 
-- 
2.37.3

Re: [PATCH 1/4] cxl/region: Calculate performance data once all targets are attached

[Li Ming]

在 2026/11/5 11:28, Alison Schofield 写道:
> Region performance data is accumulated as endpoint decoders attempt to
> join a region. Failed attachments and subsequently removed targets can
> leave stale contributions in the region's performance coordinates.
>
> Latency reflects the worst of all attempted targets, potentially
> misreporting the region's access coordinates and affecting NUMA memory
> tier placement. Bandwidth is normally recalculated when the region
> completes, but if that calculation cannot be performed, the accumulated
> bandwidth remains. Rebuilding a region can inflate it further.
>
> Calculate performance data from the attached targets when the region
> completes, for both user-created and autodiscovered regions. Reset the
> coordinates before each calculation so only current members contribute.
> Perform this calculation before the shared-upstream bandwidth update,
> preserving the sum of member bandwidths if that update cannot complete.
>
> Fixes: 3d9f4a197230 ("cxl/region: Calculate performance data for a region")
> Signed-off-by: Alison Schofield <alison.schofield@intel.com>
Reviewed-by: Li Ming <ming.li@zohomail.com>

> ---
>   drivers/cxl/core/cdat.c   | 19 +++++++++++++++++--
>   drivers/cxl/core/region.c |  4 ++--
>   drivers/cxl/cxl.h         |  3 +--
>   3 files changed, 20 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/cxl/core/cdat.c b/drivers/cxl/core/cdat.c
> index f5f86b69c86d..7acc516ae9fa 100644
> --- a/drivers/cxl/core/cdat.c
> +++ b/drivers/cxl/core/cdat.c
> @@ -1124,8 +1124,8 @@ void cxl_region_shared_upstream_bandwidth_update(struct cxl_region *cxlr)
>   	cxl_region_update_bandwidth(cxlr, usp_xa);
>   }
>   
> -void cxl_region_perf_data_calculate(struct cxl_region *cxlr,
> -				    struct cxl_endpoint_decoder *cxled)
> +static void cxl_region_perf_data_add(struct cxl_region *cxlr,
> +				     struct cxl_endpoint_decoder *cxled)
>   {
>   	struct cxl_dpa_perf *perf;
>   
> @@ -1147,3 +1147,18 @@ void cxl_region_perf_data_calculate(struct cxl_region *cxlr,
>   		cxlr->coord[i].write_bandwidth += perf->coord[i].write_bandwidth;
>   	}
>   }
> +
> +/**
> + * cxl_region_perf_data_update - Recalculate region performance data
> + * @cxlr: region with all targets attached
> + *
> + * Reset the access coordinates and calculate them from the current targets.
> + */
> +void cxl_region_perf_data_update(struct cxl_region *cxlr)
> +{
> +	struct cxl_region_params *p = &cxlr->params;
> +
> +	memset(cxlr->coord, 0, sizeof(cxlr->coord));
> +	for (int i = 0; i < p->nr_targets; i++)
> +		cxl_region_perf_data_add(cxlr, p->targets[i]);
> +}
> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index a048bd5c06cb..34d42559e33e 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -2130,8 +2130,6 @@ static int cxl_region_attach(struct cxl_region *cxlr,
>   		return -EINVAL;
>   	}
>   
> -	cxl_region_perf_data_calculate(cxlr, cxled);
> -
>   	if (test_bit(CXL_REGION_F_AUTO, &cxlr->flags)) {
>   		int i;
>   
> @@ -2171,6 +2169,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);
>   
>   		return 0;
> @@ -2193,6 +2192,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);
>   	}
>   
> diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
> index baa6df2d362a..2cd1842ce1ad 100644
> --- a/drivers/cxl/cxl.h
> +++ b/drivers/cxl/cxl.h
> @@ -932,8 +932,7 @@ void cxl_switch_parse_cdat(struct cxl_dport *dport);
>   
>   int cxl_endpoint_get_perf_coordinates(struct cxl_port *port,
>   				      struct access_coordinate *coord);
> -void cxl_region_perf_data_calculate(struct cxl_region *cxlr,
> -				    struct cxl_endpoint_decoder *cxled);
> +void cxl_region_perf_data_update(struct cxl_region *cxlr);
>   void cxl_region_shared_upstream_bandwidth_update(struct cxl_region *cxlr);
>   
>   void cxl_memdev_update_perf(struct cxl_memdev *cxlmd);

Re: [PATCH 2/4] cxl/region: Show access attributes once performance data is calculated

[Li Ming]

在 2026/11/5 11:28, Alison Schofield 写道:
> Region access attributes expose calculated latency and bandwidth to
> userspace, allowing the performance of individual CXL regions to be
> inspected independently of their use as system RAM.
>
> Currently, attribute visibility is refreshed only when performance data is
> propagated to a NUMA node. Regions whose memory is not onlined as system
> RAM, such as pmem and devdax regions, can have valid performance data that
> userspace cannot inspect.
>
> Refresh attribute visibility when the region completes and its performance
> data has been calculated. The existing visibility checks continue to hide
> attributes without valid values, and NUMA node performance updates remain
> unchanged.
>
> Fixes: c20eaf44113e ("cxl/region: Add sysfs attribute for locality attributes of CXL regions")
> Signed-off-by: Alison Schofield <alison.schofield@intel.com>
Reviewed-by: Li Ming <ming.li@zohomail.com>

> ---
>   drivers/cxl/core/region.c | 29 ++++++++++++++++-------------
>   1 file changed, 16 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index 34d42559e33e..50cd483d710d 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -144,6 +144,19 @@ static const struct attribute_group *get_cxl_region_access1_group(void)
>   	return &cxl_region_access1_coordinate_group;
>   }
>   
> +/*
> + * Region access attributes depend on calculated performance data, not
> + * NUMA node onlining. Re-evaluate visibility after region completion.
> + */
> +static void cxl_region_update_access_groups(struct cxl_region *cxlr)
> +{
> +	if (sysfs_update_group(&cxlr->dev.kobj, get_cxl_region_access0_group()))
> +		dev_dbg(&cxlr->dev, "Failed to update access0 group\n");
> +
> +	if (sysfs_update_group(&cxlr->dev.kobj, get_cxl_region_access1_group()))
> +		dev_dbg(&cxlr->dev, "Failed to update access1 group\n");
> +}
> +
>   static ssize_t uuid_show(struct device *dev, struct device_attribute *attr,
>   			 char *buf)
>   {
> @@ -2171,6 +2184,7 @@ static int cxl_region_attach(struct cxl_region *cxlr,
>   		p->state = CXL_CONFIG_COMMIT;
>   		cxl_region_perf_data_update(cxlr);
>   		cxl_region_shared_upstream_bandwidth_update(cxlr);
> +		cxl_region_update_access_groups(cxlr);
>   
>   		return 0;
>   	}
> @@ -2194,6 +2208,7 @@ static int cxl_region_attach(struct cxl_region *cxlr,
>   		p->state = CXL_CONFIG_ACTIVE;
>   		cxl_region_perf_data_update(cxlr);
>   		cxl_region_shared_upstream_bandwidth_update(cxlr);
> +		cxl_region_update_access_groups(cxlr);
>   	}
>   
>   	cxled->cxld.interleave_ways = p->interleave_ways;
> @@ -2629,7 +2644,6 @@ static struct cxl_region *cxl_region_alloc(struct cxl_root_decoder *cxlrd, int i
>   static bool cxl_region_update_coordinates(struct cxl_region *cxlr, int nid)
>   {
>   	int cset = 0;
> -	int rc;
>   
>   	for (int i = 0; i < ACCESS_COORDINATE_MAX; i++) {
>   		if (cxlr->coord[i].read_bandwidth) {
> @@ -2650,18 +2664,7 @@ static bool cxl_region_update_coordinates(struct cxl_region *cxlr, int nid)
>   		}
>   	}
>   
> -	if (!cset)
> -		return false;
> -
> -	rc = sysfs_update_group(&cxlr->dev.kobj, get_cxl_region_access0_group());
> -	if (rc)
> -		dev_dbg(&cxlr->dev, "Failed to update access0 group\n");
> -
> -	rc = sysfs_update_group(&cxlr->dev.kobj, get_cxl_region_access1_group());
> -	if (rc)
> -		dev_dbg(&cxlr->dev, "Failed to update access1 group\n");
> -
> -	return true;
> +	return cset > 0;
>   }
>   
>   static int cxl_region_perf_attrs_callback(struct notifier_block *nb,

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

[Li Ming]

在 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.
>   	cxled->cxld.interleave_ways = p->interleave_ways;

Re: [PATCH 4/4] cxl/test: Give each mock memdev its own performance data

[Li Ming]

在 2026/11/5 11:28, Alison Schofield 写道:
> Assign distinct and predictable latency and bandwidth values to mock
> memdevs so region performance calculations can be tested against the
> expected contributions of individual targets.
>
> Derive each memdev's performance data from its memdev id:
>    latency   = 100 + 10 * id     (nanoseconds)
>    bandwidth = 1000 + 100 * id   (MB/s)
>
> These values allow tests to detect stale or duplicate contributions from
> targets that are not members of the completed region. Since mock memdevs
> are not PCI devices, the shared-upstream bandwidth calculation is skipped.
> Region bandwidth in cxl_test will be the sum of the individual memdev
> bandwidths, providing a predictable result for testing.
>
> Signed-off-by: Alison Schofield <alison.schofield@intel.com>
Reviewed-by: Li Ming <ming.li@zohomail.com>

> ---
>   tools/testing/cxl/test/cxl.c | 21 ++++++++++++++++-----
>   1 file changed, 16 insertions(+), 5 deletions(-)
>
> diff --git a/tools/testing/cxl/test/cxl.c b/tools/testing/cxl/test/cxl.c
> index 3eb1051b1ee8..762c8b9ed4dd 100644
> --- a/tools/testing/cxl/test/cxl.c
> +++ b/tools/testing/cxl/test/cxl.c
> @@ -1629,18 +1629,29 @@ static struct cxl_dport *mock_cxl_add_dport_by_dev(struct cxl_port *port,
>   }
>   
>   /*
> - * Faking the cxl_dpa_perf for the memdev when appropriate.
> + * Derive mock performance data from the memdev id so tests can identify
> + * each memdev's contribution to region performance.
> + *
> + *   latency   = 100 + 10 * id     (nanoseconds)
> + *   bandwidth = 1000 + 100 * id   (MB/s)
> + *
> + * Keep the values nonzero so the region access attributes are visible.
> + * The ndctl cxl-perf-data.sh test relies on these formulas.
>    */
>   static void dpa_perf_setup(struct cxl_port *endpoint, struct range *range,
>   			   struct cxl_dpa_perf *dpa_perf)
>   {
> +	struct cxl_memdev *cxlmd = to_cxl_memdev(endpoint->uport_dev);
> +	unsigned int bandwidth = 1000 + 100 * cxlmd->id;
> +	unsigned int latency = 100 + 10 * cxlmd->id;
> +
>   	dpa_perf->qos_class = FAKE_QTG_ID;
>   	dpa_perf->dpa_range = *range;
>   	for (int i = 0; i < ACCESS_COORDINATE_MAX; i++) {
> -		dpa_perf->coord[i].read_latency = 500;
> -		dpa_perf->coord[i].write_latency = 500;
> -		dpa_perf->coord[i].read_bandwidth = 1000;
> -		dpa_perf->coord[i].write_bandwidth = 1000;
> +		dpa_perf->coord[i].read_latency = latency;
> +		dpa_perf->coord[i].write_latency = latency;
> +		dpa_perf->coord[i].read_bandwidth = bandwidth;
> +		dpa_perf->coord[i].write_bandwidth = bandwidth;
>   	}
>   }
>   

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

[Alison Schofield]

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;

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

[Li Ming]

在 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>
Reviewed-by: Li Ming <ming.li@zohomail.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);
>   	}
>   
>   	cxled->cxld.interleave_ways = p->interleave_ways;

Re: [PATCH 0/4] cxl/region: Update and test performance calculations

[Dave Jiang]

On 11/4/26 8:28 PM, Alison Schofield wrote:
> CXL regions expose latency and bandwidth through their access0 and access1
> attributes. The same performance data also contributes to NUMA node access
> coordinates and memory tier placement.
> 
> This series updates region performance calculation and reporting,
> consolidates the completion-time operations, and adds cxl_test support
> for validating the results.
> 
> This work was prompted by AI-assisted patch reviews that identified ways
> stale contributions from failed or removed targets could affect region
> performance data.
> 
> Patch 1 addresses stale contributions to region performance data by
> recalculating from the attached targets only when the region completes.
> 
> Patch 2 makes region access attributes visible as soon as performance
> data is available.
> 
> Patch 3 consolidates the three completion-time operations into one helper
> shared by user-created and autodiscovered regions.
> 
> Patch 4 assigns distinct latency and bandwidth values to cxl_test memdevs
> so that region performance calculations can be tested.
> 
> A new CXL unit test will be posted separately.
> Testing with cxl_test:
> - With all four patches, the new test and existing ndctl CXL tests pass.
> - Without patch 1, rejected, cleared, and rebuilt target cases report
>   incorrect latency or bandwidth.
> - Without patch 2, the region access0 and access1 directories remain empty.
> 
> Based on v7.4-rc1.
> 
> 
> Alison Schofield (4):
>   cxl/region: Calculate performance data once all targets are attached
>   cxl/region: Show access attributes once performance data is calculated
>   cxl/region: Consolidate region performance completion
>   cxl/test: Give each mock memdev its own performance data
> 
>  drivers/cxl/core/cdat.c      | 19 +++++++++++++++--
>  drivers/cxl/core/region.c    | 40 +++++++++++++++++++++---------------
>  drivers/cxl/cxl.h            |  3 +--
>  tools/testing/cxl/test/cxl.c | 21 ++++++++++++++-----
>  4 files changed, 57 insertions(+), 26 deletions(-)
> 
> 
> base-commit: 6628e7d4c6b25c8d18f1345e3a20d21829aae7e4

Applied to cxl/next:
cbba07ba71d5 cxl/test: Give each mock memdev its own performance data
cd60d381afc1 cxl/region: Consolidate region performance completion
5911237cee95 cxl/region: Show access attributes once performance data is calculated
57f95cb59ffd cxl/region: Calculate performance data once all targets are attached