arch/x86/mm/numa: Do not initialize nodes twice
On x86, prior to ("mm: handle uninitialized numa nodes gracecully"), NUMA nodes could be allocated at three different places. - numa_register_memblks - init_cpu_to_node - init_gi_nodes All these calls happen at setup_arch, and have the following order: setup_arch ... x86_numa_init numa_init numa_register_memblks ... init_cpu_to_node init_memory_less_node alloc_node_data free_area_init_memoryless_node init_gi_nodes init_memory_less_node alloc_node_data free_area_init_memoryless_node numa_register_memblks() is only interested in those nodes which have memory, so it skips over any memoryless node it founds. Later on, when we have read ACPI's SRAT table, we call init_cpu_to_node() and init_gi_nodes(), which initialize any memoryless node we might have that have either CPU or Initiator affinity, meaning we allocate pg_data_t struct for them and we mark them as ONLINE. So far so good, but the thing is that after ("mm: handle uninitialized numa nodes gracefully"), we allocate all possible NUMA nodes in free_area_init(), meaning we have a picture like the following: setup_arch x86_numa_init numa_init numa_register_memblks <-- allocate non-memoryless node x86_init.paging.pagetable_init ... free_area_init free_area_init_memoryless <-- allocate memoryless node init_cpu_to_node alloc_node_data <-- allocate memoryless node with CPU free_area_init_memoryless_node init_gi_nodes alloc_node_data <-- allocate memoryless node with Initiator free_area_init_memoryless_node free_area_init() already allocates all possible NUMA nodes, but init_cpu_to_node() and init_gi_nodes() are clueless about that, so they go ahead and allocate a new pg_data_t struct without checking anything, meaning we end up allocating twice. It should be mad clear that this only happens in the case where memoryless NUMA node happens to have a CPU/Initiator affinity. So get rid of init_memory_less_node() and just set the node online. Note that setting the node online is needed, otherwise we choke down the chain when bringup_nonboot_cpus() ends up calling __try_online_node()->register_one_node()->... and we blow up in bus_add_device(). As can be seen here: BUG: kernel NULL pointer dereference, address: 0000000000000060 #PF: supervisor read access in kernel mode #PF: error_code(0x0000) - not-present page PGD 0 P4D 0 Oops: 0000 [#1] PREEMPT SMP DEBUG_PAGEALLOC PTI CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.17.0-rc4-1-default+ #45 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.0.0-prebuilt.qemu-project.org 04/4 RIP: 0010:bus_add_device+0x5a/0x140 Code: 8b 74 24 20 48 89 df e8 84 96 ff ff 85 c0 89 c5 75 38 48 8b 53 50 48 85 d2 0f 84 bb 00 004 RSP: 0000:ffffc9000022bd10 EFLAGS: 00010246 RAX: 0000000000000000 RBX: ffff888100987400 RCX: ffff8881003e4e19 RDX: ffff8881009a5e00 RSI: ffff888100987400 RDI: ffff888100987400 RBP: 0000000000000000 R08: ffff8881003e4e18 R09: ffff8881003e4c98 R10: 0000000000000000 R11: ffff888100402bc0 R12: ffffffff822ceba0 R13: 0000000000000000 R14: ffff888100987400 R15: 0000000000000000 FS: 0000000000000000(0000) GS:ffff88853fc00000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 0000000000000060 CR3: 000000000200a001 CR4: 00000000001706b0 Call Trace: device_add+0x4c0/0x910 __register_one_node+0x97/0x2d0 __try_online_node+0x85/0xc0 try_online_node+0x25/0x40 cpu_up+0x4f/0x100 bringup_nonboot_cpus+0x4f/0x60 smp_init+0x26/0x79 kernel_init_freeable+0x130/0x2f1 kernel_init+0x17/0x150 ret_from_fork+0x22/0x30 The reason is simple, by the time bringup_nonboot_cpus() gets called, we did not register the node_subsys bus yet, so we crash when bus_add_device() tries to dereference bus()->p. The following shows the order of the calls: kernel_init_freeable smp_init bringup_nonboot_cpus ... bus_add_device() <- we did not register node_subsys yet do_basic_setup do_initcalls postcore_initcall(register_node_type); register_node_type subsys_system_register subsys_register bus_register <- register node_subsys bus Why setting the node online saves us then? Well, simply because __try_online_node() backs off when the node is online, meaning we do not end up calling register_one_node() in the first place. This is subtle, broken and deserves a deep analysis and thought about how to put this into shape, but for now let us have this easy fix for the leaking memory issue. [osalvador@suse.de: add comments] Link: https://lkml.kernel.org/r/20220221142649.3457-1-osalvador@suse.de Link: https://lkml.kernel.org/r/20220218224302.5282-2-osalvador@suse.de Fixes: da4490c958ad ("mm: handle uninitialized numa nodes gracefully") Signed-off-by: Oscar Salvador <osalvador@suse.de> Acked-by: Michal Hocko <mhocko@suse.com> Cc: David Hildenbrand <david@redhat.com> Cc: Rafael Aquini <raquini@redhat.com> Cc: Dave Hansen <dave.hansen@linux.intel.com> Cc: Wei Yang <richard.weiyang@gmail.com> Cc: Dennis Zhou <dennis@kernel.org> Cc: Alexey Makhalov <amakhalov@vmware.com> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
This commit is contained in:
Родитель
2a791f4412
Коммит
1ca75fa7f1
|
@ -738,17 +738,6 @@ void __init x86_numa_init(void)
|
|||
numa_init(dummy_numa_init);
|
||||
}
|
||||
|
||||
static void __init init_memory_less_node(int nid)
|
||||
{
|
||||
/* Allocate and initialize node data. Memory-less node is now online.*/
|
||||
alloc_node_data(nid);
|
||||
free_area_init_memoryless_node(nid);
|
||||
|
||||
/*
|
||||
* All zonelists will be built later in start_kernel() after per cpu
|
||||
* areas are initialized.
|
||||
*/
|
||||
}
|
||||
|
||||
/*
|
||||
* A node may exist which has one or more Generic Initiators but no CPUs and no
|
||||
|
@ -766,9 +755,18 @@ void __init init_gi_nodes(void)
|
|||
{
|
||||
int nid;
|
||||
|
||||
/*
|
||||
* Exclude this node from
|
||||
* bringup_nonboot_cpus
|
||||
* cpu_up
|
||||
* __try_online_node
|
||||
* register_one_node
|
||||
* because node_subsys is not initialized yet.
|
||||
* TODO remove dependency on node_online
|
||||
*/
|
||||
for_each_node_state(nid, N_GENERIC_INITIATOR)
|
||||
if (!node_online(nid))
|
||||
init_memory_less_node(nid);
|
||||
node_set_online(nid);
|
||||
}
|
||||
|
||||
/*
|
||||
|
@ -798,8 +796,17 @@ void __init init_cpu_to_node(void)
|
|||
if (node == NUMA_NO_NODE)
|
||||
continue;
|
||||
|
||||
/*
|
||||
* Exclude this node from
|
||||
* bringup_nonboot_cpus
|
||||
* cpu_up
|
||||
* __try_online_node
|
||||
* register_one_node
|
||||
* because node_subsys is not initialized yet.
|
||||
* TODO remove dependency on node_online
|
||||
*/
|
||||
if (!node_online(node))
|
||||
init_memory_less_node(node);
|
||||
node_set_online(node);
|
||||
|
||||
numa_set_node(cpu, node);
|
||||
}
|
||||
|
|
|
@ -2449,7 +2449,6 @@ static inline spinlock_t *pud_lock(struct mm_struct *mm, pud_t *pud)
|
|||
}
|
||||
|
||||
extern void __init pagecache_init(void);
|
||||
extern void __init free_area_init_memoryless_node(int nid);
|
||||
extern void free_initmem(void);
|
||||
|
||||
/*
|
||||
|
|
|
@ -7626,7 +7626,7 @@ static void __init free_area_init_node(int nid)
|
|||
free_area_init_core(pgdat);
|
||||
}
|
||||
|
||||
void __init free_area_init_memoryless_node(int nid)
|
||||
static void __init free_area_init_memoryless_node(int nid)
|
||||
{
|
||||
free_area_init_node(nid);
|
||||
}
|
||||
|
|
Загрузка…
Ссылка в новой задаче