* Re: [PATCH v2] cxl/region: Create node access attributes for CFMWS-only NUMA nodes
2026-09-17 2:46 [PATCH v2] cxl/region: Create node access attributes for CFMWS-only NUMA nodes Guixin Liu
@ 2026-09-17 3:00 ` sashiko-bot
2026-09-17 21:06 ` Jonathan Cameron
2026-09-17 3:05 ` Gregory Price
` (2 subsequent siblings)
3 siblings, 1 reply; 6+ messages in thread
From: sashiko-bot @ 2026-09-17 3:00 UTC (permalink / raw)
To: Guixin Liu; +Cc: linux-cxl
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] cxl/region: missing access attributes for SRAT nodes without HMAT
--
commit 2503e3249f202233ccb3653d33cc651692452053
Author: Guixin Liu <kanie@linux.alibaba.com>
cxl/region: Create node access attributes for CFMWS-only NUMA nodes
A NUMA node that only backs a CXL fixed memory window and is not
enumerated by SRAT has no memory_target in the HMAT code, so
hmat_callback() exits early at find_mem_target() and never creates
the node's accessN attributes. Create the attributes with
node_set_perf_attrs() when the node is not backed by a real SRAT pxm.
> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index f54acbf68e844..5ef0ca0694ff1 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
[ ... ]
> @@ -2631,7 +2632,19 @@ static bool cxl_region_update_coordinates(struct cxl_region *cxlr, int nid)
>
> for (int i = 0; i < ACCESS_COORDINATE_MAX; i++) {
> if (cxlr->coord[i].read_bandwidth) {
> - node_update_perf_attrs(nid, &cxlr->coord[i], i);
> + /*
> + * A CFMWS-only node (not backed by a real SRAT pxm)
> + * has no HMAT memory_target, so hmat_callback()
> + * never created the access attributes for it:
> + * create them here instead of only updating them.
> + * This is called at most once per node (see
> + * nodemask_region_seen), so the attributes are not
> + * created twice.
> + */
> + if (!acpi_node_backed_by_real_pxm(nid))
> + node_set_perf_attrs(nid, &cxlr->coord[i], i);
> + else
> + node_update_perf_attrs(nid, &cxlr->coord[i], i);
[Severity: Medium]
Will this silently drop access attributes for SRAT nodes that lack HMAT
entries?
If the system provides an ACPI SRAT table that enumerates a NUMA node,
acpi_node_backed_by_real_pxm() will return true. However, if there is no
corresponding HMAT table or entry, hmat_callback() exits early and never
calls node_set_perf_attrs() to create the node's access attributes.
When CXL regions later come online, cxl_region_update_coordinates() checks
acpi_node_backed_by_real_pxm(), which evaluates to true. It therefore calls
node_update_perf_attrs() instead of node_set_perf_attrs().
Because the attributes were never created, node_update_perf_attrs() finds
an empty node->access_list and silently returns, which drops CXL access
coordinate sysfs attributes and NUMA mempolicy updates. Using an ACPI
topology check to guess the initialization state of generic struct node
sysfs attributes leaves the underlying bug unresolved for standard SRAT
nodes lacking HMAT data.
> cset++;
> }
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260917024632.1397380-1-kanie@linux.alibaba.com?part=1
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v2] cxl/region: Create node access attributes for CFMWS-only NUMA nodes
2026-09-17 3:00 ` sashiko-bot
@ 2026-09-17 21:06 ` Jonathan Cameron
0 siblings, 0 replies; 6+ messages in thread
From: Jonathan Cameron @ 2026-09-17 21:06 UTC (permalink / raw)
To: sashiko-bot; +Cc: sashiko-reviews, Guixin Liu, linux-cxl
On Thu, 17 Sep 2026 03:00:49 +0000
sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] cxl/region: missing access attributes for SRAT nodes without HMAT
> --
>
> commit 2503e3249f202233ccb3653d33cc651692452053
> Author: Guixin Liu <kanie@linux.alibaba.com>
>
> cxl/region: Create node access attributes for CFMWS-only NUMA nodes
>
> A NUMA node that only backs a CXL fixed memory window and is not
> enumerated by SRAT has no memory_target in the HMAT code, so
> hmat_callback() exits early at find_mem_target() and never creates
> the node's accessN attributes. Create the attributes with
> node_set_perf_attrs() when the node is not backed by a real SRAT pxm.
>
> > diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> > index f54acbf68e844..5ef0ca0694ff1 100644
> > --- a/drivers/cxl/core/region.c
> > +++ b/drivers/cxl/core/region.c
> [ ... ]
> > @@ -2631,7 +2632,19 @@ static bool cxl_region_update_coordinates(struct cxl_region *cxlr, int nid)
> >
> > for (int i = 0; i < ACCESS_COORDINATE_MAX; i++) {
> > if (cxlr->coord[i].read_bandwidth) {
> > - node_update_perf_attrs(nid, &cxlr->coord[i], i);
> > + /*
> > + * A CFMWS-only node (not backed by a real SRAT pxm)
> > + * has no HMAT memory_target, so hmat_callback()
> > + * never created the access attributes for it:
> > + * create them here instead of only updating them.
> > + * This is called at most once per node (see
> > + * nodemask_region_seen), so the attributes are not
> > + * created twice.
> > + */
> > + if (!acpi_node_backed_by_real_pxm(nid))
> > + node_set_perf_attrs(nid, &cxlr->coord[i], i);
> > + else
> > + node_update_perf_attrs(nid, &cxlr->coord[i], i);
>
> [Severity: Medium]
> Will this silently drop access attributes for SRAT nodes that lack HMAT
> entries?
>
> If the system provides an ACPI SRAT table that enumerates a NUMA node,
> acpi_node_backed_by_real_pxm() will return true. However, if there is no
> corresponding HMAT table or entry, hmat_callback() exits early and never
> calls node_set_perf_attrs() to create the node's access attributes.
>
> When CXL regions later come online, cxl_region_update_coordinates() checks
> acpi_node_backed_by_real_pxm(), which evaluates to true. It therefore calls
> node_update_perf_attrs() instead of node_set_perf_attrs().
>
> Because the attributes were never created, node_update_perf_attrs() finds
> an empty node->access_list and silently returns, which drops CXL access
> coordinate sysfs attributes and NUMA mempolicy updates. Using an ACPI
> topology check to guess the initialization state of generic struct node
> sysfs attributes leaves the underlying bug unresolved for standard SRAT
> nodes lacking HMAT data.
>
If there is no HMAT, there is no perf data to the generic ports and
none of this can be created or updated anyway.
Jonathan
> > cset++;
> > }
> > }
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] cxl/region: Create node access attributes for CFMWS-only NUMA nodes
2026-09-17 2:46 [PATCH v2] cxl/region: Create node access attributes for CFMWS-only NUMA nodes Guixin Liu
2026-09-17 3:00 ` sashiko-bot
@ 2026-09-17 3:05 ` Gregory Price
2026-09-17 22:44 ` Alison Schofield
2026-09-18 15:39 ` Dave Jiang
3 siblings, 0 replies; 6+ messages in thread
From: Gregory Price @ 2026-09-17 3:05 UTC (permalink / raw)
To: Guixin Liu
Cc: Davidlohr Bueso, Jonathan Cameron, Dave Jiang, Alison Schofield,
Vishal Verma, Dan Williams, Ira Weiny, Li Ming, linux-cxl
On Thu, Sep 17, 2026 at 10:46:32AM +0800, Guixin Liu wrote:
> A NUMA node that only backs a CXL fixed memory window and is not
> enumerated by SRAT has no memory_target in the HMAT code, so
> hmat_callback() exits early at find_mem_target() and never creates
> the node's accessN attributes:
> /sys/devices/system/node/nodeX/accessN/initiators/* stays missing.
>
> Commit debdce20c4f2 ("cxl/region: Deal with numa nodes not enumerated
> by SRAT") had the CXL region notifier call node_set_perf_attrs()
> directly for such nodes, but commit 2e454fb8056d ("cxl, acpi/hmat:
> Update CXL access coordinates directly instead of through HMAT")
> replaced it with node_update_perf_attrs(), which only updates
> attributes that already exist and thus does nothing for these nodes.
>
> Create the attributes with node_set_perf_attrs() when the node is not
> backed by a real SRAT pxm.
>
> Tested on a QEMU CXL topology with a CFMWS window not described by
> SRAT or HMAT: node1/access0/initiators/{read,write}_{bandwidth,latency}
> appear with the calculated coordinates once the region's first memory
> block comes online.
>
> Fixes: 2e454fb8056d ("cxl, acpi/hmat: Update CXL access coordinates directly instead of through HMAT")
> Reviewed-by: Jonathan Cameron <jonathan.cameron@oss.qualcomm.com>
> Signed-off-by: Guixin Liu <kanie@linux.alibaba.com>
Reviewed-by: Gregory Price (Meta) <gourry@gourry.net>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v2] cxl/region: Create node access attributes for CFMWS-only NUMA nodes
2026-09-17 2:46 [PATCH v2] cxl/region: Create node access attributes for CFMWS-only NUMA nodes Guixin Liu
2026-09-17 3:00 ` sashiko-bot
2026-09-17 3:05 ` Gregory Price
@ 2026-09-17 22:44 ` Alison Schofield
2026-09-18 15:39 ` Dave Jiang
3 siblings, 0 replies; 6+ messages in thread
From: Alison Schofield @ 2026-09-17 22:44 UTC (permalink / raw)
To: Guixin Liu
Cc: Davidlohr Bueso, Jonathan Cameron, Dave Jiang, Vishal Verma,
Dan Williams, Ira Weiny, Li Ming, linux-cxl
On Thu, Sep 17, 2026 at 10:46:32AM +0800, Guixin Liu wrote:
> A NUMA node that only backs a CXL fixed memory window and is not
> enumerated by SRAT has no memory_target in the HMAT code, so
> hmat_callback() exits early at find_mem_target() and never creates
> the node's accessN attributes:
> /sys/devices/system/node/nodeX/accessN/initiators/* stays missing.
>
> Commit debdce20c4f2 ("cxl/region: Deal with numa nodes not enumerated
> by SRAT") had the CXL region notifier call node_set_perf_attrs()
> directly for such nodes, but commit 2e454fb8056d ("cxl, acpi/hmat:
> Update CXL access coordinates directly instead of through HMAT")
> replaced it with node_update_perf_attrs(), which only updates
> attributes that already exist and thus does nothing for these nodes.
>
> Create the attributes with node_set_perf_attrs() when the node is not
> backed by a real SRAT pxm.
>
> Tested on a QEMU CXL topology with a CFMWS window not described by
> SRAT or HMAT: node1/access0/initiators/{read,write}_{bandwidth,latency}
> appear with the calculated coordinates once the region's first memory
> block comes online.
Reviewed-by: Alison Schofield <alison.schofield@intel.com>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v2] cxl/region: Create node access attributes for CFMWS-only NUMA nodes
2026-09-17 2:46 [PATCH v2] cxl/region: Create node access attributes for CFMWS-only NUMA nodes Guixin Liu
` (2 preceding siblings ...)
2026-09-17 22:44 ` Alison Schofield
@ 2026-09-18 15:39 ` Dave Jiang
3 siblings, 0 replies; 6+ messages in thread
From: Dave Jiang @ 2026-09-18 15:39 UTC (permalink / raw)
To: Guixin Liu, Davidlohr Bueso, Jonathan Cameron, Alison Schofield,
Vishal Verma, Dan Williams, Ira Weiny, Li Ming
Cc: linux-cxl
On 9/16/26 7:46 PM, Guixin Liu wrote:
> A NUMA node that only backs a CXL fixed memory window and is not
> enumerated by SRAT has no memory_target in the HMAT code, so
> hmat_callback() exits early at find_mem_target() and never creates
> the node's accessN attributes:
> /sys/devices/system/node/nodeX/accessN/initiators/* stays missing.
>
> Commit debdce20c4f2 ("cxl/region: Deal with numa nodes not enumerated
> by SRAT") had the CXL region notifier call node_set_perf_attrs()
> directly for such nodes, but commit 2e454fb8056d ("cxl, acpi/hmat:
> Update CXL access coordinates directly instead of through HMAT")
> replaced it with node_update_perf_attrs(), which only updates
> attributes that already exist and thus does nothing for these nodes.
>
> Create the attributes with node_set_perf_attrs() when the node is not
> backed by a real SRAT pxm.
>
> Tested on a QEMU CXL topology with a CFMWS window not described by
> SRAT or HMAT: node1/access0/initiators/{read,write}_{bandwidth,latency}
> appear with the calculated coordinates once the region's first memory
> block comes online.
>
> Fixes: 2e454fb8056d ("cxl, acpi/hmat: Update CXL access coordinates directly instead of through HMAT")
> Reviewed-by: Jonathan Cameron <jonathan.cameron@oss.qualcomm.com>
> Signed-off-by: Guixin Liu <kanie@linux.alibaba.com>
Applied to cxl/next:
ef71014055f7
> ---
> Changes since v1 (https://lore.kernel.org/r/20260916120338.369436-1-kanie@linux.alibaba.com):
>
> - Open code the one-use helper (Dave Jiang)
> - Comment why creating the attributes here is safe: this path runs
> at most once per node (Jonathan Cameron)
> - Lead with the user visible regression (Alison Schofield)
> ---
> drivers/cxl/core/region.c | 15 ++++++++++++++-
> 1 file changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index 27e63e6dab7c..9a5fd3da539d 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -1,5 +1,6 @@
> // SPDX-License-Identifier: GPL-2.0-only
> /* Copyright(c) 2022 Intel Corporation. All rights reserved. */
> +#include <linux/acpi.h>
> #include <linux/memregion.h>
> #include <linux/genalloc.h>
> #include <linux/debugfs.h>
> @@ -2630,7 +2631,19 @@ static bool cxl_region_update_coordinates(struct cxl_region *cxlr, int nid)
>
> for (int i = 0; i < ACCESS_COORDINATE_MAX; i++) {
> if (cxlr->coord[i].read_bandwidth) {
> - node_update_perf_attrs(nid, &cxlr->coord[i], i);
> + /*
> + * A CFMWS-only node (not backed by a real SRAT pxm)
> + * has no HMAT memory_target, so hmat_callback()
> + * never created the access attributes for it:
> + * create them here instead of only updating them.
> + * This is called at most once per node (see
> + * nodemask_region_seen), so the attributes are not
> + * created twice.
> + */
> + if (!acpi_node_backed_by_real_pxm(nid))
> + node_set_perf_attrs(nid, &cxlr->coord[i], i);
> + else
> + node_update_perf_attrs(nid, &cxlr->coord[i], i);
> cset++;
> }
> }
^ permalink raw reply [flat|nested] 6+ messages in thread