Re: [PATCH] cxl/region: Create node access attributes for CFMWS-only NUMA nodes

[Guixin Liu <kanie@linux.alibaba.com>]

在 2026/9/17 02:00, Jonathan Cameron 写道:
> On Wed, 16 Sep 2026 20:03:38 +0800
> Guixin Liu <kanie@linux.alibaba.com> 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. When the
>> first memory of such a node comes online, hmat_callback() exits early
>> at find_mem_target() without creating anything, and
>> node_update_perf_attrs() only updates attributes that already exist,
>> so the node never gets its accessN sysfs attributes at all:
>> /sys/devices/system/node/nodeX/accessN/initiators/* stays missing even
>> though the region's own sysfs reports the coordinates.
>>
>> Commit debdce20c4f2 ("cxl/region: Deal with numa nodes not enumerated
>> by SRAT") handled this by having 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 that call with node_update_perf_attrs() on the assumption
>> that the HMAT callback has already created the attributes. That
>> assumption does not hold for CFMWS-only nodes.
>>
>> Restore the distinction: when the node is not backed by a real SRAT
>> pxm, create the attributes with node_set_perf_attrs() instead of
>> only trying to update them.
>>
>> Tested on a QEMU CXL topology with a CFMWS window whose memory is not
>> described by SRAT or HMAT: after the region's first memory block comes
>> online, node1 has no accessN directory before this patch; with it,
>> node1/access0/initiators/{read,write}_{bandwidth,latency} appear with
>> the calculated coordinates.
>>
>> Fixes: 2e454fb8056d ("cxl, acpi/hmat: Update CXL access coordinates directly instead of through HMAT")
>> Signed-off-by: Guixin Liu <kanie@linux.alibaba.com>
> Ah.  I should have caught this one.  Indeed looks correct.
>
> My only concern is it is relying heavily on the fact we only do this
> once which is not apparent at this layer in the code.  You need to go
> looking in cxl_region_perf_attrs_callback() for that.
>
> It is also a one way gate so even you tear down all regions in the NUMA
> node (so the CFWMS) we don't do this update again.
>
> Anyhow, I think all we need here is a note that this function will only
> be called once per CFWMS and thus it is fine to register the sysfs attrs
> here.
OK, added in v2, thanks.

Best Regards,
Guixin Liu
>
> With that added, this looks good to me
> Reviewed-by: Jonathan Cameron <jonathan.cameron@oss.qualcomm.com>
>
>
>> ---
>> checkpatch reports one "Prefer a maximum 75 chars per line" warning:
>> the 2e454fb8056d reference line above is 84 columns because checkpatch
>> requires the quoted title to match the git subject exactly, and that
>> subject is 62 characters long. Wrapping it instead produces a
>> GIT_COMMIT_ID error, so the long line is the lesser evil.
>>   drivers/cxl/core/cdat.c   | 5 +++++
>>   drivers/cxl/core/core.h   | 1 +
>>   drivers/cxl/core/region.c | 5 ++++-
>>   3 files changed, 10 insertions(+), 1 deletion(-)
>> ---
>>   drivers/cxl/core/cdat.c   | 5 +++++
>>   drivers/cxl/core/core.h   | 1 +
>>   drivers/cxl/core/region.c | 5 ++++-
>>   3 files changed, 10 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/cxl/core/cdat.c b/drivers/cxl/core/cdat.c
>> index 5c9f07262513..e28c40159d94 100644
>> --- a/drivers/cxl/core/cdat.c
>> +++ b/drivers/cxl/core/cdat.c
>> @@ -1072,3 +1072,8 @@ void cxl_region_perf_data_calculate(struct cxl_region *cxlr,
>>   		cxlr->coord[i].write_bandwidth += perf->coord[i].write_bandwidth;
>>   	}
>>   }
>> +
>> +bool cxl_need_node_perf_attrs_update(int nid)
>> +{
>> +	return !acpi_node_backed_by_real_pxm(nid);
>> +}
>> diff --git a/drivers/cxl/core/core.h b/drivers/cxl/core/core.h
>> index 35eaf636adc9..19a5b8edbfda 100644
>> --- a/drivers/cxl/core/core.h
>> +++ b/drivers/cxl/core/core.h
>> @@ -229,4 +229,5 @@ int cxl_set_feature(struct cxl_mailbox *cxl_mbox, const uuid_t *feat_uuid,
>>   
>>   resource_size_t cxl_rcd_component_reg_phys(struct device *dev,
>>   					   struct cxl_dport *dport);
>> +bool cxl_need_node_perf_attrs_update(int nid);
>>   #endif /* __CXL_CORE_H__ */
>> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
>> index 27e63e6dab7c..636435733754 100644
>> --- a/drivers/cxl/core/region.c
>> +++ b/drivers/cxl/core/region.c
>> @@ -2630,7 +2630,10 @@ 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);
>> +			if (cxl_need_node_perf_attrs_update(nid))
>> +				node_set_perf_attrs(nid, &cxlr->coord[i], i);
>> +			else
>> +				node_update_perf_attrs(nid, &cxlr->coord[i], i);
>>   			cset++;
>>   		}
>>   	}