* [PATCH] arch_numa: Restore nid checks before registering a memblock with a node
@ 2024-11-27 19:30 Marc Zyngier
2024-11-28 7:03 ` Mike Rapoport
0 siblings, 1 reply; 7+ messages in thread
From: Marc Zyngier @ 2024-11-27 19:30 UTC (permalink / raw)
To: linux-kernel, linux-arm-kernel
Cc: Mike Rapoport, Catalin Marinas, Will Deacon, Zi Yan, Dan Williams,
David Hildenbrand, Andrew Morton, stable
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;
--
2.39.2
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] arch_numa: Restore nid checks before registering a memblock with a node
2024-11-27 19:30 [PATCH] arch_numa: Restore nid checks before registering a memblock with a node Marc Zyngier
@ 2024-11-28 7:03 ` Mike Rapoport
2024-11-28 16:52 ` Marc Zyngier
0 siblings, 1 reply; 7+ messages in thread
From: Mike Rapoport @ 2024-11-28 7:03 UTC (permalink / raw)
To: Marc Zyngier
Cc: linux-kernel, linux-arm-kernel, Catalin Marinas, Will Deacon,
Zi Yan, Dan Williams, David Hildenbrand, Andrew Morton, stable
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
>
--
Sincerely yours,
Mike.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] arch_numa: Restore nid checks before registering a memblock with a node
2024-11-28 7:03 ` Mike Rapoport
@ 2024-11-28 16:52 ` Marc Zyngier
2024-11-29 8:24 ` Mike Rapoport
0 siblings, 1 reply; 7+ messages in thread
From: Marc Zyngier @ 2024-11-28 16:52 UTC (permalink / raw)
To: Mike Rapoport
Cc: linux-kernel, linux-arm-kernel, Catalin Marinas, Will Deacon,
Zi Yan, Dan Williams, David Hildenbrand, Andrew Morton, stable
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.
--
Without deviation from the norm, progress is not possible.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] arch_numa: Restore nid checks before registering a memblock with a node
2024-11-28 16:52 ` Marc Zyngier
@ 2024-11-29 8:24 ` Mike Rapoport
2024-11-29 8:42 ` Marc Zyngier
0 siblings, 1 reply; 7+ messages in thread
From: Mike Rapoport @ 2024-11-29 8:24 UTC (permalink / raw)
To: Marc Zyngier
Cc: linux-kernel, linux-arm-kernel, Catalin Marinas, Will Deacon,
Zi Yan, Dan Williams, David Hildenbrand, Andrew Morton, stable
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.
--
Sincerely yours,
Mike.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] arch_numa: Restore nid checks before registering a memblock with a node
2024-11-29 8:24 ` Mike Rapoport
@ 2024-11-29 8:42 ` Marc Zyngier
2024-11-29 9:23 ` Mike Rapoport
0 siblings, 1 reply; 7+ messages in thread
From: Marc Zyngier @ 2024-11-29 8:42 UTC (permalink / raw)
To: Mike Rapoport
Cc: linux-kernel, linux-arm-kernel, Catalin Marinas, Will Deacon,
Zi Yan, Dan Williams, David Hildenbrand, Andrew Morton, stable
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.
--
Without deviation from the norm, progress is not possible.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] arch_numa: Restore nid checks before registering a memblock with a node
2024-11-29 8:42 ` Marc Zyngier
@ 2024-11-29 9:23 ` Mike Rapoport
2024-11-29 10:41 ` Andrew Morton
0 siblings, 1 reply; 7+ messages in thread
From: Mike Rapoport @ 2024-11-29 9:23 UTC (permalink / raw)
To: Marc Zyngier
Cc: linux-kernel, linux-arm-kernel, Catalin Marinas, Will Deacon,
Zi Yan, Dan Williams, David Hildenbrand, Andrew Morton, stable
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);
--
2.45.2
> Thanks for your help,
>
> M.
--
Sincerely yours,
Mike.
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] arch_numa: Restore nid checks before registering a memblock with a node
2024-11-29 9:23 ` Mike Rapoport
@ 2024-11-29 10:41 ` Andrew Morton
0 siblings, 0 replies; 7+ messages in thread
From: Andrew Morton @ 2024-11-29 10:41 UTC (permalink / raw)
To: Mike Rapoport
Cc: Marc Zyngier, linux-kernel, linux-arm-kernel, Catalin Marinas,
Will Deacon, Zi Yan, Dan Williams, David Hildenbrand, stable
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.
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2024-11-29 10:42 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-11-27 19:30 [PATCH] arch_numa: Restore nid checks before registering a memblock with a node Marc Zyngier
2024-11-28 7:03 ` Mike Rapoport
2024-11-28 16:52 ` Marc Zyngier
2024-11-29 8:24 ` Mike Rapoport
2024-11-29 8:42 ` Marc Zyngier
2024-11-29 9:23 ` Mike Rapoport
2024-11-29 10:41 ` Andrew Morton
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).