* [PATCH 1/2] mm, slub: prevent kmalloc_node crashes and memory leaks
@ 2020-03-20 11:55 ` Vlastimil Babka
0 siblings, 0 replies; 8+ messages in thread
From: Vlastimil Babka @ 2020-03-20 11:55 UTC (permalink / raw)
To: Andrew Morton, linux-mm
Cc: Vlastimil Babka, Sachin Sant, PUVICHAKRAVARTHY RAMACHANDRAN,
Bharata B Rao, Srikar Dronamraju, stable, Mel Gorman,
Michael Ellerman, Michal Hocko, Christopher Lameter,
linuxppc-dev, Joonsoo Kim, Pekka Enberg, David Rientjes,
Kirill Tkhai, Nathan Lynch
Sachin reports [1] a crash in SLUB __slab_alloc():
BUG: Kernel NULL pointer dereference on read at 0x000073b0
Faulting instruction address: 0xc0000000003d55f4
Oops: Kernel access of bad area, sig: 11 [#1]
LE PAGE_SIZE=64K MMU=Hash SMP NR_CPUS=2048 NUMA pSeries
Modules linked in:
CPU: 19 PID: 1 Comm: systemd Not tainted 5.6.0-rc2-next-20200218-autotest #1
NIP: c0000000003d55f4 LR: c0000000003d5b94 CTR: 0000000000000000
REGS: c0000008b37836d0 TRAP: 0300 Not tainted (5.6.0-rc2-next-20200218-autotest)
MSR: 8000000000009033 <SF,EE,ME,IR,DR,RI,LE> CR: 24004844 XER: 00000000
CFAR: c00000000000dec4 DAR: 00000000000073b0 DSISR: 40000000 IRQMASK: 1
GPR00: c0000000003d5b94 c0000008b3783960 c00000000155d400 c0000008b301f500
GPR04: 0000000000000dc0 0000000000000002 c0000000003443d8 c0000008bb398620
GPR08: 00000008ba2f0000 0000000000000001 0000000000000000 0000000000000000
GPR12: 0000000024004844 c00000001ec52a00 0000000000000000 0000000000000000
GPR16: c0000008a1b20048 c000000001595898 c000000001750c18 0000000000000002
GPR20: c000000001750c28 c000000001624470 0000000fffffffe0 5deadbeef0000122
GPR24: 0000000000000001 0000000000000dc0 0000000000000002 c0000000003443d8
GPR28: c0000008b301f500 c0000008bb398620 0000000000000000 c00c000002287180
NIP [c0000000003d55f4] ___slab_alloc+0x1f4/0x760
LR [c0000000003d5b94] __slab_alloc+0x34/0x60
Call Trace:
[c0000008b3783960] [c0000000003d5734] ___slab_alloc+0x334/0x760 (unreliable)
[c0000008b3783a40] [c0000000003d5b94] __slab_alloc+0x34/0x60
[c0000008b3783a70] [c0000000003d6fa0] __kmalloc_node+0x110/0x490
[c0000008b3783af0] [c0000000003443d8] kvmalloc_node+0x58/0x110
[c0000008b3783b30] [c0000000003fee38] mem_cgroup_css_online+0x108/0x270
[c0000008b3783b90] [c000000000235aa8] online_css+0x48/0xd0
[c0000008b3783bc0] [c00000000023eaec] cgroup_apply_control_enable+0x2ec/0x4d0
[c0000008b3783ca0] [c000000000242318] cgroup_mkdir+0x228/0x5f0
[c0000008b3783d10] [c00000000051e170] kernfs_iop_mkdir+0x90/0xf0
[c0000008b3783d50] [c00000000043dc00] vfs_mkdir+0x110/0x230
[c0000008b3783da0] [c000000000441c90] do_mkdirat+0xb0/0x1a0
[c0000008b3783e20] [c00000000000b278] system_call+0x5c/0x68
This is a PowerPC platform with following NUMA topology:
available: 2 nodes (0-1)
node 0 cpus:
node 0 size: 0 MB
node 0 free: 0 MB
node 1 cpus: 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31
node 1 size: 35247 MB
node 1 free: 30907 MB
node distances:
node 0 1
0: 10 40
1: 40 10
possible numa nodes: 0-31
This only happens with a mmotm patch "mm/memcontrol.c: allocate shrinker_map on
appropriate NUMA node" [2] which effectively calls kmalloc_node for each
possible node. SLUB however only allocates kmem_cache_node on online
N_NORMAL_MEMORY nodes, and relies on node_to_mem_node to return such valid node
for other nodes since commit a561ce00b09e ("slub: fall back to
node_to_mem_node() node if allocating on memoryless node"). This is however not
true in this configuration where the _node_numa_mem_ array is not initialized
for nodes 0 and 2-31, thus it contains zeroes and get_partial() ends up
accessing non-allocated kmem_cache_node.
A related issue was reported by Bharata (originally by Ramachandran) [3] where
a similar PowerPC configuration, but with mainline kernel without patch [2]
ends up allocating large amounts of pages by kmalloc-1k kmalloc-512. This seems
to have the same underlying issue with node_to_mem_node() not behaving as
expected, and might probably also lead to an infinite loop with
CONFIG_SLUB_CPU_PARTIAL [4].
This patch should fix both issues by not relying on node_to_mem_node() anymore
and instead simply falling back to NUMA_NO_NODE, when kmalloc_node(node) is
attempted for a node that's not online, or has no usable memory. The "usable
memory" condition is also changed from node_present_pages() to N_NORMAL_MEMORY
node state, as that is exactly the condition that SLUB uses to allocate
kmem_cache_node structures. The check in get_partial() is removed completely,
as the checks in ___slab_alloc() are now sufficient to prevent get_partial()
being reached with an invalid node.
[1] https://lore.kernel.org/linux-next/3381CD91-AB3D-4773-BA04-E7A072A63968@linux.vnet.ibm.com/
[2] https://lore.kernel.org/linux-mm/fff0e636-4c36-ed10-281c-8cdb0687c839@virtuozzo.com/
[3] https://lore.kernel.org/linux-mm/20200317092624.GB22538@in.ibm.com/
[4] https://lore.kernel.org/linux-mm/088b5996-faae-8a56-ef9c-5b567125ae54@suse.cz/
Reported-and-tested-by: Sachin Sant <sachinp@linux.vnet.ibm.com>
Reported-by: PUVICHAKRAVARTHY RAMACHANDRAN <puvichakravarthy@in.ibm.com>
Tested-by: Bharata B Rao <bharata@linux.ibm.com>
Debugged-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
Fixes: a561ce00b09e ("slub: fall back to node_to_mem_node() node if allocating on memoryless node")
Cc: stable@vger.kernel.org
Cc: Mel Gorman <mgorman@techsingularity.net>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Michal Hocko <mhocko@kernel.org>
Cc: Christopher Lameter <cl@linux.com>
Cc: linuxppc-dev@lists.ozlabs.org
Cc: Joonsoo Kim <iamjoonsoo.kim@lge.com>
Cc: Pekka Enberg <penberg@kernel.org>
Cc: David Rientjes <rientjes@google.com>
Cc: Kirill Tkhai <ktkhai@virtuozzo.com>
Cc: Vlastimil Babka <vbabka@suse.cz>
Cc: Nathan Lynch <nathanl@linux.ibm.com>
---
Hi, after the RFC got positive test results from reporters, I'm resending
as a proper patch.
Thanks,
Vlastimil
mm/slub.c | 26 +++++++++++++++++---------
1 file changed, 17 insertions(+), 9 deletions(-)
diff --git a/mm/slub.c b/mm/slub.c
index 17dc00e33115..7113b1f9cd77 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -1973,8 +1973,6 @@ static void *get_partial(struct kmem_cache *s, gfp_t flags, int node,
if (node == NUMA_NO_NODE)
searchnode = numa_mem_id();
- else if (!node_present_pages(node))
- searchnode = node_to_mem_node(node);
object = get_partial_node(s, get_node(s, searchnode), c, flags);
if (object || node != NUMA_NO_NODE)
@@ -2563,17 +2561,27 @@ static void *___slab_alloc(struct kmem_cache *s, gfp_t gfpflags, int node,
struct page *page;
page = c->page;
- if (!page)
+ if (!page) {
+ /*
+ * if the node is not online or has no normal memory, just
+ * ignore the node constraint
+ */
+ if (unlikely(node != NUMA_NO_NODE &&
+ !node_state(node, N_NORMAL_MEMORY)))
+ node = NUMA_NO_NODE;
goto new_slab;
+ }
redo:
if (unlikely(!node_match(page, node))) {
- int searchnode = node;
-
- if (node != NUMA_NO_NODE && !node_present_pages(node))
- searchnode = node_to_mem_node(node);
-
- if (unlikely(!node_match(page, searchnode))) {
+ /*
+ * same as above but node_match() being false already
+ * implies node != NUMA_NO_NODE
+ */
+ if (!node_state(node, N_NORMAL_MEMORY)) {
+ node = NUMA_NO_NODE;
+ goto redo;
+ } else {
stat(s, ALLOC_NODE_MISMATCH);
deactivate_slab(s, page, c->freelist, c);
goto new_slab;
--
2.25.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 1/2] mm, slub: prevent kmalloc_node crashes and memory leaks
@ 2020-03-20 11:55 ` Vlastimil Babka
0 siblings, 0 replies; 8+ messages in thread
From: Vlastimil Babka @ 2020-03-20 11:55 UTC (permalink / raw)
To: Andrew Morton, linux-mm
Cc: Sachin Sant, Nathan Lynch, Srikar Dronamraju,
PUVICHAKRAVARTHY RAMACHANDRAN, linuxppc-dev, stable,
Bharata B Rao, Pekka Enberg, Kirill Tkhai, David Rientjes,
Christopher Lameter, Michal Hocko, Mel Gorman, Joonsoo Kim,
Vlastimil Babka
Sachin reports [1] a crash in SLUB __slab_alloc():
BUG: Kernel NULL pointer dereference on read at 0x000073b0
Faulting instruction address: 0xc0000000003d55f4
Oops: Kernel access of bad area, sig: 11 [#1]
LE PAGE_SIZE=64K MMU=Hash SMP NR_CPUS=2048 NUMA pSeries
Modules linked in:
CPU: 19 PID: 1 Comm: systemd Not tainted 5.6.0-rc2-next-20200218-autotest #1
NIP: c0000000003d55f4 LR: c0000000003d5b94 CTR: 0000000000000000
REGS: c0000008b37836d0 TRAP: 0300 Not tainted (5.6.0-rc2-next-20200218-autotest)
MSR: 8000000000009033 <SF,EE,ME,IR,DR,RI,LE> CR: 24004844 XER: 00000000
CFAR: c00000000000dec4 DAR: 00000000000073b0 DSISR: 40000000 IRQMASK: 1
GPR00: c0000000003d5b94 c0000008b3783960 c00000000155d400 c0000008b301f500
GPR04: 0000000000000dc0 0000000000000002 c0000000003443d8 c0000008bb398620
GPR08: 00000008ba2f0000 0000000000000001 0000000000000000 0000000000000000
GPR12: 0000000024004844 c00000001ec52a00 0000000000000000 0000000000000000
GPR16: c0000008a1b20048 c000000001595898 c000000001750c18 0000000000000002
GPR20: c000000001750c28 c000000001624470 0000000fffffffe0 5deadbeef0000122
GPR24: 0000000000000001 0000000000000dc0 0000000000000002 c0000000003443d8
GPR28: c0000008b301f500 c0000008bb398620 0000000000000000 c00c000002287180
NIP [c0000000003d55f4] ___slab_alloc+0x1f4/0x760
LR [c0000000003d5b94] __slab_alloc+0x34/0x60
Call Trace:
[c0000008b3783960] [c0000000003d5734] ___slab_alloc+0x334/0x760 (unreliable)
[c0000008b3783a40] [c0000000003d5b94] __slab_alloc+0x34/0x60
[c0000008b3783a70] [c0000000003d6fa0] __kmalloc_node+0x110/0x490
[c0000008b3783af0] [c0000000003443d8] kvmalloc_node+0x58/0x110
[c0000008b3783b30] [c0000000003fee38] mem_cgroup_css_online+0x108/0x270
[c0000008b3783b90] [c000000000235aa8] online_css+0x48/0xd0
[c0000008b3783bc0] [c00000000023eaec] cgroup_apply_control_enable+0x2ec/0x4d0
[c0000008b3783ca0] [c000000000242318] cgroup_mkdir+0x228/0x5f0
[c0000008b3783d10] [c00000000051e170] kernfs_iop_mkdir+0x90/0xf0
[c0000008b3783d50] [c00000000043dc00] vfs_mkdir+0x110/0x230
[c0000008b3783da0] [c000000000441c90] do_mkdirat+0xb0/0x1a0
[c0000008b3783e20] [c00000000000b278] system_call+0x5c/0x68
This is a PowerPC platform with following NUMA topology:
available: 2 nodes (0-1)
node 0 cpus:
node 0 size: 0 MB
node 0 free: 0 MB
node 1 cpus: 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31
node 1 size: 35247 MB
node 1 free: 30907 MB
node distances:
node 0 1
0: 10 40
1: 40 10
possible numa nodes: 0-31
This only happens with a mmotm patch "mm/memcontrol.c: allocate shrinker_map on
appropriate NUMA node" [2] which effectively calls kmalloc_node for each
possible node. SLUB however only allocates kmem_cache_node on online
N_NORMAL_MEMORY nodes, and relies on node_to_mem_node to return such valid node
for other nodes since commit a561ce00b09e ("slub: fall back to
node_to_mem_node() node if allocating on memoryless node"). This is however not
true in this configuration where the _node_numa_mem_ array is not initialized
for nodes 0 and 2-31, thus it contains zeroes and get_partial() ends up
accessing non-allocated kmem_cache_node.
A related issue was reported by Bharata (originally by Ramachandran) [3] where
a similar PowerPC configuration, but with mainline kernel without patch [2]
ends up allocating large amounts of pages by kmalloc-1k kmalloc-512. This seems
to have the same underlying issue with node_to_mem_node() not behaving as
expected, and might probably also lead to an infinite loop with
CONFIG_SLUB_CPU_PARTIAL [4].
This patch should fix both issues by not relying on node_to_mem_node() anymore
and instead simply falling back to NUMA_NO_NODE, when kmalloc_node(node) is
attempted for a node that's not online, or has no usable memory. The "usable
memory" condition is also changed from node_present_pages() to N_NORMAL_MEMORY
node state, as that is exactly the condition that SLUB uses to allocate
kmem_cache_node structures. The check in get_partial() is removed completely,
as the checks in ___slab_alloc() are now sufficient to prevent get_partial()
being reached with an invalid node.
[1] https://lore.kernel.org/linux-next/3381CD91-AB3D-4773-BA04-E7A072A63968@linux.vnet.ibm.com/
[2] https://lore.kernel.org/linux-mm/fff0e636-4c36-ed10-281c-8cdb0687c839@virtuozzo.com/
[3] https://lore.kernel.org/linux-mm/20200317092624.GB22538@in.ibm.com/
[4] https://lore.kernel.org/linux-mm/088b5996-faae-8a56-ef9c-5b567125ae54@suse.cz/
Reported-and-tested-by: Sachin Sant <sachinp@linux.vnet.ibm.com>
Reported-by: PUVICHAKRAVARTHY RAMACHANDRAN <puvichakravarthy@in.ibm.com>
Tested-by: Bharata B Rao <bharata@linux.ibm.com>
Debugged-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
Fixes: a561ce00b09e ("slub: fall back to node_to_mem_node() node if allocating on memoryless node")
Cc: stable@vger.kernel.org
Cc: Mel Gorman <mgorman@techsingularity.net>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Michal Hocko <mhocko@kernel.org>
Cc: Christopher Lameter <cl@linux.com>
Cc: linuxppc-dev@lists.ozlabs.org
Cc: Joonsoo Kim <iamjoonsoo.kim@lge.com>
Cc: Pekka Enberg <penberg@kernel.org>
Cc: David Rientjes <rientjes@google.com>
Cc: Kirill Tkhai <ktkhai@virtuozzo.com>
Cc: Vlastimil Babka <vbabka@suse.cz>
Cc: Nathan Lynch <nathanl@linux.ibm.com>
---
Hi, after the RFC got positive test results from reporters, I'm resending
as a proper patch.
Thanks,
Vlastimil
mm/slub.c | 26 +++++++++++++++++---------
1 file changed, 17 insertions(+), 9 deletions(-)
diff --git a/mm/slub.c b/mm/slub.c
index 17dc00e33115..7113b1f9cd77 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -1973,8 +1973,6 @@ static void *get_partial(struct kmem_cache *s, gfp_t flags, int node,
if (node == NUMA_NO_NODE)
searchnode = numa_mem_id();
- else if (!node_present_pages(node))
- searchnode = node_to_mem_node(node);
object = get_partial_node(s, get_node(s, searchnode), c, flags);
if (object || node != NUMA_NO_NODE)
@@ -2563,17 +2561,27 @@ static void *___slab_alloc(struct kmem_cache *s, gfp_t gfpflags, int node,
struct page *page;
page = c->page;
- if (!page)
+ if (!page) {
+ /*
+ * if the node is not online or has no normal memory, just
+ * ignore the node constraint
+ */
+ if (unlikely(node != NUMA_NO_NODE &&
+ !node_state(node, N_NORMAL_MEMORY)))
+ node = NUMA_NO_NODE;
goto new_slab;
+ }
redo:
if (unlikely(!node_match(page, node))) {
- int searchnode = node;
-
- if (node != NUMA_NO_NODE && !node_present_pages(node))
- searchnode = node_to_mem_node(node);
-
- if (unlikely(!node_match(page, searchnode))) {
+ /*
+ * same as above but node_match() being false already
+ * implies node != NUMA_NO_NODE
+ */
+ if (!node_state(node, N_NORMAL_MEMORY)) {
+ node = NUMA_NO_NODE;
+ goto redo;
+ } else {
stat(s, ALLOC_NODE_MISMATCH);
deactivate_slab(s, page, c->freelist, c);
goto new_slab;
--
2.25.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/2] Revert "topology: add support for node_to_mem_node() to determine the fallback node"
2020-03-20 11:55 ` Vlastimil Babka
(?)
@ 2020-03-20 11:55 ` Vlastimil Babka
2020-03-20 13:12 ` Srikar Dronamraju
2020-03-24 13:00 ` Vlastimil Babka
-1 siblings, 2 replies; 8+ messages in thread
From: Vlastimil Babka @ 2020-03-20 11:55 UTC (permalink / raw)
To: Andrew Morton, linux-mm
Cc: Vlastimil Babka, Joonsoo Kim, Bharata B Rao, Christopher Lameter,
David Rientjes, Kirill Tkhai, Mel Gorman, Michael Ellerman,
Michal Hocko, Nathan Lynch, Pekka Enberg,
PUVICHAKRAVARTHY RAMACHANDRAN, Sachin Sant, Srikar Dronamraju
This reverts commit ad2c8144418c6a81cefe65379fd47bbe8344cef2.
The function node_to_mem_node() was introduced by that commit for use in SLUB
on systems with memoryless nodes, but it turned out to be unreliable on some
architectures/configurations and a simpler solution exists than fixing it up.
Thus the previous commit "mm, slub: prevent kmalloc_node crashes and memory
leaks" removed the only user of node_to_mem_node() and we can revert the commit
that introduced it.
Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
Cc: Joonsoo Kim <iamjoonsoo.kim@lge.com>
---
include/linux/topology.h | 17 -----------------
mm/page_alloc.c | 1 -
2 files changed, 18 deletions(-)
diff --git a/include/linux/topology.h b/include/linux/topology.h
index eb2fe6edd73c..608fa4aadf0e 100644
--- a/include/linux/topology.h
+++ b/include/linux/topology.h
@@ -130,20 +130,11 @@ static inline int numa_node_id(void)
* Use the accessor functions set_numa_mem(), numa_mem_id() and cpu_to_mem().
*/
DECLARE_PER_CPU(int, _numa_mem_);
-extern int _node_numa_mem_[MAX_NUMNODES];
#ifndef set_numa_mem
static inline void set_numa_mem(int node)
{
this_cpu_write(_numa_mem_, node);
- _node_numa_mem_[numa_node_id()] = node;
-}
-#endif
-
-#ifndef node_to_mem_node
-static inline int node_to_mem_node(int node)
-{
- return _node_numa_mem_[node];
}
#endif
@@ -166,7 +157,6 @@ static inline int cpu_to_mem(int cpu)
static inline void set_cpu_numa_mem(int cpu, int node)
{
per_cpu(_numa_mem_, cpu) = node;
- _node_numa_mem_[cpu_to_node(cpu)] = node;
}
#endif
@@ -180,13 +170,6 @@ static inline int numa_mem_id(void)
}
#endif
-#ifndef node_to_mem_node
-static inline int node_to_mem_node(int node)
-{
- return node;
-}
-#endif
-
#ifndef cpu_to_mem
static inline int cpu_to_mem(int cpu)
{
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 3c4eb750a199..6e7e9c1d6caa 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -95,7 +95,6 @@ DEFINE_STATIC_KEY_TRUE(vm_numa_stat_key);
*/
DEFINE_PER_CPU(int, _numa_mem_); /* Kernel "local memory" node */
EXPORT_PER_CPU_SYMBOL(_numa_mem_);
-int _node_numa_mem_[MAX_NUMNODES];
#endif
/* work_structs for global per-cpu drains */
--
2.25.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] mm, slub: prevent kmalloc_node crashes and memory leaks
2020-03-20 11:55 ` Vlastimil Babka
@ 2020-03-20 13:10 ` Srikar Dronamraju
-1 siblings, 0 replies; 8+ messages in thread
From: Srikar Dronamraju @ 2020-03-20 13:10 UTC (permalink / raw)
To: Vlastimil Babka
Cc: Andrew Morton, linux-mm, Sachin Sant,
PUVICHAKRAVARTHY RAMACHANDRAN, Bharata B Rao, stable, Mel Gorman,
Michael Ellerman, Michal Hocko, Christopher Lameter,
linuxppc-dev, Joonsoo Kim, Pekka Enberg, David Rientjes,
Kirill Tkhai, Nathan Lynch
* Vlastimil Babka <vbabka@suse.cz> [2020-03-20 12:55:32]:
> Sachin reports [1] a crash in SLUB __slab_alloc():
>
> BUG: Kernel NULL pointer dereference on read at 0x000073b0
> Faulting instruction address: 0xc0000000003d55f4
> Oops: Kernel access of bad area, sig: 11 [#1]
> LE PAGE_SIZE=64K MMU=Hash SMP NR_CPUS=2048 NUMA pSeries
> Modules linked in:
> CPU: 19 PID: 1 Comm: systemd Not tainted 5.6.0-rc2-next-20200218-autotest #1
> NIP: c0000000003d55f4 LR: c0000000003d5b94 CTR: 0000000000000000
> REGS: c0000008b37836d0 TRAP: 0300 Not tainted (5.6.0-rc2-next-20200218-autotest)
> MSR: 8000000000009033 <SF,EE,ME,IR,DR,RI,LE> CR: 24004844 XER: 00000000
> CFAR: c00000000000dec4 DAR: 00000000000073b0 DSISR: 40000000 IRQMASK: 1
> GPR00: c0000000003d5b94 c0000008b3783960 c00000000155d400 c0000008b301f500
> GPR04: 0000000000000dc0 0000000000000002 c0000000003443d8 c0000008bb398620
> GPR08: 00000008ba2f0000 0000000000000001 0000000000000000 0000000000000000
> GPR12: 0000000024004844 c00000001ec52a00 0000000000000000 0000000000000000
> GPR16: c0000008a1b20048 c000000001595898 c000000001750c18 0000000000000002
> GPR20: c000000001750c28 c000000001624470 0000000fffffffe0 5deadbeef0000122
> GPR24: 0000000000000001 0000000000000dc0 0000000000000002 c0000000003443d8
> GPR28: c0000008b301f500 c0000008bb398620 0000000000000000 c00c000002287180
> NIP [c0000000003d55f4] ___slab_alloc+0x1f4/0x760
> LR [c0000000003d5b94] __slab_alloc+0x34/0x60
> Call Trace:
> [c0000008b3783960] [c0000000003d5734] ___slab_alloc+0x334/0x760 (unreliable)
> [c0000008b3783a40] [c0000000003d5b94] __slab_alloc+0x34/0x60
> [c0000008b3783a70] [c0000000003d6fa0] __kmalloc_node+0x110/0x490
> [c0000008b3783af0] [c0000000003443d8] kvmalloc_node+0x58/0x110
> [c0000008b3783b30] [c0000000003fee38] mem_cgroup_css_online+0x108/0x270
> [c0000008b3783b90] [c000000000235aa8] online_css+0x48/0xd0
> [c0000008b3783bc0] [c00000000023eaec] cgroup_apply_control_enable+0x2ec/0x4d0
> [c0000008b3783ca0] [c000000000242318] cgroup_mkdir+0x228/0x5f0
> [c0000008b3783d10] [c00000000051e170] kernfs_iop_mkdir+0x90/0xf0
> [c0000008b3783d50] [c00000000043dc00] vfs_mkdir+0x110/0x230
> [c0000008b3783da0] [c000000000441c90] do_mkdirat+0xb0/0x1a0
> [c0000008b3783e20] [c00000000000b278] system_call+0x5c/0x68
>
> This is a PowerPC platform with following NUMA topology:
>
> available: 2 nodes (0-1)
> node 0 cpus:
> node 0 size: 0 MB
> node 0 free: 0 MB
> node 1 cpus: 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31
> node 1 size: 35247 MB
> node 1 free: 30907 MB
> node distances:
> node 0 1
> 0: 10 40
> 1: 40 10
>
> possible numa nodes: 0-31
>
> This only happens with a mmotm patch "mm/memcontrol.c: allocate shrinker_map on
> appropriate NUMA node" [2] which effectively calls kmalloc_node for each
> possible node. SLUB however only allocates kmem_cache_node on online
> N_NORMAL_MEMORY nodes, and relies on node_to_mem_node to return such valid node
> for other nodes since commit a561ce00b09e ("slub: fall back to
> node_to_mem_node() node if allocating on memoryless node"). This is however not
> true in this configuration where the _node_numa_mem_ array is not initialized
> for nodes 0 and 2-31, thus it contains zeroes and get_partial() ends up
> accessing non-allocated kmem_cache_node.
>
> A related issue was reported by Bharata (originally by Ramachandran) [3] where
> a similar PowerPC configuration, but with mainline kernel without patch [2]
> ends up allocating large amounts of pages by kmalloc-1k kmalloc-512. This seems
> to have the same underlying issue with node_to_mem_node() not behaving as
> expected, and might probably also lead to an infinite loop with
> CONFIG_SLUB_CPU_PARTIAL [4].
>
> This patch should fix both issues by not relying on node_to_mem_node() anymore
> and instead simply falling back to NUMA_NO_NODE, when kmalloc_node(node) is
> attempted for a node that's not online, or has no usable memory. The "usable
> memory" condition is also changed from node_present_pages() to N_NORMAL_MEMORY
> node state, as that is exactly the condition that SLUB uses to allocate
> kmem_cache_node structures. The check in get_partial() is removed completely,
> as the checks in ___slab_alloc() are now sufficient to prevent get_partial()
> being reached with an invalid node.
>
> [1] https://lore.kernel.org/linux-next/3381CD91-AB3D-4773-BA04-E7A072A63968@linux.vnet.ibm.com/
> [2] https://lore.kernel.org/linux-mm/fff0e636-4c36-ed10-281c-8cdb0687c839@virtuozzo.com/
> [3] https://lore.kernel.org/linux-mm/20200317092624.GB22538@in.ibm.com/
> [4] https://lore.kernel.org/linux-mm/088b5996-faae-8a56-ef9c-5b567125ae54@suse.cz/
>
> Reported-and-tested-by: Sachin Sant <sachinp@linux.vnet.ibm.com>
> Reported-by: PUVICHAKRAVARTHY RAMACHANDRAN <puvichakravarthy@in.ibm.com>
> Tested-by: Bharata B Rao <bharata@linux.ibm.com>
> Debugged-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
> Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
> Fixes: a561ce00b09e ("slub: fall back to node_to_mem_node() node if allocating on memoryless node")
Reviewed-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
--
Thanks and Regards
Srikar Dronamraju
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] mm, slub: prevent kmalloc_node crashes and memory leaks
@ 2020-03-20 13:10 ` Srikar Dronamraju
0 siblings, 0 replies; 8+ messages in thread
From: Srikar Dronamraju @ 2020-03-20 13:10 UTC (permalink / raw)
To: Vlastimil Babka
Cc: Sachin Sant, Nathan Lynch, PUVICHAKRAVARTHY RAMACHANDRAN,
linuxppc-dev, stable, Bharata B Rao, Pekka Enberg, linux-mm,
Kirill Tkhai, David Rientjes, Joonsoo Kim, Andrew Morton,
Michal Hocko, Mel Gorman, Christopher Lameter
* Vlastimil Babka <vbabka@suse.cz> [2020-03-20 12:55:32]:
> Sachin reports [1] a crash in SLUB __slab_alloc():
>
> BUG: Kernel NULL pointer dereference on read at 0x000073b0
> Faulting instruction address: 0xc0000000003d55f4
> Oops: Kernel access of bad area, sig: 11 [#1]
> LE PAGE_SIZE=64K MMU=Hash SMP NR_CPUS=2048 NUMA pSeries
> Modules linked in:
> CPU: 19 PID: 1 Comm: systemd Not tainted 5.6.0-rc2-next-20200218-autotest #1
> NIP: c0000000003d55f4 LR: c0000000003d5b94 CTR: 0000000000000000
> REGS: c0000008b37836d0 TRAP: 0300 Not tainted (5.6.0-rc2-next-20200218-autotest)
> MSR: 8000000000009033 <SF,EE,ME,IR,DR,RI,LE> CR: 24004844 XER: 00000000
> CFAR: c00000000000dec4 DAR: 00000000000073b0 DSISR: 40000000 IRQMASK: 1
> GPR00: c0000000003d5b94 c0000008b3783960 c00000000155d400 c0000008b301f500
> GPR04: 0000000000000dc0 0000000000000002 c0000000003443d8 c0000008bb398620
> GPR08: 00000008ba2f0000 0000000000000001 0000000000000000 0000000000000000
> GPR12: 0000000024004844 c00000001ec52a00 0000000000000000 0000000000000000
> GPR16: c0000008a1b20048 c000000001595898 c000000001750c18 0000000000000002
> GPR20: c000000001750c28 c000000001624470 0000000fffffffe0 5deadbeef0000122
> GPR24: 0000000000000001 0000000000000dc0 0000000000000002 c0000000003443d8
> GPR28: c0000008b301f500 c0000008bb398620 0000000000000000 c00c000002287180
> NIP [c0000000003d55f4] ___slab_alloc+0x1f4/0x760
> LR [c0000000003d5b94] __slab_alloc+0x34/0x60
> Call Trace:
> [c0000008b3783960] [c0000000003d5734] ___slab_alloc+0x334/0x760 (unreliable)
> [c0000008b3783a40] [c0000000003d5b94] __slab_alloc+0x34/0x60
> [c0000008b3783a70] [c0000000003d6fa0] __kmalloc_node+0x110/0x490
> [c0000008b3783af0] [c0000000003443d8] kvmalloc_node+0x58/0x110
> [c0000008b3783b30] [c0000000003fee38] mem_cgroup_css_online+0x108/0x270
> [c0000008b3783b90] [c000000000235aa8] online_css+0x48/0xd0
> [c0000008b3783bc0] [c00000000023eaec] cgroup_apply_control_enable+0x2ec/0x4d0
> [c0000008b3783ca0] [c000000000242318] cgroup_mkdir+0x228/0x5f0
> [c0000008b3783d10] [c00000000051e170] kernfs_iop_mkdir+0x90/0xf0
> [c0000008b3783d50] [c00000000043dc00] vfs_mkdir+0x110/0x230
> [c0000008b3783da0] [c000000000441c90] do_mkdirat+0xb0/0x1a0
> [c0000008b3783e20] [c00000000000b278] system_call+0x5c/0x68
>
> This is a PowerPC platform with following NUMA topology:
>
> available: 2 nodes (0-1)
> node 0 cpus:
> node 0 size: 0 MB
> node 0 free: 0 MB
> node 1 cpus: 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31
> node 1 size: 35247 MB
> node 1 free: 30907 MB
> node distances:
> node 0 1
> 0: 10 40
> 1: 40 10
>
> possible numa nodes: 0-31
>
> This only happens with a mmotm patch "mm/memcontrol.c: allocate shrinker_map on
> appropriate NUMA node" [2] which effectively calls kmalloc_node for each
> possible node. SLUB however only allocates kmem_cache_node on online
> N_NORMAL_MEMORY nodes, and relies on node_to_mem_node to return such valid node
> for other nodes since commit a561ce00b09e ("slub: fall back to
> node_to_mem_node() node if allocating on memoryless node"). This is however not
> true in this configuration where the _node_numa_mem_ array is not initialized
> for nodes 0 and 2-31, thus it contains zeroes and get_partial() ends up
> accessing non-allocated kmem_cache_node.
>
> A related issue was reported by Bharata (originally by Ramachandran) [3] where
> a similar PowerPC configuration, but with mainline kernel without patch [2]
> ends up allocating large amounts of pages by kmalloc-1k kmalloc-512. This seems
> to have the same underlying issue with node_to_mem_node() not behaving as
> expected, and might probably also lead to an infinite loop with
> CONFIG_SLUB_CPU_PARTIAL [4].
>
> This patch should fix both issues by not relying on node_to_mem_node() anymore
> and instead simply falling back to NUMA_NO_NODE, when kmalloc_node(node) is
> attempted for a node that's not online, or has no usable memory. The "usable
> memory" condition is also changed from node_present_pages() to N_NORMAL_MEMORY
> node state, as that is exactly the condition that SLUB uses to allocate
> kmem_cache_node structures. The check in get_partial() is removed completely,
> as the checks in ___slab_alloc() are now sufficient to prevent get_partial()
> being reached with an invalid node.
>
> [1] https://lore.kernel.org/linux-next/3381CD91-AB3D-4773-BA04-E7A072A63968@linux.vnet.ibm.com/
> [2] https://lore.kernel.org/linux-mm/fff0e636-4c36-ed10-281c-8cdb0687c839@virtuozzo.com/
> [3] https://lore.kernel.org/linux-mm/20200317092624.GB22538@in.ibm.com/
> [4] https://lore.kernel.org/linux-mm/088b5996-faae-8a56-ef9c-5b567125ae54@suse.cz/
>
> Reported-and-tested-by: Sachin Sant <sachinp@linux.vnet.ibm.com>
> Reported-by: PUVICHAKRAVARTHY RAMACHANDRAN <puvichakravarthy@in.ibm.com>
> Tested-by: Bharata B Rao <bharata@linux.ibm.com>
> Debugged-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
> Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
> Fixes: a561ce00b09e ("slub: fall back to node_to_mem_node() node if allocating on memoryless node")
Reviewed-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
--
Thanks and Regards
Srikar Dronamraju
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] Revert "topology: add support for node_to_mem_node() to determine the fallback node"
2020-03-20 11:55 ` [PATCH 2/2] Revert "topology: add support for node_to_mem_node() to determine the fallback node" Vlastimil Babka
@ 2020-03-20 13:12 ` Srikar Dronamraju
2020-03-24 13:00 ` Vlastimil Babka
1 sibling, 0 replies; 8+ messages in thread
From: Srikar Dronamraju @ 2020-03-20 13:12 UTC (permalink / raw)
To: Vlastimil Babka
Cc: Andrew Morton, linux-mm, Joonsoo Kim, Bharata B Rao,
Christopher Lameter, David Rientjes, Kirill Tkhai, Mel Gorman,
Michael Ellerman, Michal Hocko, Nathan Lynch, Pekka Enberg,
PUVICHAKRAVARTHY RAMACHANDRAN, Sachin Sant
* Vlastimil Babka <vbabka@suse.cz> [2020-03-20 12:55:33]:
> This reverts commit ad2c8144418c6a81cefe65379fd47bbe8344cef2.
>
> The function node_to_mem_node() was introduced by that commit for use in SLUB
> on systems with memoryless nodes, but it turned out to be unreliable on some
> architectures/configurations and a simpler solution exists than fixing it up.
>
> Thus the previous commit "mm, slub: prevent kmalloc_node crashes and memory
> leaks" removed the only user of node_to_mem_node() and we can revert the commit
> that introduced it.
>
> Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
> Cc: Joonsoo Kim <iamjoonsoo.kim@lge.com>
Reviewed-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
> ---
> include/linux/topology.h | 17 -----------------
> mm/page_alloc.c | 1 -
> 2 files changed, 18 deletions(-)
>
> diff --git a/include/linux/topology.h b/include/linux/topology.h
> index eb2fe6edd73c..608fa4aadf0e 100644
> --- a/include/linux/topology.h
> +++ b/include/linux/topology.h
> @@ -130,20 +130,11 @@ static inline int numa_node_id(void)
> * Use the accessor functions set_numa_mem(), numa_mem_id() and cpu_to_mem().
> */
> DECLARE_PER_CPU(int, _numa_mem_);
> -extern int _node_numa_mem_[MAX_NUMNODES];
>
> #ifndef set_numa_mem
> static inline void set_numa_mem(int node)
> {
> this_cpu_write(_numa_mem_, node);
> - _node_numa_mem_[numa_node_id()] = node;
> -}
> -#endif
> -
> -#ifndef node_to_mem_node
> -static inline int node_to_mem_node(int node)
> -{
> - return _node_numa_mem_[node];
> }
> #endif
>
> @@ -166,7 +157,6 @@ static inline int cpu_to_mem(int cpu)
> static inline void set_cpu_numa_mem(int cpu, int node)
> {
> per_cpu(_numa_mem_, cpu) = node;
> - _node_numa_mem_[cpu_to_node(cpu)] = node;
> }
> #endif
>
> @@ -180,13 +170,6 @@ static inline int numa_mem_id(void)
> }
> #endif
>
> -#ifndef node_to_mem_node
> -static inline int node_to_mem_node(int node)
> -{
> - return node;
> -}
> -#endif
> -
> #ifndef cpu_to_mem
> static inline int cpu_to_mem(int cpu)
> {
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 3c4eb750a199..6e7e9c1d6caa 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -95,7 +95,6 @@ DEFINE_STATIC_KEY_TRUE(vm_numa_stat_key);
> */
> DEFINE_PER_CPU(int, _numa_mem_); /* Kernel "local memory" node */
> EXPORT_PER_CPU_SYMBOL(_numa_mem_);
> -int _node_numa_mem_[MAX_NUMNODES];
> #endif
>
> /* work_structs for global per-cpu drains */
> --
> 2.25.1
>
--
Thanks and Regards
Srikar Dronamraju
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] Revert "topology: add support for node_to_mem_node() to determine the fallback node"
2020-03-20 11:55 ` [PATCH 2/2] Revert "topology: add support for node_to_mem_node() to determine the fallback node" Vlastimil Babka
2020-03-20 13:12 ` Srikar Dronamraju
@ 2020-03-24 13:00 ` Vlastimil Babka
2020-03-26 0:54 ` Andrew Morton
1 sibling, 1 reply; 8+ messages in thread
From: Vlastimil Babka @ 2020-03-24 13:00 UTC (permalink / raw)
To: Andrew Morton, linux-mm
Cc: Joonsoo Kim, Bharata B Rao, Christopher Lameter, David Rientjes,
Kirill Tkhai, Mel Gorman, Michael Ellerman, Michal Hocko,
Nathan Lynch, Pekka Enberg, PUVICHAKRAVARTHY RAMACHANDRAN,
Sachin Sant, Srikar Dronamraju
On 3/20/20 12:55 PM, Vlastimil Babka wrote:
> This reverts commit ad2c8144418c6a81cefe65379fd47bbe8344cef2.
>
> The function node_to_mem_node() was introduced by that commit for use in SLUB
> on systems with memoryless nodes, but it turned out to be unreliable on some
> architectures/configurations and a simpler solution exists than fixing it up.
>
> Thus the previous commit "mm, slub: prevent kmalloc_node crashes and memory
> leaks" removed the only user of node_to_mem_node() and we can revert the commit
> that introduced it.
Since the bugfix was fast-tracked to Linus, we can now reference it properly and
replace the second paragraph with:
Thus commit 0715e6c516f1 ("mm, slub: prevent kmalloc_node crashes and memory
leaks") removed the only user of node_to_mem_node() and we can revert the commit
that introduced the function.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] Revert "topology: add support for node_to_mem_node() to determine the fallback node"
2020-03-24 13:00 ` Vlastimil Babka
@ 2020-03-26 0:54 ` Andrew Morton
0 siblings, 0 replies; 8+ messages in thread
From: Andrew Morton @ 2020-03-26 0:54 UTC (permalink / raw)
To: Vlastimil Babka
Cc: linux-mm, Joonsoo Kim, Bharata B Rao, Christopher Lameter,
David Rientjes, Kirill Tkhai, Mel Gorman, Michael Ellerman,
Michal Hocko, Nathan Lynch, Pekka Enberg,
PUVICHAKRAVARTHY RAMACHANDRAN, Sachin Sant, Srikar Dronamraju
On Tue, 24 Mar 2020 14:00:52 +0100 Vlastimil Babka <vbabka@suse.cz> wrote:
> On 3/20/20 12:55 PM, Vlastimil Babka wrote:
> > This reverts commit ad2c8144418c6a81cefe65379fd47bbe8344cef2.
> >
> > The function node_to_mem_node() was introduced by that commit for use in SLUB
> > on systems with memoryless nodes, but it turned out to be unreliable on some
> > architectures/configurations and a simpler solution exists than fixing it up.
> >
> > Thus the previous commit "mm, slub: prevent kmalloc_node crashes and memory
> > leaks" removed the only user of node_to_mem_node() and we can revert the commit
> > that introduced it.
>
> Since the bugfix was fast-tracked to Linus, we can now reference it properly and
> replace the second paragraph with:
>
> Thus commit 0715e6c516f1 ("mm, slub: prevent kmalloc_node crashes and memory
> leaks") removed the only user of node_to_mem_node() and we can revert the commit
> that introduced the function.
I updated the changelog, thanks.
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2020-03-26 0:54 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-20 11:55 [PATCH 1/2] mm, slub: prevent kmalloc_node crashes and memory leaks Vlastimil Babka
2020-03-20 11:55 ` Vlastimil Babka
2020-03-20 11:55 ` [PATCH 2/2] Revert "topology: add support for node_to_mem_node() to determine the fallback node" Vlastimil Babka
2020-03-20 13:12 ` Srikar Dronamraju
2020-03-24 13:00 ` Vlastimil Babka
2020-03-26 0:54 ` Andrew Morton
2020-03-20 13:10 ` [PATCH 1/2] mm, slub: prevent kmalloc_node crashes and memory leaks Srikar Dronamraju
2020-03-20 13:10 ` Srikar Dronamraju
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.