arm64/numa: fix pcpu_cpu_distance() to get correct CPU proximity

PATCH Superseded Hanjun Guo 4 participants 11 messages Oct 20, 2016 arm64

lore

Share & tools

lore

Hanjun Guo

author

2016-10-20 03:52 UTC #0 lore raw

PATCH 1/2 arm64/numa: fix pcpu_cpu_distance() to get correct CPU proximity

From: Yisheng Xie <[email protected]>

The pcpu_build_alloc_info() function group CPUs according to their proximity, by call callback function @cpu_distance_fn from different ARCHs.

For arm64 the callback of @cpu_distance_fn is

    pcpu_cpu_distance(from, to)
        -> node_distance(from, to)

The @from and @to for function node_distance() should be nid.

However, pcpu_cpu_distance() in arch/arm64/mm/numa.c just past the cpu id for @from and @to.

For this incorrect cpu proximity get from ARCH, it may cause each CPU in one group and make group_cnt out of bound:

setup_per_cpu_areas()
pcpu_embed_first_chunk()
pcpu_build_alloc_info()
in pcpu_build_alloc_info, since cpu_distance_fn will return REMOTE_DISTANCE if we pass cpu ids (0,1,2...), so cpu_distance_fn(cpu, tcpu) > LOCAL_DISTANCE will wrongly be ture.

This may results in triggering the BUG_ON(unit != nr_units) later:

