Commit 767507654c22 ("arch_numa: switch over to numa_memblks") significantly cleaned up the NUMA registration code, but also dropped a significant check that was refusing to accept to configure a memblock with an invalid nid.
On "quality hardware" such as my ThunderX machine, this results in a kernel that dies immediately:
[ 0.000000] Booting Linux on physical CPU 0x0000000000 [0x431f0a10] [ 0.000000] Linux version 6.12.0-00013-g8920d74cf8db (maz@valley-girl) (gcc (Debian 12.2.0-14) 12.2.0, GNU ld (GNU Binutils for Debian) 2.40) #3872 SMP PREEMPT Wed Nov 27 15:25:49 GMT 2024 [ 0.000000] KASLR disabled due to lack of seed [ 0.000000] Machine model: Cavium ThunderX CN88XX board [ 0.000000] efi: EFI v2.4 by American Megatrends [ 0.000000] efi: ESRT=0xffce0ff18 SMBIOS 3.0=0xfffb0000 ACPI 2.0=0xffec60000 MEMRESERVE=0xffc905d98 [ 0.000000] esrt: Reserving ESRT space from 0x0000000ffce0ff18 to 0x0000000ffce0ff50. [ 0.000000] earlycon: pl11 at MMIO 0x000087e024000000 (options '115200n8') [ 0.000000] printk: legacy bootconsole [pl11] enabled [ 0.000000] NODE_DATA(0) allocated [mem 0xff6754580-0xff67566bf] [ 0.000000] Unable to handle kernel paging request at virtual address 0000000000001d40 [ 0.000000] Mem abort info: [ 0.000000] ESR = 0x0000000096000004 [ 0.000000] EC = 0x25: DABT (current EL), IL = 32 bits [ 0.000000] SET = 0, FnV = 0 [ 0.000000] EA = 0, S1PTW = 0 [ 0.000000] FSC = 0x04: level 0 translation fault [ 0.000000] Data abort info: [ 0.000000] ISV = 0, ISS = 0x00000004, ISS2 = 0x00000000 [ 0.000000] CM = 0, WnR = 0, TnD = 0, TagAccess = 0 [ 0.000000] GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0 [ 0.000000] [0000000000001d40] user address but active_mm is swapper [ 0.000000] Internal error: Oops: 0000000096000004 [#1] PREEMPT SMP [ 0.000000] Modules linked in: [ 0.000000] CPU: 0 UID: 0 PID: 0 Comm: swapper Not tainted 6.12.0-00013-g8920d74cf8db #3872 [ 0.000000] Hardware name: Cavium ThunderX CN88XX board (DT) [ 0.000000] pstate: a00000c5 (NzCv daIF -PAN -UAO -TCO -DIT -SSBS BTYPE=--) [ 0.000000] pc : sparse_init_nid+0x54/0x428 [ 0.000000] lr : sparse_init+0x118/0x240 [ 0.000000] sp : ffff800081da3cb0 [ 0.000000] x29: ffff800081da3cb0 x28: 0000000fedbab10c x27: 0000000000000001 [ 0.000000] x26: 0000000ffee250f8 x25: 0000000000000001 x24: ffff800082102cd0 [ 0.000000] x23: 0000000000000001 x22: 0000000000000000 x21: 00000000001fffff [ 0.000000] x20: 0000000000000001 x19: 0000000000000000 x18: ffffffffffffffff [ 0.000000] x17: 0000000001b00000 x16: 0000000ffd130000 x15: 0000000000000000 [ 0.000000] x14: 00000000003e0000 x13: 00000000000001c8 x12: 0000000000000014 [ 0.000000] x11: ffff800081e82860 x10: ffff8000820fb2c8 x9 : ffff8000820fb490 [ 0.000000] x8 : 0000000000ffed20 x7 : 0000000000000014 x6 : 00000000001fffff [ 0.000000] x5 : 00000000ffffffff x4 : 0000000000000000 x3 : 0000000000000000 [ 0.000000] x2 : 0000000000000000 x1 : 0000000000000040 x0 : 0000000000000007 [ 0.000000] Call trace: [ 0.000000] sparse_init_nid+0x54/0x428 [ 0.000000] sparse_init+0x118/0x240 [ 0.000000] bootmem_init+0x70/0x1c8 [ 0.000000] setup_arch+0x184/0x270 [ 0.000000] start_kernel+0x74/0x670 [ 0.000000] __primary_switched+0x80/0x90 [ 0.000000] Code: f865d804 d37df060 cb030000 d2800003 (b95d4084) [ 0.000000] ---[ end trace 0000000000000000 ]--- [ 0.000000] Kernel panic - not syncing: Attempted to kill the idle task! [ 0.000000] ---[ end Kernel panic - not syncing: Attempted to kill the idle task! ]---
while previous kernel versions were able to recognise how brain-damaged the machine is, and only build a fake node.
Restoring the check brings back some sanity and a "working" system.
Fixes: 767507654c22 ("arch_numa: switch over to numa_memblks") Signed-off-by: Marc Zyngier maz@kernel.org Cc: Mike Rapoport rppt@kernel.org Cc: Catalin Marinas catalin.marinas@arm.com Cc: Will Deacon will@kernel.org Cc: Zi Yan ziy@nvidia.com Cc: Dan Williams dan.j.williams@intel.com Cc: David Hildenbrand david@redhat.com Cc: Andrew Morton akpm@linux-foundation.org Cc: stable@vger.kernel.org --- drivers/base/arch_numa.c | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/drivers/base/arch_numa.c b/drivers/base/arch_numa.c index e187016764265..5457248eb0811 100644 --- a/drivers/base/arch_numa.c +++ b/drivers/base/arch_numa.c @@ -207,7 +207,21 @@ static void __init setup_node_data(int nid, u64 start_pfn, u64 end_pfn) static int __init numa_register_nodes(void) { int nid; - + struct memblock_region *mblk; + + /* Check that valid nid is set to memblks */ + for_each_mem_region(mblk) { + int mblk_nid = memblock_get_region_node(mblk); + phys_addr_t start = mblk->base; + phys_addr_t end = mblk->base + mblk->size - 1; + + if (mblk_nid == NUMA_NO_NODE || mblk_nid >= MAX_NUMNODES) { + pr_warn("Warning: invalid memblk node %d [mem %pap-%pap]\n", + mblk_nid, &start, &end); + return -EINVAL; + } + } + /* Finally register nodes. */ for_each_node_mask(nid, numa_nodes_parsed) { unsigned long start_pfn, end_pfn;
Hi Marc,
On Wed, Nov 27, 2024 at 07:30:00PM +0000, Marc Zyngier wrote:
Commit 767507654c22 ("arch_numa: switch over to numa_memblks") significantly cleaned up the NUMA registration code, but also dropped a significant check that was refusing to accept to configure a memblock with an invalid nid.
On "quality hardware" such as my ThunderX machine, this results in a kernel that dies immediately:
[ 0.000000] Booting Linux on physical CPU 0x0000000000 [0x431f0a10] [ 0.000000] Linux version 6.12.0-00013-g8920d74cf8db (maz@valley-girl) (gcc (Debian 12.2.0-14) 12.2.0, GNU ld (GNU Binutils for Debian) 2.40) #3872 SMP PREEMPT Wed Nov 27 15:25:49 GMT 2024 [ 0.000000] KASLR disabled due to lack of seed [ 0.000000] Machine model: Cavium ThunderX CN88XX board [ 0.000000] efi: EFI v2.4 by American Megatrends [ 0.000000] efi: ESRT=0xffce0ff18 SMBIOS 3.0=0xfffb0000 ACPI 2.0=0xffec60000 MEMRESERVE=0xffc905d98 [ 0.000000] esrt: Reserving ESRT space from 0x0000000ffce0ff18 to 0x0000000ffce0ff50. [ 0.000000] earlycon: pl11 at MMIO 0x000087e024000000 (options '115200n8') [ 0.000000] printk: legacy bootconsole [pl11] enabled [ 0.000000] NODE_DATA(0) allocated [mem 0xff6754580-0xff67566bf] [ 0.000000] Unable to handle kernel paging request at virtual address 0000000000001d40 [ 0.000000] Mem abort info: [ 0.000000] ESR = 0x0000000096000004 [ 0.000000] EC = 0x25: DABT (current EL), IL = 32 bits [ 0.000000] SET = 0, FnV = 0 [ 0.000000] EA = 0, S1PTW = 0 [ 0.000000] FSC = 0x04: level 0 translation fault [ 0.000000] Data abort info: [ 0.000000] ISV = 0, ISS = 0x00000004, ISS2 = 0x00000000 [ 0.000000] CM = 0, WnR = 0, TnD = 0, TagAccess = 0 [ 0.000000] GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0 [ 0.000000] [0000000000001d40] user address but active_mm is swapper [ 0.000000] Internal error: Oops: 0000000096000004 [#1] PREEMPT SMP [ 0.000000] Modules linked in: [ 0.000000] CPU: 0 UID: 0 PID: 0 Comm: swapper Not tainted 6.12.0-00013-g8920d74cf8db #3872 [ 0.000000] Hardware name: Cavium ThunderX CN88XX board (DT) [ 0.000000] pstate: a00000c5 (NzCv daIF -PAN -UAO -TCO -DIT -SSBS BTYPE=--) [ 0.000000] pc : sparse_init_nid+0x54/0x428 [ 0.000000] lr : sparse_init+0x118/0x240 [ 0.000000] sp : ffff800081da3cb0 [ 0.000000] x29: ffff800081da3cb0 x28: 0000000fedbab10c x27: 0000000000000001 [ 0.000000] x26: 0000000ffee250f8 x25: 0000000000000001 x24: ffff800082102cd0 [ 0.000000] x23: 0000000000000001 x22: 0000000000000000 x21: 00000000001fffff [ 0.000000] x20: 0000000000000001 x19: 0000000000000000 x18: ffffffffffffffff [ 0.000000] x17: 0000000001b00000 x16: 0000000ffd130000 x15: 0000000000000000 [ 0.000000] x14: 00000000003e0000 x13: 00000000000001c8 x12: 0000000000000014 [ 0.000000] x11: ffff800081e82860 x10: ffff8000820fb2c8 x9 : ffff8000820fb490 [ 0.000000] x8 : 0000000000ffed20 x7 : 0000000000000014 x6 : 00000000001fffff [ 0.000000] x5 : 00000000ffffffff x4 : 0000000000000000 x3 : 0000000000000000 [ 0.000000] x2 : 0000000000000000 x1 : 0000000000000040 x0 : 0000000000000007 [ 0.000000] Call trace: [ 0.000000] sparse_init_nid+0x54/0x428 [ 0.000000] sparse_init+0x118/0x240 [ 0.000000] bootmem_init+0x70/0x1c8 [ 0.000000] setup_arch+0x184/0x270 [ 0.000000] start_kernel+0x74/0x670 [ 0.000000] __primary_switched+0x80/0x90 [ 0.000000] Code: f865d804 d37df060 cb030000 d2800003 (b95d4084) [ 0.000000] ---[ end trace 0000000000000000 ]--- [ 0.000000] Kernel panic - not syncing: Attempted to kill the idle task! [ 0.000000] ---[ end Kernel panic - not syncing: Attempted to kill the idle task! ]---
while previous kernel versions were able to recognise how brain-damaged the machine is, and only build a fake node.
Restoring the check brings back some sanity and a "working" system.
Fixes: 767507654c22 ("arch_numa: switch over to numa_memblks") Signed-off-by: Marc Zyngier maz@kernel.org Cc: Mike Rapoport rppt@kernel.org Cc: Catalin Marinas catalin.marinas@arm.com Cc: Will Deacon will@kernel.org Cc: Zi Yan ziy@nvidia.com Cc: Dan Williams dan.j.williams@intel.com Cc: David Hildenbrand david@redhat.com Cc: Andrew Morton akpm@linux-foundation.org Cc: stable@vger.kernel.org
drivers/base/arch_numa.c | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/drivers/base/arch_numa.c b/drivers/base/arch_numa.c index e187016764265..5457248eb0811 100644 --- a/drivers/base/arch_numa.c +++ b/drivers/base/arch_numa.c @@ -207,7 +207,21 @@ static void __init setup_node_data(int nid, u64 start_pfn, u64 end_pfn) static int __init numa_register_nodes(void) { int nid;
- struct memblock_region *mblk;
- /* Check that valid nid is set to memblks */
- for_each_mem_region(mblk) {
int mblk_nid = memblock_get_region_node(mblk);phys_addr_t start = mblk->base;phys_addr_t end = mblk->base + mblk->size - 1;if (mblk_nid == NUMA_NO_NODE || mblk_nid >= MAX_NUMNODES) {pr_warn("Warning: invalid memblk node %d [mem %pap-%pap]\n",mblk_nid, &start, &end);return -EINVAL;}
We have memblock_validate_numa_coverage() that checks that amount of memory with unset node id is less than a threshold. The loop here can be replaced with something like
if (!memblock_validate_numa_coverage(0)) return -EINVAL;
- }
- /* Finally register nodes. */ for_each_node_mask(nid, numa_nodes_parsed) { unsigned long start_pfn, end_pfn;
-- 2.39.2
Hi Mike,
On Thu, 28 Nov 2024 07:03:33 +0000, Mike Rapoport rppt@kernel.org wrote:
Hi Marc,
diff --git a/drivers/base/arch_numa.c b/drivers/base/arch_numa.c index e187016764265..5457248eb0811 100644 --- a/drivers/base/arch_numa.c +++ b/drivers/base/arch_numa.c @@ -207,7 +207,21 @@ static void __init setup_node_data(int nid, u64 start_pfn, u64 end_pfn) static int __init numa_register_nodes(void) { int nid;
- struct memblock_region *mblk;
- /* Check that valid nid is set to memblks */
- for_each_mem_region(mblk) {
int mblk_nid = memblock_get_region_node(mblk);phys_addr_t start = mblk->base;phys_addr_t end = mblk->base + mblk->size - 1;if (mblk_nid == NUMA_NO_NODE || mblk_nid >= MAX_NUMNODES) {pr_warn("Warning: invalid memblk node %d [mem %pap-%pap]\n",mblk_nid, &start, &end);return -EINVAL;}We have memblock_validate_numa_coverage() that checks that amount of memory with unset node id is less than a threshold. The loop here can be replaced with something like
if (!memblock_validate_numa_coverage(0)) return -EINVAL;
Unfortunately, that doesn't seem to result in something that works (relevant extract only):
[ 0.000000] NUMA: no nodes coverage for 9MB of 65516MB RAM [ 0.000000] NUMA: Faking a node at [mem 0x0000000000500000-0x0000000fff0fffff] [ 0.000000] NUMA: no nodes coverage for 0MB of 65516MB RAM [ 0.000000] Unable to handle kernel paging request at virtual address 0000000000001d40
Any idea?
M.
On Thu, Nov 28, 2024 at 04:52:14PM +0000, Marc Zyngier wrote:
Hi Mike,
On Thu, 28 Nov 2024 07:03:33 +0000, Mike Rapoport rppt@kernel.org wrote:
Hi Marc,
diff --git a/drivers/base/arch_numa.c b/drivers/base/arch_numa.c index e187016764265..5457248eb0811 100644 --- a/drivers/base/arch_numa.c +++ b/drivers/base/arch_numa.c @@ -207,7 +207,21 @@ static void __init setup_node_data(int nid, u64 start_pfn, u64 end_pfn) static int __init numa_register_nodes(void) { int nid;
- struct memblock_region *mblk;
- /* Check that valid nid is set to memblks */
- for_each_mem_region(mblk) {
int mblk_nid = memblock_get_region_node(mblk);phys_addr_t start = mblk->base;phys_addr_t end = mblk->base + mblk->size - 1;if (mblk_nid == NUMA_NO_NODE || mblk_nid >= MAX_NUMNODES) {pr_warn("Warning: invalid memblk node %d [mem %pap-%pap]\n",mblk_nid, &start, &end);return -EINVAL;}We have memblock_validate_numa_coverage() that checks that amount of memory with unset node id is less than a threshold. The loop here can be replaced with something like
if (!memblock_validate_numa_coverage(0)) return -EINVAL;
Unfortunately, that doesn't seem to result in something that works (relevant extract only):
[ 0.000000] NUMA: no nodes coverage for 9MB of 65516MB RAM [ 0.000000] NUMA: Faking a node at [mem 0x0000000000500000-0x0000000fff0fffff] [ 0.000000] NUMA: no nodes coverage for 0MB of 65516MB RAM [ 0.000000] Unable to handle kernel paging request at virtual address 0000000000001d40
Any idea?
With 0 as the threshold the validation fails for the fake node, but it should be fine with memblock_validate_numa_coverage(1)
M.
-- Without deviation from the norm, progress is not possible.
On Fri, 29 Nov 2024 08:24:16 +0000, Mike Rapoport rppt@kernel.org wrote:
On Thu, Nov 28, 2024 at 04:52:14PM +0000, Marc Zyngier wrote:
Hi Mike,
On Thu, 28 Nov 2024 07:03:33 +0000, Mike Rapoport rppt@kernel.org wrote:
Hi Marc,
diff --git a/drivers/base/arch_numa.c b/drivers/base/arch_numa.c index e187016764265..5457248eb0811 100644 --- a/drivers/base/arch_numa.c +++ b/drivers/base/arch_numa.c @@ -207,7 +207,21 @@ static void __init setup_node_data(int nid, u64 start_pfn, u64 end_pfn) static int __init numa_register_nodes(void) { int nid;
- struct memblock_region *mblk;
- /* Check that valid nid is set to memblks */
- for_each_mem_region(mblk) {
int mblk_nid = memblock_get_region_node(mblk);phys_addr_t start = mblk->base;phys_addr_t end = mblk->base + mblk->size - 1;if (mblk_nid == NUMA_NO_NODE || mblk_nid >= MAX_NUMNODES) {pr_warn("Warning: invalid memblk node %d [mem %pap-%pap]\n",mblk_nid, &start, &end);return -EINVAL;}We have memblock_validate_numa_coverage() that checks that amount of memory with unset node id is less than a threshold. The loop here can be replaced with something like
if (!memblock_validate_numa_coverage(0)) return -EINVAL;
Unfortunately, that doesn't seem to result in something that works (relevant extract only):
[ 0.000000] NUMA: no nodes coverage for 9MB of 65516MB RAM [ 0.000000] NUMA: Faking a node at [mem 0x0000000000500000-0x0000000fff0fffff] [ 0.000000] NUMA: no nodes coverage for 0MB of 65516MB RAM [ 0.000000] Unable to handle kernel paging request at virtual address 0000000000001d40
Any idea?
With 0 as the threshold the validation fails for the fake node, but it should be fine with memblock_validate_numa_coverage(1)
Huh, subtle. This indeed seems to work. I'll respin the patch next week.
Thanks for your help,
M.
On Fri, Nov 29, 2024 at 08:42:55AM +0000, Marc Zyngier wrote:
On Fri, 29 Nov 2024 08:24:16 +0000, Mike Rapoport rppt@kernel.org wrote:
On Thu, Nov 28, 2024 at 04:52:14PM +0000, Marc Zyngier wrote:
Hi Mike,
On Thu, 28 Nov 2024 07:03:33 +0000, Mike Rapoport rppt@kernel.org wrote:
Hi Marc,
diff --git a/drivers/base/arch_numa.c b/drivers/base/arch_numa.c index e187016764265..5457248eb0811 100644 --- a/drivers/base/arch_numa.c +++ b/drivers/base/arch_numa.c @@ -207,7 +207,21 @@ static void __init setup_node_data(int nid, u64 start_pfn, u64 end_pfn) static int __init numa_register_nodes(void) { int nid;
- struct memblock_region *mblk;
- /* Check that valid nid is set to memblks */
- for_each_mem_region(mblk) {
int mblk_nid = memblock_get_region_node(mblk);phys_addr_t start = mblk->base;phys_addr_t end = mblk->base + mblk->size - 1;if (mblk_nid == NUMA_NO_NODE || mblk_nid >= MAX_NUMNODES) {pr_warn("Warning: invalid memblk node %d [mem %pap-%pap]\n",mblk_nid, &start, &end);return -EINVAL;}We have memblock_validate_numa_coverage() that checks that amount of memory with unset node id is less than a threshold. The loop here can be replaced with something like
if (!memblock_validate_numa_coverage(0)) return -EINVAL;
Unfortunately, that doesn't seem to result in something that works (relevant extract only):
[ 0.000000] NUMA: no nodes coverage for 9MB of 65516MB RAM [ 0.000000] NUMA: Faking a node at [mem 0x0000000000500000-0x0000000fff0fffff] [ 0.000000] NUMA: no nodes coverage for 0MB of 65516MB RAM [ 0.000000] Unable to handle kernel paging request at virtual address 0000000000001d40
Any idea?
With 0 as the threshold the validation fails for the fake node, but it should be fine with memblock_validate_numa_coverage(1)
Huh, subtle. This indeed seems to work. I'll respin the patch next week.
With the patch below memblock_validate_numa_coverage(0) should also work and it makes more sense.
@Andrew, I can take both this and Marc's new patch via memblock tree if you prefer.
From de55af44c02bc9aa43f05a785ac66a5aafa43354 Mon Sep 17 00:00:00 2001 From: "Mike Rapoport (Microsoft)" rppt@kernel.org Date: Fri, 29 Nov 2024 11:13:47 +0200 Subject: [PATCH] memblock: allow zero threshold in validate_numa_converage()
Currently memblock validate_numa_converage() returns false negative when threshold set to zero.
Make the check if the memory size with invalid node ID is greater than the threshold exclusive to fix that.
Signed-off-by: Mike Rapoport (Microsoft) rppt@kernel.org --- mm/memblock.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/mm/memblock.c b/mm/memblock.c index 0389ce5cd281..095c18b5c430 100644 --- a/mm/memblock.c +++ b/mm/memblock.c @@ -735,7 +735,7 @@ int __init_memblock memblock_add(phys_addr_t base, phys_addr_t size) /** * memblock_validate_numa_coverage - check if amount of memory with * no node ID assigned is less than a threshold - * @threshold_bytes: maximal number of pages that can have unassigned node + * @threshold_bytes: maximal memory size that can have unassigned node * ID (in bytes). * * A buggy firmware may report memory that does not belong to any node. @@ -755,7 +755,7 @@ bool __init_memblock memblock_validate_numa_coverage(unsigned long threshold_byt nr_pages += end_pfn - start_pfn; }
- if ((nr_pages << PAGE_SHIFT) >= threshold_bytes) { + if ((nr_pages << PAGE_SHIFT) > threshold_bytes) { mem_size_mb = memblock_phys_mem_size() >> 20; pr_err("NUMA: no nodes coverage for %luMB of %luMB RAM\n", (nr_pages << PAGE_SHIFT) >> 20, mem_size_mb);
On Fri, 29 Nov 2024 11:23:24 +0200 Mike Rapoport rppt@kernel.org wrote:
@Andrew, I can take both this and Marc's new patch via memblock tree if you prefer.
Go for it.
linux-stable-mirror@lists.linaro.org
-
Andrew Morton -
Marc Zyngier -
Mike Rapoport