cxl/region: Update and test performance calculations

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 <[email protected]>

---
  drivers/cxl/core/cdat.c   | 19 +++++++++++++++++--
  drivers/cxl/core/region.c |  4 ++--
  drivers/cxl/cxl.h         |  3 +--
  3 files changed, 20 insertions(+), 6 deletions(-)

drivers/cxl/core/cdat.c +17 -2

index f5f86b69c86d..7acc516ae9fa 100644
@@ -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]);
+}

drivers/cxl/core/region.c +2 -2

index a048bd5c06cb..34d42559e33e 100644
@@ -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);
  	}
 

drivers/cxl/cxl.h +1 -2

index baa6df2d362a..2cd1842ce1ad 100644
@@ -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);

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 <[email protected]>

---
  drivers/cxl/core/region.c | 29 ++++++++++++++++-------------
  1 file changed, 16 insertions(+), 13 deletions(-)

drivers/cxl/core/region.c +16 -13

index 34d42559e33e..50cd483d710d 100644
@@ -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,

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 <[email protected]>

---
  drivers/cxl/core/region.c | 15 +++++++++------
  1 file changed, 9 insertions(+), 6 deletions(-)

drivers/cxl/core/region.c +8 -3

index 50cd483d710d..f9435e65c695 100644
@@ -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;
  	}

drivers/cxl/core/region.c +0 -0

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

Quoted 46 lines · 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 <[email protected]>

---
  drivers/cxl/core/region.c | 15 +++++++++------
  1 file changed, 9 insertions(+), 6 deletions(-)

drivers/cxl/core/region.c +8 -3

index 50cd483d710d..f9435e65c695 100644
@@ -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;

drivers/cxl/core/region.c static int cxl_region_attach(struct cxl_region *cxlr, · line 2195

Quoted 5 lines

drivers/cxl/core/region.c +1 -3

-		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.

Quoted 1 line

drivers/cxl/core/region.c +0 -0

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

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 <[email protected]>

---
  drivers/cxl/core/region.c | 15 +++++++++------
  1 file changed, 9 insertions(+), 6 deletions(-)

drivers/cxl/core/region.c +9 -6

index 50cd483d710d..f9435e65c695 100644
@@ -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;

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 <[email protected]>

---
  tools/testing/cxl/test/cxl.c | 21 ++++++++++++++++-----
  1 file changed, 16 insertions(+), 5 deletions(-)

tools/testing/cxl/test/cxl.c +16 -5

index 3eb1051b1ee8..762c8b9ed4dd 100644
@@ -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;
  	}
  }
 

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