Hi, I was about to send this patch as a part of a cleanup patchset that goes on top of last Michal's work about properly init memoryless nodes [1], but I thought I may send it as a standalone one as it is the only real fix of that series. Discussing the fix with Michal, another concern came up, about whether memoryless-nodes but with CPUs should be marked as online at all. Here Michal and I have different opinions, while I think that memoryless-nodes with its CPUs online should be marked online, he thinks that the property 'online' applied to node has to do more with memory, and I have to confess that after having a look at some of the usages of for_each_online_node, most of them are checked to do something memory-realted afterwards, so I guess he has a point there. My main concern is that if memoryless-nodes do not get online, its sysfs showing the cpu<->numa topology will not be created and I am not sure about losing that information. E.g: i guess 'numactl -H' would not be able to show the right topolovy as it coul not walk sysfs to get it right (in case it does). Anyway, as a quick test, I wanted to see what happens if init_node_to_cpu() and init_gi_nodes(), do not online the nodes. (those nodes get online because of the CPU and Generic Initiator affinity). The outcome is that we blow up badly down the chain, and we do because of a nasty side-effect (more information can be found at the end of patch#1) Short-long story: by the time bringup_nonboot_cpus() gets called to bring up the cpus and bring up their respective nodes, we have not even registered the node_subsys bus yet, so bringup_nonboot_cpus()->..->__try_online_node->()register_one_node()->bus_add_device() will crash when dereferencing some of the bus' struct fields. We happen not to crash now, because init_to_cpu_node() and init_gi_nodes() mark the node online, and by doing that, it has the effect of __try_online_node() backing off and not calling register_one_node(). I think the whole thing is kinda broken, or at the very least subtle and fragile. I am willing to have a look at how we can make this optimal, but wanted to share with you. Anyway, It is getting late here, so I just wanted to 1) send this quick fix, 2) expose some nasty behavior and 3) kick off a discussion about how to improve this situation. [1] https://lore.kernel.org/lkml/20220127085305.20890-1-mhocko@xxxxxxxxxx/ Oscar Salvador (1): arch/x86/mm/numa: Do not initialize nodes twice arch/x86/mm/numa.c | 15 ++------------- include/linux/mm.h | 1 - mm/page_alloc.c | 2 +- 3 files changed, 3 insertions(+), 15 deletions(-) -- 2.34.1