[    0.000000] kernel BUG at mm/percpu.c:1916!
[    0.000000] Internal error: Oops - BUG: 0 [#1] PREEMPT SMP
[    0.000000] Modules linked in:
[    0.000000] CPU: 0 PID: 0 Comm: swapper Not tainted 4.9.0-rc1-00003-g14155ca-dirty #26
[    0.000000] Hardware name: Hisilicon Hi1616 Evaluation Board (DT)
[    0.000000] task: ffff000008d6e900 task.stack: ffff000008d60000
[    0.000000] PC is at pcpu_embed_first_chunk+0x420/0x704
[    0.000000] LR is at pcpu_embed_first_chunk+0x3bc/0x704
[    0.000000] pc : [<ffff000008c754f4>] lr : [<ffff000008c75490>] pstate: 800000c5
[    0.000000] sp : ffff000008d63eb0
[    0.000000] x29: ffff000008d63eb0 [    0.000000] x28: 0000000000000000
[    0.000000] x27: 0000000000000040 [    0.000000] x26: ffff8413fbfcef00
[    0.000000] x25: 0000000000000042 [    0.000000] x24: 0000000000000042
[    0.000000] x23: 0000000000001000 [    0.000000] x22: 0000000000000046
[    0.000000] x21: 0000000000000001 [    0.000000] x20: ffff000008cb3bc8
[    0.000000] x19: ffff8413fbfcf570 [    0.000000] x18: 0000000000000000
[    0.000000] x17: ffff000008e49ae0 [    0.000000] x16: 0000000000000003
[    0.000000] x15: 000000000000001e [    0.000000] x14: 0000000000000004
[    0.000000] x13: 0000000000000000 [    0.000000] x12: 000000000000006f
[    0.000000] x11: 00000413fbffff00 [    0.000000] x10: 0000000000000004
[    0.000000] x9 : 0000000000000000 [    0.000000] x8 : 0000000000000001
[    0.000000] x7 : ffff8413fbfcf63c [    0.000000] x6 : ffff000008d65d28
[    0.000000] x5 : ffff000008d65e50 [    0.000000] x4 : 0000000000000000
[    0.000000] x3 : ffff000008cb3cc8 [    0.000000] x2 : 0000000000000040
[    0.000000] x1 : 0000000000000040 [    0.000000] x0 : 0000000000000000
[...]
[    0.000000] Call trace:
[    0.000000] Exception stack(0xffff000008d63ce0 to 0xffff000008d63e10)
[    0.000000] 3ce0: ffff8413fbfcf570 0001000000000000 ffff000008d63eb0 ffff000008c754f4
[    0.000000] 3d00: ffff000008d63d50 ffff0000081af210 00000413fbfff010 0000000000001000
[    0.000000] 3d20: ffff000008d63d50 ffff0000081af220 00000413fbfff010 0000000000001000
[    0.000000] 3d40: 00000413fbfcef00 0000000000000004 ffff000008d63db0 ffff0000081af390
[    0.000000] 3d60: 00000413fbfcef00 0000000000001000 0000000000000000 0000000000001000
[    0.000000] 3d80: 0000000000000000 0000000000000040 0000000000000040 ffff000008cb3cc8
[    0.000000] 3da0: 0000000000000000 ffff000008d65e50 ffff000008d65d28 ffff8413fbfcf63c
[    0.000000] 3dc0: 0000000000000001 0000000000000000 0000000000000004 00000413fbffff00
[    0.000000] 3de0: 000000000000006f 0000000000000000 0000000000000004 000000000000001e
[    0.000000] 3e00: 0000000000000003 ffff000008e49ae0
[    0.000000] [<ffff000008c754f4>] pcpu_embed_first_chunk+0x420/0x704
[    0.000000] [<ffff000008c6658c>] setup_per_cpu_areas+0x38/0xc8
[    0.000000] [<ffff000008c608d8>] start_kernel+0x10c/0x390
[    0.000000] [<ffff000008c601d8>] __primary_switched+0x5c/0x64
[    0.000000] Code: b8018660 17ffffd7 6b16037f 54000080 (d4210000)
[    0.000000] ---[ end trace 0000000000000000 ]---
[    0.000000] Kernel panic - not syncing: Attempted to kill the idle task!

Fix by getting CPUs proximity through its node. We only care about whether it is LOCAL_DISTANCE or not, for pcpu_build_alloc_info() only use this to group CPUs.

Fixes 7af3a0a99252 ("arm64/numa: support HAVE_SETUP_PER_CPU_AREA")

Signed-off-by Yisheng Xie <[email protected]>

Signed-off-by Hanjun Guo <[email protected]>

Cc Catalin Marinas <[email protected]>

Cc Lorenzo Pieralisi <[email protected]>

Cc Will Deacon <[email protected]>

Cc Zhen Lei <[email protected]>

---
 arch/arm64/mm/numa.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

arch/arm64/mm/numa.c +4 -1

index 778a985..34415fc 100644
@@ -147,7 +147,10 @@ static int __init early_cpu_to_node(int cpu)
 
 static int __init pcpu_cpu_distance(unsigned int from, unsigned int to)
 {
-	return node_distance(from, to);
+	if (early_cpu_to_node(from) == early_cpu_to_node(to))
+		return LOCAL_DISTANCE;
+	else
+		return REMOTE_DISTANCE;
 }
 
 static void * __init pcpu_fc_alloc(unsigned int cpu, size_t size,

Signature · 1 line

1.7.12.4

Hanjun Guo

author

2016-10-20 04:03 UTC #1 lore raw

PATCH 1/2 arm64/numa: fix pcpu_cpu_distance() to get correct CPU proximity

Quoted 93 lines · Hanjun Guo

From: Yisheng Xie <[email protected]>

The pcpu_build_alloc_info() function group CPUs according to their proximity, by call callback function @cpu_distance_fn from different ARCHs.

For arm64 the callback of @cpu_distance_fn is

    pcpu_cpu_distance(from, to)
        -> node_distance(from, to)

The @from and @to for function node_distance() should be nid.

However, pcpu_cpu_distance() in arch/arm64/mm/numa.c just past the cpu id for @from and @to.

For this incorrect cpu proximity get from ARCH, it may cause each CPU in one group and make group_cnt out of bound:

This may results in triggering the BUG_ON(unit != nr_units) later:

[    0.000000] kernel BUG at mm/percpu.c:1916!
[    0.000000] Internal error: Oops - BUG: 0 [#1] PREEMPT SMP
[    0.000000] Modules linked in:
[    0.000000] CPU: 0 PID: 0 Comm: swapper Not tainted 4.9.0-rc1-00003-g14155ca-dirty #26
[    0.000000] Hardware name: Hisilicon Hi1616 Evaluation Board (DT)
[    0.000000] task: ffff000008d6e900 task.stack: ffff000008d60000
[    0.000000] PC is at pcpu_embed_first_chunk+0x420/0x704
[    0.000000] LR is at pcpu_embed_first_chunk+0x3bc/0x704
[    0.000000] pc : [<ffff000008c754f4>] lr : [<ffff000008c75490>] pstate: 800000c5
[    0.000000] sp : ffff000008d63eb0
[    0.000000] x29: ffff000008d63eb0 [    0.000000] x28: 0000000000000000
[    0.000000] x27: 0000000000000040 [    0.000000] x26: ffff8413fbfcef00
[    0.000000] x25: 0000000000000042 [    0.000000] x24: 0000000000000042
[    0.000000] x23: 0000000000001000 [    0.000000] x22: 0000000000000046
[    0.000000] x21: 0000000000000001 [    0.000000] x20: ffff000008cb3bc8
[    0.000000] x19: ffff8413fbfcf570 [    0.000000] x18: 0000000000000000
[    0.000000] x17: ffff000008e49ae0 [    0.000000] x16: 0000000000000003
[    0.000000] x15: 000000000000001e [    0.000000] x14: 0000000000000004
[    0.000000] x13: 0000000000000000 [    0.000000] x12: 000000000000006f
[    0.000000] x11: 00000413fbffff00 [    0.000000] x10: 0000000000000004
[    0.000000] x9 : 0000000000000000 [    0.000000] x8 : 0000000000000001
[    0.000000] x7 : ffff8413fbfcf63c [    0.000000] x6 : ffff000008d65d28
[    0.000000] x5 : ffff000008d65e50 [    0.000000] x4 : 0000000000000000
[    0.000000] x3 : ffff000008cb3cc8 [    0.000000] x2 : 0000000000000040
[    0.000000] x1 : 0000000000000040 [    0.000000] x0 : 0000000000000000
[...]
[    0.000000] Call trace:
[    0.000000] Exception stack(0xffff000008d63ce0 to 0xffff000008d63e10)
[    0.000000] 3ce0: ffff8413fbfcf570 0001000000000000 ffff000008d63eb0 ffff000008c754f4
[    0.000000] 3d00: ffff000008d63d50 ffff0000081af210 00000413fbfff010 0000000000001000
[    0.000000] 3d20: ffff000008d63d50 ffff0000081af220 00000413fbfff010 0000000000001000
[    0.000000] 3d40: 00000413fbfcef00 0000000000000004 ffff000008d63db0 ffff0000081af390
[    0.000000] 3d60: 00000413fbfcef00 0000000000001000 0000000000000000 0000000000001000
[    0.000000] 3d80: 0000000000000000 0000000000000040 0000000000000040 ffff000008cb3cc8
[    0.000000] 3da0: 0000000000000000 ffff000008d65e50 ffff000008d65d28 ffff8413fbfcf63c
[    0.000000] 3dc0: 0000000000000001 0000000000000000 0000000000000004 00000413fbffff00
[    0.000000] 3de0: 000000000000006f 0000000000000000 0000000000000004 000000000000001e
[    0.000000] 3e00: 0000000000000003 ffff000008e49ae0
[    0.000000] [<ffff000008c754f4>] pcpu_embed_first_chunk+0x420/0x704
[    0.000000] [<ffff000008c6658c>] setup_per_cpu_areas+0x38/0xc8
[    0.000000] [<ffff000008c608d8>] start_kernel+0x10c/0x390
[    0.000000] [<ffff000008c601d8>] __primary_switched+0x5c/0x64
[    0.000000] Code: b8018660 17ffffd7 6b16037f 54000080 (d4210000)
[    0.000000] ---[ end trace 0000000000000000 ]---
[    0.000000] Kernel panic - not syncing: Attempted to kill the idle task!

Fix by getting CPUs proximity through its node. We only care about whether it is LOCAL_DISTANCE or not, for pcpu_build_alloc_info() only use this to group CPUs.

Fixes 7af3a0a99252 ("arm64/numa: support HAVE_SETUP_PER_CPU_AREA")

Signed-off-by Yisheng Xie <[email protected]>

Signed-off-by Hanjun Guo <[email protected]>

Cc Catalin Marinas <[email protected]>

Cc Lorenzo Pieralisi <[email protected]>

Cc Will Deacon <[email protected]>

Cc Zhen Lei <[email protected]>

---
 arch/arm64/mm/numa.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

arch/arm64/mm/numa.c +0 -0

index 778a985..34415fc 100644
@@ -147,7 +147,10 @@ static int __init early_cpu_to_node(int cpu)

arch/arm64/mm/numa.c static int __init early_cpu_to_node(int cpu) · line 156

Quoted 10 lines

arch/arm64/mm/numa.c +4 -1

 static int __init pcpu_cpu_distance(unsigned int from, unsigned int to)
 {
-	return node_distance(from, to);
+	if (early_cpu_to_node(from) == early_cpu_to_node(to))
+		return LOCAL_DISTANCE;
+	else
+		return REMOTE_DISTANCE;
 }
 
 static void * __init pcpu_fc_alloc(unsigned int cpu, size_t size,

We trigger this bug with NR_CPUS=64 in the config instead of 4096 in defconfig for arm64, and we have 64 cpus in the system. I think that's why this bug wasn't triggered yet on other ARM64 NUMA platforms, this bug should generic for all ARM64 platforms.

Thanks
Hanjun

Hanjun Guo

author

2016-10-20 03:52 UTC #2 lore raw

PATCH 2/2 arm64/numa: fix incorrect print of end_pfn

In reply to Hanjun Guo

From: Hanjun Guo <[email protected]>

When booting on NUMA system with memory-less node (no memory dimm on this memory controller), the print for setup_node_data() is incorrect:

NUMA: Initmem setup node 2 [mem 0x00000000-0xffffffffffffffff]

It should be 0, not 0xffffffffffffffff as there is no memory on that node.

Fixes 1a2db300348b ("arm64, numa: Add NUMA support for arm64 platforms.")

Signed-off-by Hanjun Guo <[email protected]>

Cc Catalin Marinas <[email protected]>

Cc Ganapatrao Kulkarni <[email protected]>

Cc Lorenzo Pieralisi <[email protected]>

Cc Will Deacon <[email protected]>

---
 arch/arm64/mm/numa.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

arch/arm64/mm/numa.c +2 -1

index 34415fc..148e1fc 100644
@@ -227,7 +227,8 @@ static void __init setup_node_data(int nid, u64 start_pfn, u64 end_pfn)
 	int tnid;
 
 	pr_info("Initmem setup node %d [mem %#010Lx-%#010Lx]\n",
-		nid, start_pfn << PAGE_SHIFT, (end_pfn << PAGE_SHIFT) - 1);
+		nid, start_pfn << PAGE_SHIFT,
+		end_pfn ? (end_pfn << PAGE_SHIFT) - 1 : 0);
 
 	nd_pa = memblock_alloc_try_nid(nd_size, SMP_CACHE_BYTES, nid);
 	nd = __va(nd_pa);

Signature · 1 line

1.7.12.4

Will Deacon

2016-10-20 10:51 UTC #3 lore raw

PATCH 2/2 arm64/numa: fix incorrect print of end_pfn

Quoted 10 lines · Hanjun Guo

From: Hanjun Guo <[email protected]>
When booting on NUMA system with memory-less node (no memory dimm on this memory controller), the print for setup_node_data() is incorrect:
NUMA: Initmem setup node 2 [mem 0x00000000-0xffffffffffffffff]
It should be 0, not 0xffffffffffffffff as there is no memory on that node.

Wouldn't it make more sense to print something useful, like "memory-less node"?

Will

Hanjun Guo

author

2016-10-20 12:21 UTC #4 lore raw

PATCH 2/2 arm64/numa: fix incorrect print of end_pfn

Quoted 7 lines · Will Deacon

Quoted 6 lines · Hanjun Guo
From: Hanjun Guo <[email protected]>
When booting on NUMA system with memory-less node (no memory dimm on this memory controller), the print for setup_node_data() is incorrect:

Quoted 6 lines

Quoted 4 lines
NUMA: Initmem setup node 2 [mem 0x00000000-0xffffffffffffffff]
It should be 0, not 0xffffffffffffffff as there is no memory on that node.
Wouldn't it make more sense to print something useful, like "memory-less node"?

in the log,

[    0.000000] NUMA: Initmem setup node 0 [mem 0x00000000-0x13fbffffff]
[    0.000000] NUMA: NODE_DATA [mem 0x13fbffe500-0x13fbffffff]
[    0.000000] NUMA: Initmem setup node 1 [mem 0x1400000000-0x17fbffffff]
[    0.000000] NUMA: NODE_DATA [mem 0x17fbfec500-0x17fbfedfff]
[    0.000000] NUMA: Initmem setup node 2 [mem 0x00000000-0xffffffffffffffff]
[    0.000000] NUMA: NODE_DATA [mem 0x17fbfeaa00-0x17fbfec4ff]
[    0.000000] NUMA: NODE_DATA(2) on node 1
[    0.000000] NUMA: Initmem setup node 3 [mem 0x00000000-0xffffffffffffffff]
[    0.000000] NUMA: NODE_DATA [mem 0x17fbfe8f00-0x17fbfea9ff]
[    0.000000] NUMA: NODE_DATA(3) on node 1

if printing "NUMA: Initmem setup node 2 [mem 0x00000000-0x00000000]", it will make the log consistent with others, and obvious it's a memory-less node as memory range 0x00000000-0x00000000, what do you think?

Thanks
Hanjun

Will Deacon

2016-10-20 12:52 UTC #5 lore raw

PATCH 2/2 arm64/numa: fix incorrect print of end_pfn

Quoted 28 lines · Hanjun Guo

Quoted 7 lines · Will Deacon
Quoted 6 lines · Hanjun Guo
From: Hanjun Guo <[email protected]>
When booting on NUMA system with memory-less node (no memory dimm on this memory controller), the print for setup_node_data() is incorrect:
Quoted 6 lines
Quoted 4 lines
NUMA: Initmem setup node 2 [mem 0x00000000-0xffffffffffffffff]
It should be 0, not 0xffffffffffffffff as there is no memory on that node.
Wouldn't it make more sense to print something useful, like "memory-less node"?
in the log,
[    0.000000] NUMA: Initmem setup node 0 [mem 0x00000000-0x13fbffffff]
[    0.000000] NUMA: NODE_DATA [mem 0x13fbffe500-0x13fbffffff]
[    0.000000] NUMA: Initmem setup node 1 [mem 0x1400000000-0x17fbffffff]
[    0.000000] NUMA: NODE_DATA [mem 0x17fbfec500-0x17fbfedfff]
[    0.000000] NUMA: Initmem setup node 2 [mem 0x00000000-0xffffffffffffffff]
[    0.000000] NUMA: NODE_DATA [mem 0x17fbfeaa00-0x17fbfec4ff]
[    0.000000] NUMA: NODE_DATA(2) on node 1
[    0.000000] NUMA: Initmem setup node 3 [mem 0x00000000-0xffffffffffffffff]
[    0.000000] NUMA: NODE_DATA [mem 0x17fbfe8f00-0x17fbfea9ff]
[    0.000000] NUMA: NODE_DATA(3) on node 1

Quoted 3 lines

if printing "NUMA: Initmem setup node 2 [mem 0x00000000-0x00000000]", it will make the log consistent with others, and obvious it's a memory-less node as memory range 0x00000000-0x00000000, what do you think?

How is that more obvious than printing the string "memory-less node"? Is this data parsed by something?

Will

Mark Rutland

reviewer

2016-10-20 12:55 UTC #6 lore raw

PATCH 2/2 arm64/numa: fix incorrect print of end_pfn

In reply to Hanjun Guo

Quoted 28 lines · Hanjun Guo

Quoted 7 lines · Will Deacon
Quoted 6 lines · Hanjun Guo
From: Hanjun Guo <[email protected]>
When booting on NUMA system with memory-less node (no memory dimm on this memory controller), the print for setup_node_data() is incorrect:
Quoted 6 lines
Quoted 4 lines
NUMA: Initmem setup node 2 [mem 0x00000000-0xffffffffffffffff]
It should be 0, not 0xffffffffffffffff as there is no memory on that node.
Wouldn't it make more sense to print something useful, like "memory-less node"?
in the log,
[    0.000000] NUMA: Initmem setup node 0 [mem 0x00000000-0x13fbffffff]
[    0.000000] NUMA: NODE_DATA [mem 0x13fbffe500-0x13fbffffff]
[    0.000000] NUMA: Initmem setup node 1 [mem 0x1400000000-0x17fbffffff]
[    0.000000] NUMA: NODE_DATA [mem 0x17fbfec500-0x17fbfedfff]
[    0.000000] NUMA: Initmem setup node 2 [mem 0x00000000-0xffffffffffffffff]
[    0.000000] NUMA: NODE_DATA [mem 0x17fbfeaa00-0x17fbfec4ff]
[    0.000000] NUMA: NODE_DATA(2) on node 1
[    0.000000] NUMA: Initmem setup node 3 [mem 0x00000000-0xffffffffffffffff]
[    0.000000] NUMA: NODE_DATA [mem 0x17fbfe8f00-0x17fbfea9ff]
[    0.000000] NUMA: NODE_DATA(3) on node 1

Quoted 1 line

if printing "NUMA: Initmem setup node 2 [mem 0x00000000-0x00000000]",

Seeing "NUMA: Initmem setup node 2 [<memoryless node>]" would be far more obvious as a memoryless node, and I don't see that this would be inconsistent.

Thanks,
Mark.

Hanjun Guo

author

2016-10-20 13:26 UTC #7 lore raw

PATCH 2/2 arm64/numa: fix incorrect print of end_pfn

Quoted 28 lines · Mark Rutland

Quoted 27 lines · Hanjun Guo
Quoted 7 lines · Will Deacon
Quoted 6 lines · Hanjun Guo
From: Hanjun Guo <[email protected]>
When booting on NUMA system with memory-less node (no memory dimm on this memory controller), the print for setup_node_data() is incorrect:
Quoted 6 lines
Quoted 4 lines
NUMA: Initmem setup node 2 [mem 0x00000000-0xffffffffffffffff]
It should be 0, not 0xffffffffffffffff as there is no memory on that node.
Wouldn't it make more sense to print something useful, like "memory-less node"?
in the log,
[    0.000000] NUMA: Initmem setup node 0 [mem 0x00000000-0x13fbffffff]
[    0.000000] NUMA: NODE_DATA [mem 0x13fbffe500-0x13fbffffff]
[    0.000000] NUMA: Initmem setup node 1 [mem 0x1400000000-0x17fbffffff]
[    0.000000] NUMA: NODE_DATA [mem 0x17fbfec500-0x17fbfedfff]
[    0.000000] NUMA: Initmem setup node 2 [mem 0x00000000-0xffffffffffffffff]
[    0.000000] NUMA: NODE_DATA [mem 0x17fbfeaa00-0x17fbfec4ff]
[    0.000000] NUMA: NODE_DATA(2) on node 1
[    0.000000] NUMA: Initmem setup node 3 [mem 0x00000000-0xffffffffffffffff]
[    0.000000] NUMA: NODE_DATA [mem 0x17fbfe8f00-0x17fbfea9ff]
[    0.000000] NUMA: NODE_DATA(3) on node 1

Quoted 4 lines

Quoted 1 line
if printing "NUMA: Initmem setup node 2 [mem 0x00000000-0x00000000]",
Seeing "NUMA: Initmem setup node 2 [<memoryless node>]" would be far more obvious as a memoryless node, and I don't see that this would be inconsistent.

I misunderstood Will's intention, I though printing "NUMA: memory-less node 2". so do you mean code like below?

arch/arm64/mm/numa.c +5 -2

index 34415fc..bf4e39b 100644
@@ -226,8 +226,11 @@ static void __init setup_node_data(int nid, u64 start_pfn, u64 end_pfn)
        void *nd;
        int tnid;
 
-   pr_info("Initmem setup node %d [mem %#010Lx-%#010Lx]\n",
-           nid, start_pfn << PAGE_SHIFT, (end_pfn << PAGE_SHIFT) - 1);
+ if ((end_pfn - start_pfn) != 0)
+         pr_info("Initmem setup node %d [mem %#010Lx-%#010Lx]\n", nid,
+                 start_pfn << PAGE_SHIFT, (end_pfn << PAGE_SHIFT) - 1);
+ else
+         pr_info("Initmem setup node %d [<memory-less node>]\n", nid);
 
        nd_pa = memblock_alloc_try_nid(nd_size, SMP_CACHE_BYTES, nid);
        nd = __va(nd_pa);

Thanks
Hanjun

Leizhen (ThunderTown)

2016-10-20 06:39 UTC #8 lore raw

PATCH 1/2 arm64/numa: fix pcpu_cpu_distance() to get correct CPU proximity

In reply to Hanjun Guo

Quoted 92 lines · Hanjun Guo

From: Yisheng Xie <[email protected]>

The pcpu_build_alloc_info() function group CPUs according to their proximity, by call callback function @cpu_distance_fn from different ARCHs.

For arm64 the callback of @cpu_distance_fn is

    pcpu_cpu_distance(from, to)
        -> node_distance(from, to)

The @from and @to for function node_distance() should be nid.

However, pcpu_cpu_distance() in arch/arm64/mm/numa.c just past the cpu id for @from and @to.

For this incorrect cpu proximity get from ARCH, it may cause each CPU in one group and make group_cnt out of bound:

This may results in triggering the BUG_ON(unit != nr_units) later:

[    0.000000] kernel BUG at mm/percpu.c:1916!
[    0.000000] Internal error: Oops - BUG: 0 [#1] PREEMPT SMP
[    0.000000] Modules linked in:
[    0.000000] CPU: 0 PID: 0 Comm: swapper Not tainted 4.9.0-rc1-00003-g14155ca-dirty #26
[    0.000000] Hardware name: Hisilicon Hi1616 Evaluation Board (DT)
[    0.000000] task: ffff000008d6e900 task.stack: ffff000008d60000
[    0.000000] PC is at pcpu_embed_first_chunk+0x420/0x704
[    0.000000] LR is at pcpu_embed_first_chunk+0x3bc/0x704
[    0.000000] pc : [<ffff000008c754f4>] lr : [<ffff000008c75490>] pstate: 800000c5
[    0.000000] sp : ffff000008d63eb0
[    0.000000] x29: ffff000008d63eb0 [    0.000000] x28: 0000000000000000
[    0.000000] x27: 0000000000000040 [    0.000000] x26: ffff8413fbfcef00
[    0.000000] x25: 0000000000000042 [    0.000000] x24: 0000000000000042
[    0.000000] x23: 0000000000001000 [    0.000000] x22: 0000000000000046
[    0.000000] x21: 0000000000000001 [    0.000000] x20: ffff000008cb3bc8
[    0.000000] x19: ffff8413fbfcf570 [    0.000000] x18: 0000000000000000
[    0.000000] x17: ffff000008e49ae0 [    0.000000] x16: 0000000000000003
[    0.000000] x15: 000000000000001e [    0.000000] x14: 0000000000000004
[    0.000000] x13: 0000000000000000 [    0.000000] x12: 000000000000006f
[    0.000000] x11: 00000413fbffff00 [    0.000000] x10: 0000000000000004
[    0.000000] x9 : 0000000000000000 [    0.000000] x8 : 0000000000000001
[    0.000000] x7 : ffff8413fbfcf63c [    0.000000] x6 : ffff000008d65d28
[    0.000000] x5 : ffff000008d65e50 [    0.000000] x4 : 0000000000000000
[    0.000000] x3 : ffff000008cb3cc8 [    0.000000] x2 : 0000000000000040
[    0.000000] x1 : 0000000000000040 [    0.000000] x0 : 0000000000000000
[...]
[    0.000000] Call trace:
[    0.000000] Exception stack(0xffff000008d63ce0 to 0xffff000008d63e10)
[    0.000000] 3ce0: ffff8413fbfcf570 0001000000000000 ffff000008d63eb0 ffff000008c754f4
[    0.000000] 3d00: ffff000008d63d50 ffff0000081af210 00000413fbfff010 0000000000001000
[    0.000000] 3d20: ffff000008d63d50 ffff0000081af220 00000413fbfff010 0000000000001000
[    0.000000] 3d40: 00000413fbfcef00 0000000000000004 ffff000008d63db0 ffff0000081af390
[    0.000000] 3d60: 00000413fbfcef00 0000000000001000 0000000000000000 0000000000001000
[    0.000000] 3d80: 0000000000000000 0000000000000040 0000000000000040 ffff000008cb3cc8
[    0.000000] 3da0: 0000000000000000 ffff000008d65e50 ffff000008d65d28 ffff8413fbfcf63c
[    0.000000] 3dc0: 0000000000000001 0000000000000000 0000000000000004 00000413fbffff00
[    0.000000] 3de0: 000000000000006f 0000000000000000 0000000000000004 000000000000001e
[    0.000000] 3e00: 0000000000000003 ffff000008e49ae0
[    0.000000] [<ffff000008c754f4>] pcpu_embed_first_chunk+0x420/0x704
[    0.000000] [<ffff000008c6658c>] setup_per_cpu_areas+0x38/0xc8
[    0.000000] [<ffff000008c608d8>] start_kernel+0x10c/0x390
[    0.000000] [<ffff000008c601d8>] __primary_switched+0x5c/0x64
[    0.000000] Code: b8018660 17ffffd7 6b16037f 54000080 (d4210000)
[    0.000000] ---[ end trace 0000000000000000 ]---
[    0.000000] Kernel panic - not syncing: Attempted to kill the idle task!

Fix by getting CPUs proximity through its node. We only care about whether it is LOCAL_DISTANCE or not, for pcpu_build_alloc_info() only use this to group CPUs.

Fixes 7af3a0a99252 ("arm64/numa: support HAVE_SETUP_PER_CPU_AREA")

Signed-off-by Yisheng Xie <[email protected]>

Signed-off-by Hanjun Guo <[email protected]>

Cc Catalin Marinas <[email protected]>

Cc Lorenzo Pieralisi <[email protected]>

Cc Will Deacon <[email protected]>

Cc Zhen Lei <[email protected]>

---
 arch/arm64/mm/numa.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

arch/arm64/mm/numa.c +0 -0

index 778a985..34415fc 100644

arch/arm64/mm/numa.c static int __init early_cpu_to_node(int cpu) · line 154

Quoted 9 lines

arch/arm64/mm/numa.c +4 -1

@@ -147,7 +147,10 @@ static int __init early_cpu_to_node(int cpu)
 
 static int __init pcpu_cpu_distance(unsigned int from, unsigned int to)
 {
-	return node_distance(from, to);
+	if (early_cpu_to_node(from) == early_cpu_to_node(to))
+		return LOCAL_DISTANCE;
+	else
+		return REMOTE_DISTANCE;
 }

Reviewd-by Zhen Lei <[email protected]>

Quoted 3 lines

arch/arm64/mm/numa.c +0 -0

 static void * __init pcpu_fc_alloc(unsigned int cpu, size_t size,

Will Deacon

2016-10-20 10:48 UTC #9 lore raw

PATCH 1/2 arm64/numa: fix pcpu_cpu_distance() to get correct CPU proximity

In reply to Hanjun Guo

Quoted 92 lines · Hanjun Guo

From: Yisheng Xie <[email protected]>

The pcpu_build_alloc_info() function group CPUs according to their proximity, by call callback function @cpu_distance_fn from different ARCHs.

For arm64 the callback of @cpu_distance_fn is

    pcpu_cpu_distance(from, to)
        -> node_distance(from, to)

The @from and @to for function node_distance() should be nid.

However, pcpu_cpu_distance() in arch/arm64/mm/numa.c just past the cpu id for @from and @to.

For this incorrect cpu proximity get from ARCH, it may cause each CPU in one group and make group_cnt out of bound:

This may results in triggering the BUG_ON(unit != nr_units) later:

[    0.000000] kernel BUG at mm/percpu.c:1916!
[    0.000000] Internal error: Oops - BUG: 0 [#1] PREEMPT SMP
[    0.000000] Modules linked in:
[    0.000000] CPU: 0 PID: 0 Comm: swapper Not tainted 4.9.0-rc1-00003-g14155ca-dirty #26
[    0.000000] Hardware name: Hisilicon Hi1616 Evaluation Board (DT)
[    0.000000] task: ffff000008d6e900 task.stack: ffff000008d60000
[    0.000000] PC is at pcpu_embed_first_chunk+0x420/0x704
[    0.000000] LR is at pcpu_embed_first_chunk+0x3bc/0x704
[    0.000000] pc : [<ffff000008c754f4>] lr : [<ffff000008c75490>] pstate: 800000c5
[    0.000000] sp : ffff000008d63eb0
[    0.000000] x29: ffff000008d63eb0 [    0.000000] x28: 0000000000000000
[    0.000000] x27: 0000000000000040 [    0.000000] x26: ffff8413fbfcef00
[    0.000000] x25: 0000000000000042 [    0.000000] x24: 0000000000000042
[    0.000000] x23: 0000000000001000 [    0.000000] x22: 0000000000000046
[    0.000000] x21: 0000000000000001 [    0.000000] x20: ffff000008cb3bc8
[    0.000000] x19: ffff8413fbfcf570 [    0.000000] x18: 0000000000000000
[    0.000000] x17: ffff000008e49ae0 [    0.000000] x16: 0000000000000003
[    0.000000] x15: 000000000000001e [    0.000000] x14: 0000000000000004
[    0.000000] x13: 0000000000000000 [    0.000000] x12: 000000000000006f
[    0.000000] x11: 00000413fbffff00 [    0.000000] x10: 0000000000000004
[    0.000000] x9 : 0000000000000000 [    0.000000] x8 : 0000000000000001
[    0.000000] x7 : ffff8413fbfcf63c [    0.000000] x6 : ffff000008d65d28
[    0.000000] x5 : ffff000008d65e50 [    0.000000] x4 : 0000000000000000
[    0.000000] x3 : ffff000008cb3cc8 [    0.000000] x2 : 0000000000000040
[    0.000000] x1 : 0000000000000040 [    0.000000] x0 : 0000000000000000
[...]
[    0.000000] Call trace:
[    0.000000] Exception stack(0xffff000008d63ce0 to 0xffff000008d63e10)
[    0.000000] 3ce0: ffff8413fbfcf570 0001000000000000 ffff000008d63eb0 ffff000008c754f4
[    0.000000] 3d00: ffff000008d63d50 ffff0000081af210 00000413fbfff010 0000000000001000
[    0.000000] 3d20: ffff000008d63d50 ffff0000081af220 00000413fbfff010 0000000000001000
[    0.000000] 3d40: 00000413fbfcef00 0000000000000004 ffff000008d63db0 ffff0000081af390
[    0.000000] 3d60: 00000413fbfcef00 0000000000001000 0000000000000000 0000000000001000
[    0.000000] 3d80: 0000000000000000 0000000000000040 0000000000000040 ffff000008cb3cc8
[    0.000000] 3da0: 0000000000000000 ffff000008d65e50 ffff000008d65d28 ffff8413fbfcf63c
[    0.000000] 3dc0: 0000000000000001 0000000000000000 0000000000000004 00000413fbffff00
[    0.000000] 3de0: 000000000000006f 0000000000000000 0000000000000004 000000000000001e
[    0.000000] 3e00: 0000000000000003 ffff000008e49ae0
[    0.000000] [<ffff000008c754f4>] pcpu_embed_first_chunk+0x420/0x704
[    0.000000] [<ffff000008c6658c>] setup_per_cpu_areas+0x38/0xc8
[    0.000000] [<ffff000008c608d8>] start_kernel+0x10c/0x390
[    0.000000] [<ffff000008c601d8>] __primary_switched+0x5c/0x64
[    0.000000] Code: b8018660 17ffffd7 6b16037f 54000080 (d4210000)
[    0.000000] ---[ end trace 0000000000000000 ]---
[    0.000000] Kernel panic - not syncing: Attempted to kill the idle task!

Fix by getting CPUs proximity through its node. We only care about whether it is LOCAL_DISTANCE or not, for pcpu_build_alloc_info() only use this to group CPUs.

Fixes 7af3a0a99252 ("arm64/numa: support HAVE_SETUP_PER_CPU_AREA")

Signed-off-by Yisheng Xie <[email protected]>

Signed-off-by Hanjun Guo <[email protected]>

Cc Catalin Marinas <[email protected]>

Cc Lorenzo Pieralisi <[email protected]>

Cc Will Deacon <[email protected]>

Cc Zhen Lei <[email protected]>

---
 arch/arm64/mm/numa.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

arch/arm64/mm/numa.c +0 -0

index 778a985..34415fc 100644

arch/arm64/mm/numa.c static int __init early_cpu_to_node(int cpu) · line 153

Quoted 8 lines

arch/arm64/mm/numa.c +4 -1

@@ -147,7 +147,10 @@ static int __init early_cpu_to_node(int cpu)
 
 static int __init pcpu_cpu_distance(unsigned int from, unsigned int to)
 {
-	return node_distance(from, to);
+	if (early_cpu_to_node(from) == early_cpu_to_node(to))
+		return LOCAL_DISTANCE;
+	else
+		return REMOTE_DISTANCE;

Why can't this be node_distance(early_cpu_to_node(from), early_cpu_to_node(to))?

Will

Hanjun Guo

author

2016-10-20 12:05 UTC #10 lore raw

PATCH 1/2 arm64/numa: fix pcpu_cpu_distance() to get correct CPU proximity

Quoted 94 lines · Will Deacon

Quoted 93 lines · Hanjun Guo

From: Yisheng Xie <[email protected]>

The pcpu_build_alloc_info() function group CPUs according to their proximity, by call callback function @cpu_distance_fn from different ARCHs.

For arm64 the callback of @cpu_distance_fn is

    pcpu_cpu_distance(from, to)
        -> node_distance(from, to)

The @from and @to for function node_distance() should be nid.

However, pcpu_cpu_distance() in arch/arm64/mm/numa.c just past the cpu id for @from and @to.

For this incorrect cpu proximity get from ARCH, it may cause each CPU in one group and make group_cnt out of bound:

This may results in triggering the BUG_ON(unit != nr_units) later:

[    0.000000] kernel BUG at mm/percpu.c:1916!
[    0.000000] Internal error: Oops - BUG: 0 [#1] PREEMPT SMP
[    0.000000] Modules linked in:
[    0.000000] CPU: 0 PID: 0 Comm: swapper Not tainted 4.9.0-rc1-00003-g14155ca-dirty #26
[    0.000000] Hardware name: Hisilicon Hi1616 Evaluation Board (DT)
[    0.000000] task: ffff000008d6e900 task.stack: ffff000008d60000
[    0.000000] PC is at pcpu_embed_first_chunk+0x420/0x704
[    0.000000] LR is at pcpu_embed_first_chunk+0x3bc/0x704
[    0.000000] pc : [<ffff000008c754f4>] lr : [<ffff000008c75490>] pstate: 800000c5
[    0.000000] sp : ffff000008d63eb0
[    0.000000] x29: ffff000008d63eb0 [    0.000000] x28: 0000000000000000
[    0.000000] x27: 0000000000000040 [    0.000000] x26: ffff8413fbfcef00
[    0.000000] x25: 0000000000000042 [    0.000000] x24: 0000000000000042
[    0.000000] x23: 0000000000001000 [    0.000000] x22: 0000000000000046
[    0.000000] x21: 0000000000000001 [    0.000000] x20: ffff000008cb3bc8
[    0.000000] x19: ffff8413fbfcf570 [    0.000000] x18: 0000000000000000
[    0.000000] x17: ffff000008e49ae0 [    0.000000] x16: 0000000000000003
[    0.000000] x15: 000000000000001e [    0.000000] x14: 0000000000000004
[    0.000000] x13: 0000000000000000 [    0.000000] x12: 000000000000006f
[    0.000000] x11: 00000413fbffff00 [    0.000000] x10: 0000000000000004
[    0.000000] x9 : 0000000000000000 [    0.000000] x8 : 0000000000000001
[    0.000000] x7 : ffff8413fbfcf63c [    0.000000] x6 : ffff000008d65d28
[    0.000000] x5 : ffff000008d65e50 [    0.000000] x4 : 0000000000000000
[    0.000000] x3 : ffff000008cb3cc8 [    0.000000] x2 : 0000000000000040
[    0.000000] x1 : 0000000000000040 [    0.000000] x0 : 0000000000000000
[...]
[    0.000000] Call trace:
[    0.000000] Exception stack(0xffff000008d63ce0 to 0xffff000008d63e10)
[    0.000000] 3ce0: ffff8413fbfcf570 0001000000000000 ffff000008d63eb0 ffff000008c754f4
[    0.000000] 3d00: ffff000008d63d50 ffff0000081af210 00000413fbfff010 0000000000001000
[    0.000000] 3d20: ffff000008d63d50 ffff0000081af220 00000413fbfff010 0000000000001000
[    0.000000] 3d40: 00000413fbfcef00 0000000000000004 ffff000008d63db0 ffff0000081af390
[    0.000000] 3d60: 00000413fbfcef00 0000000000001000 0000000000000000 0000000000001000
[    0.000000] 3d80: 0000000000000000 0000000000000040 0000000000000040 ffff000008cb3cc8
[    0.000000] 3da0: 0000000000000000 ffff000008d65e50 ffff000008d65d28 ffff8413fbfcf63c
[    0.000000] 3dc0: 0000000000000001 0000000000000000 0000000000000004 00000413fbffff00
[    0.000000] 3de0: 000000000000006f 0000000000000000 0000000000000004 000000000000001e
[    0.000000] 3e00: 0000000000000003 ffff000008e49ae0
[    0.000000] [<ffff000008c754f4>] pcpu_embed_first_chunk+0x420/0x704
[    0.000000] [<ffff000008c6658c>] setup_per_cpu_areas+0x38/0xc8
[    0.000000] [<ffff000008c608d8>] start_kernel+0x10c/0x390
[    0.000000] [<ffff000008c601d8>] __primary_switched+0x5c/0x64
[    0.000000] Code: b8018660 17ffffd7 6b16037f 54000080 (d4210000)
[    0.000000] ---[ end trace 0000000000000000 ]---
[    0.000000] Kernel panic - not syncing: Attempted to kill the idle task!

Fix by getting CPUs proximity through its node. We only care about whether it is LOCAL_DISTANCE or not, for pcpu_build_alloc_info() only use this to group CPUs.

Fixes 7af3a0a99252 ("arm64/numa: support HAVE_SETUP_PER_CPU_AREA")

Signed-off-by Yisheng Xie <[email protected]>

Signed-off-by Hanjun Guo <[email protected]>

Cc Catalin Marinas <[email protected]>

Cc Lorenzo Pieralisi <[email protected]>

Cc Will Deacon <[email protected]>

Cc Zhen Lei <[email protected]>

---
 arch/arm64/mm/numa.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

arch/arm64/mm/numa.c +0 -0

index 778a985..34415fc 100644
@@ -147,7 +147,10 @@ static int __init early_cpu_to_node(int cpu)

Quoted 8 lines

arch/arm64/mm/numa.c static int __init early_cpu_to_node(int cpu) · line 153
Quoted 7 lines
arch/arm64/mm/numa.c +4 -1
 static int __init pcpu_cpu_distance(unsigned int from, unsigned int to)
 {
-	return node_distance(from, to);
+	if (early_cpu_to_node(from) == early_cpu_to_node(to))
+		return LOCAL_DISTANCE;
+	else
+		return REMOTE_DISTANCE;
Why can't this be node_distance(early_cpu_to_node(from), early_cpu_to_node(to))?

It's really some coding style preference and the caller function is only care about it's LOCAL_DISTANCE or not, as we said in the commit message.

But using node_distance() will save few lines of code and no functional change, will update it.

Thanks
Hanjun