On 10.10.2022 09:03, Wei Chen wrote: > On 2022/10/9 15:25, Wei Chen wrote: >>>>> Even more so an answer to my question would be nice: You'll then have >>>>> CONFIG_HAS_NUMA_NODE_FWID=y even on Arm (using PXM as mandated by ACPI >>>>> when in ACPI mode). But then what's the FWID for DT? I know it was me >>>>> to suggest this build time distinction, but I'm afraid I wasn't doing >>>>> much good with that (and I'm sorry). >>>> >>>> How about introducing a flag for selected NUMA implementation to >>>> set it in runtime? >>>> For example: >>>> bool numa_has_fw_nodeid; >>>> >>>> ACPI NUMA will set this flag to 1, but 0 for DT NUMA. >>> >>> That's an option alongside going back to what you had in an earlier >>> version. Another would be (name subject to improvement) >>> >>> const char *__ro_after_init numa_fw_nid_name; >>> > > When I was dealing with this comment, I found that I was still a little > unclear: > > When we were introducing "CONFIG_HAS_NUMA_NODE_FWID", we wanted to > eliminate the redundant code of: > if ( fwid_name not equal to "node" ) > printk(KERN_INFO "NUMA: Node %u %s %u [%"PRIpaddr"%"PRIpaddr"]%s\n", > node, fwid_name , arch_nid, start, end - 1, > hotplug ? " (hotplug)" : ""); > else > printk(KERN_INFO "NUMA: Node %u [%"PRIpaddr", %"PRIpaddr"]%s\n", > node, start, end - 1, hotplug ? " (hotplug)" : ""); > > But when I am working with numa_fw_nid_name, I find it's still not > easy to reduce above redundant code. For example:
As said - this attempt to limit redundancy was a mistake when it comes to the existence of two models in parallel for an arch (with either one picked at runtime), unless ... > "NUMA: Node %u %s %u > When numa_fw_nid_name = NULL, we can print "" for %s, but can't reduce > the second %u. > > So can we split this message into 3 lines like: > printk(KERN_INFO "NUMA: Node %u"...); > if (numa_fw_nid_name) > printk(KERN_INFO " %s %u"...); > printk(KERN_INFO "[%"PRIpaddr"%"PRIpaddr"]%s\n"...); > > Or another option, we can force each NUMA implementation to assign a > string for numa_fw_nid_name. For example, in DT NUMA, we can assign > numa_fw_nid_name="SOCKET". ... we can assume numa_fw_nid_name to always be non-NULL (is there's any way to reach this piece of code). I have no insight whether "socket" is a correct term to use in the DT case; that would need to be confirmed by an Arm person. Jan
