Re: [PATCH v7 5/6] xen/x86: move NUMA process nodes nodes code from x86 to common

[Jan Beulich <jbeulich@xxxxxxxx>]

On 09.11.2022 09:51, Wei Chen wrote:
>> -----Original Message-----
>> From: Jan Beulich <jbeulich@xxxxxxxx>
>> Sent: 2022年11月9日 0:55
>>
>>> @@ -341,159 +247,14 @@ acpi_numa_memory_affinity_init(const struct
>> acpi_srat_mem_affinity *ma)
>>>             pxm &= 0xff;
>>>     node = setup_node(pxm);
>>>     if (node == NUMA_NO_NODE) {
>>> -           bad_srat();
>>> +           numa_fw_bad();
>>>             return;
>>> -                           }
>>> -           } while (found && start < end);
>>> -
>>> -           if (start < end) {
>>> -                   printk(KERN_ERR "NUMA: No NODE for RAM range: "
>>> -                           "[%"PRIpaddr", %"PRIpaddr"]\n", start, end - 1);
>>> -                   return 0;
>>> -           }
>>> -   }
>>> -   return 1;
>>> +   numa_fw_nid_name = "PXM";
>>
>> ... this to be happening too late. Not because I can see a way for current
>> code to use the variable earlier, but because of the risk of future code
>> potentially doing so. Afaics srat_parse_regions() is called quite a bit
>> earlier, so perhaps the field should (also?) be set there, presumably
>> after acpi_table_parse() has succeeded. I've included "(also?)" because I
>> think to be on the safe side the setting here may want keeping, albeit
>> perhaps moving up in the function.
>>
> 
> When I was composing this patch, I also thought current place to call this
> "PXM" setting would be a little late. But since there is only one function
> that uses this prefix right now, I thought it was acceptable at the time.
> But obviously your concerns make sense, I will move this call to
> srat_parse_regions after acpi_table_parse has been done successfully.

As said - perhaps not move, but copy. There is an (extremely unlikely) case
where srat_parse_regions() would not be called at all.

Jan