Re: [PATCH v5 12/12] cxl/region: Deal with numa nodes not enumarated by SRAT

[Dave Jiang <dave.jiang@intel.com>]

On 2/15/24 10:24 AM, Jonathan Cameron wrote:
> On Tue, 6 Feb 2024 15:28:40 -0700
> Dave Jiang <dave.jiang@intel.com> wrote:
> 
>> For the numa nodes that are not created by SRAT, no memory_target is
>> allocated and is not managed by the HMAT_REPORTING code. Therefore
>> hmat_callback() memory hotplug notifier will exit early on those NUMA
>> nodes. The CXL memory hotplug notifier will need to call
>> node_set_perf_attrs() directly in order to setup the access sysfs
>> attributes.
>>
>> In acpi_numa_init(), the last proximity domain (pxm) id created by SRAT is
>> stored. Add a helper function acpi_node_backed_by_real_pxm() in order to
>> check if a NUMA node id is defined by SRAT or created by CFMWS.
>>
>> node_set_perf_attrs() symbol is exported to allow update of perf attribs
>> for a node. The sysfs path of
>> /sys/devices/system/node/nodeX/access0/initiators/* is created by
>> ndoe_set_perf_attrs() for the various attributes where nodeX is matched
> node
> 
>> to the NUMA node of the CXL region.
>>
>> Cc: Rafael J. Wysocki <rafael@kernel.org>
>> Reviewed-by: Alison Schofield <alison.schofield@intel.com>
>> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> Trivial comment inline.
> Otherwise LGTM
> 
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
>> ---
>> v5:
>> - Fix ARM compile of missing prototype (0day)
>> ---
> 
> 
> 
>> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
>> index 19e419f18472..db51e35cb44f 100644
>> --- a/drivers/cxl/core/region.c
>> +++ b/drivers/cxl/core/region.c
>> @@ -3084,7 +3084,12 @@ 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) {
>> -			rc = cxl_update_hmat_access_coordinates(nid, cxlr, i);
>> +			rc = 0;
>> +			if (cxl_need_node_perf_attrs_update(nid))
>> +				node_set_perf_attrs(nid, &cxlr->coord[i], i);
> maybe
> continue;
> here

Whether we are updating the node attrs or the hmat perf attrs, we want to refresh the CXL sysfs attrs to make sure is_visible() has changed.

DJ
 
>> +			else
>> +				rc = cxl_update_hmat_access_coordinates(nid, cxlr, i);
>> +
> 
> Then no need to indent the above
> 
>>  			if (rc == 0)
>>  				cset++;
>>  		}
>