Hi Jan, On 2022/9/27 15:37, Jan Beulich wrote:
On 20.09.2022 11:12, Wei Chen wrote:--- a/xen/arch/x86/numa.c +++ b/xen/arch/x86/numa.c @@ -50,9 +50,28 @@ nodemask_t __read_mostly node_online_map = { { [0] = 1UL } }; bool numa_off; s8 acpi_numa = 0;-int srat_disabled(void)+int __init arch_numa_setup(const char *opt) { - return numa_off || acpi_numa < 0; +#ifdef CONFIG_ACPI_NUMA + if ( !strncmp(opt, "noacpi", 6) ) + { + numa_off = false; + acpi_numa = -1;When making the v5 changes, did you go over the results to check they are actually consistent? I'm afraid they still aren't, because of the line above: Here we disable NUMA, but that doesn't mean there's broken firmware.
Yes, you're right. I had not realized it while I was modifying this patch.
Therefore I guess I need to ask for another round of renaming of the two helper functions; I'm sorry for that. What you introduce ...+ return 0; + } +#endif + + return -EINVAL; +} + +bool arch_numa_broken(void) +{ + return acpi_numa < 0; +}... here wants to be arch_numa_disabled(), whereas the function currently named this way (in patch 5) wants to be e.g. arch_numa_unavailable() (or, using inverted sense, arch_numa_available()). Of course I'll be happy to see other naming suggestions for both functions, as long as they reflect the respective purposes. Alternatively, to retain the current naming, the assignments to acpi_numa would need revising. But I think that would be the more fragile approach.
Yes, I agree with you, I will rename these two functions. Your suggested names are reasonable, I will use them in next version.
Cheers, Wei Chen
Jan
