sparsemem/bootmem: catch greater than section size allocations
Subsystems (3)
PATCH 1/1 sparsemem/bootmem: catch greater than section size allocations
While testing AMS (Active Memory Sharing) / CMO (Cooperative Memory Overcommit) on powerpc, we tripped the following:
kernel BUG at mm/bootmem.c:483!
cpu 0x0: Vector: 700 (Program Check) at [c000000000c03940]
pc: c000000000a62bd8: .alloc_bootmem_core+0x90/0x39c
lr: c000000000a64bcc: .sparse_early_usemaps_alloc_node+0x84/0x29c
sp: c000000000c03bc0 msr: 8000000000021032
current = 0xc000000000b0cce0
paca = 0xc000000001d80000
pid = 0, comm = swapper
kernel BUG at mm/bootmem.c:483!
enter ? for help
[c000000000c03c80] c000000000a64bcc
.sparse_early_usemaps_alloc_node+0x84/0x29c [c000000000c03d50] c000000000a64f10 .sparse_init+0x12c/0x28c [c000000000c03e20] c000000000a474f4 .setup_arch+0x20c/0x294 [c000000000c03ee0] c000000000a4079c .start_kernel+0xb4/0x460 [c000000000c03f90] c000000000009670 .start_here_common+0x1c/0x2c
This is
BUG_ON(limit && goal + size > limit);
and after some debugging, it seems that
goal = 0x7ffff000000 limit = 0x80000000000
and sparse_early_usemaps_alloc_node -> sparse_early_usemaps_alloc_pgdat_section -> alloc_bootmem_section calls
return alloc_bootmem_section(usemap_size() * count, section_nr);
This is on a system with 8TB available via the AMS pool, and as a quirk of AMS in firmware, all of that memory shows up in node 0. So, we end up with an allocation that will fail the goal/limit constraints. In theory, we could "fall-back" to alloc_bootmem_node() in sparse_early_usemaps_alloc_node(), but since we actually have HOTREMOVE defined, we'll BUG_ON() instead. A simple solution appears to be to disable the limit check if the size of the allocation in alloc_bootmem_secition exceeds the section size.
--- include/linux/mmzone.h | 2 ++ mm/bootmem.c | 5 ++++- 2 files changed, 6 insertions(+), 1 deletions(-)
include/linux/mmzone.h +2 -0
@@ -967,6 +967,8 @@ static inline unsigned long early_pfn_to_nid(unsigned long pfn)* PA_SECTION_SHIFT physical address to/from section number * PFN_SECTION_SHIFT pfn to/from section number */ +#define BYTES_PER_SECTION (1UL << SECTION_SIZE_BITS) +#define SECTIONS_SHIFT (MAX_PHYSMEM_BITS - SECTION_SIZE_BITS)#define PA_SECTION_SHIFT (SECTION_SIZE_BITS)
mm/bootmem.c +4 -1
@@ -770,7 +770,10 @@ void * __init alloc_bootmem_section(unsigned long size, pfn =section_nr_to_pfn (section_nr); goal = pfn <<PAGE_SHIFT ; - limit =section_nr_to_pfn (section_nr +1 ) <<PAGE_SHIFT ; +if (size >BYTES_PER_SECTION ) + limit =0 ; +else + limit =section_nr_to_pfn (section_nr +1 ) <<PAGE_SHIFT ; bdata = &bootmem_node_data[early_pfn_to_nid (pfn)];return alloc_bootmem_core (bdata, size,SMP_CACHE_BYTES , goal, limit);
Signature · 8 lines
1.7.5.4 -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Quoted 35 lines · Nishanth Aravamudan
While testing AMS (Active Memory Sharing) / CMO (Cooperative Memory Overcommit) on powerpc, we tripped the following:
kernel BUG at
mm/bootmem.c:483!
cpu0x0: Vector: 700 (Program Check) at [c000000000c03940]pc: c000000000a62bd8: .alloc_bootmem_core+0x90/0x39c lr: c000000000a64bcc: .sparse_early_usemaps_alloc_node+0x84/0x29c sp: c000000000c03bc0msr: 8000000000021032
current =0xc000000000b0cce0
paca =0xc000000001d80000
pid = 0, comm = swapper
kernel BUG atmm/bootmem.c:483!
enter ? for help
[c000000000c03c80]c000000000a64bcc.sparse_early_usemaps_alloc_node+0x84/0x29c [c000000000c03d50] c000000000a64f10 .sparse_init+0x12c/0x28c [c000000000c03e20] c000000000a474f4 .setup_arch+0x20c/0x294 [c000000000c03ee0] c000000000a4079c .start_kernel+0xb4/0x460 [c000000000c03f90] c000000000009670 .start_here_common+0x1c/0x2cThis is
BUG_ON(limit && goal + size > limit);and after some debugging, it seems that
goal = 0x7ffff000000 limit = 0x80000000000and
sparse_early_usemaps_alloc_node->sparse_early_usemaps_alloc_pgdat_section->alloc_bootmem_sectioncallsreturn alloc_bootmem_section(usemap_size() * count, section_nr);
Quoted 8 lines
This is on a system with 8TB available via the AMS pool, and as a quirk of AMS in firmware, all of that memory shows up in node 0. So, we end up with an allocation that will fail the goal/limit constraints. In theory, we could "fall-back" to
alloc_bootmem_node()insparse_early_usemaps_alloc_node(), but since we actually have HOTREMOVE defined, we'llBUG_ON()instead. A simple solution appears to be to disable the limit check if the size of the allocation inalloc_bootmem_secitionexceeds the section size.
It makes sense to allow the usemaps to spill over to subsequent sections instead of panicking, so FWIW:
That being said, it would be good if check_usemap_section_nr() printed the cross-dependencies between pgdats and sections when the usemaps of a node spilled over to other sections than the ones holding the pgdat.
How about this?
---
From: Johannes Weiner <[email protected]>
Subject: sparsemem/bootmem: catch greater than section size allocations fix
If alloc_bootmem_section() no longer guarantees section-locality, we need check_usemap_section_nr() to print possible cross-dependencies between node descriptors and the usemaps allocated through it.
---
mm/sparse.c +1 -0
@@ -359,6 +359,7 @@ static void __init sparse_early_usemaps_alloc_node(unsigned long**usemap_map,continue ; usemap_map[pnum] = usemap; usemap += size; +check_usemap_section_nr (nodeid, usemap_map[pnum]); }return ; }
---
Furthermore, I wonder if we can remove the sparse-specific stuff from bootmem.c as well, as now even more so than before, calculating the desired area is really none of bootmem's business.
Would something like this be okay?
---
From: Johannes Weiner <[email protected]>
Subject: [patch] mm: remove sparsemem allocation details from the bootmem allocator
alloc_bootmem_section() derives allocation area constraints from the specified sparsemem section. This is a bit specific for a generic memory allocator like bootmem, though, so move it over to sparsemem.
Since __alloc_bootmem_node() already retries failed allocations with relaxed area constraints, the fallback code in sparsemem.c can be removed and the code becomes a bit more compact overall.
--- include/linux/bootmem.h | 3 --- mm/bootmem.c | 26 -------------------------- mm/sparse.c | 29 +++++++++-------------------- 3 files changed, 9 insertions(+), 49 deletions(-)
include/linux/bootmem.h +0 -3
@@ -135,9 +135,6 @@ extern void *__alloc_bootmem_low_node(pg_data_t *pgdat,extern int reserve_bootmem_generic (unsigned long addr,unsigned long size,int flags); -extern void *alloc_bootmem_section (unsigned long size, -unsigned long section_nr); -#ifdef CONFIG_HAVE_ARCH_ALLOC_REMAP extern void *alloc_remap (int nid,unsigned long size);#else
mm/bootmem.c +0 -26
@@ -756,32 +756,6 @@ void * __init __alloc_bootmem_node_high(pg_data_t *pgdat, unsigned long size, } -#ifdef CONFIG_SPARSEMEM -/** -* alloc_bootmem_section - allocate boot memory from a specific section -* @size: size of the request in bytes -* @section_nr: sparse map section to allocate from -* -* Return NULL on failure. -*/ -void *__init alloc_bootmem_section (unsigned long size, -unsigned long section_nr) -{ -bootmem_data_t *bdata; -unsigned long pfn, goal, limit; - - pfn =section_nr_to_pfn (section_nr); - goal = pfn <<PAGE_SHIFT ; -if (size >BYTES_PER_SECTION ) - limit =0 ; -else - limit =section_nr_to_pfn (section_nr +1 ) <<PAGE_SHIFT ; - bdata = &bootmem_node_data[early_pfn_to_nid (pfn)]; - -return alloc_bootmem_core (bdata, size,SMP_CACHE_BYTES , goal, limit); -} -#endif -void *__init __alloc_bootmem_node_nopanic (pg_data_t *pgdat,unsigned long size,unsigned long align,unsigned long goal) {
mm/sparse.c +9 -20
@@ -273,10 +273,10 @@ static unsigned long *__kmalloc_section_usemap(void)#ifdef CONFIG_MEMORY_HOTREMOVE static unsigned long *__init sparse_early_usemaps_alloc_pgdat_section (struct pglist_data *pgdat, -unsigned long count) +unsigned long size) { -unsigned long section_nr; - +pg_data_t *host_pgdat; +unsigned long goal;/* * A page may contain usemaps for other sections preventing the * page being freed and making a section unremovable while
@@ -287,8 +287,9 @@ sparse_early_usemaps_alloc_pgdat_section(struct pglist_data *pgdat,* from the same section as the pgdat where possible to avoid * this problem. */ - section_nr =pfn_to_section_nr (__pa (pgdat) >>PAGE_SHIFT ); -return alloc_bootmem_section (usemap_size () * count, section_nr); + goal =__pa (pgdat) &PAGE_SECTION_MASK ; + host_pgdat =NODE_DATA (early_pfn_to_nid (goal)); +return __alloc_bootmem_node (host_pgdat, size,SMP_CACHE_BYTES , goal); }static void __init check_usemap_section_nr (int nid,unsigned long *usemap)
@@ -332,9 +333,9 @@ static void __init check_usemap_section_nr(int nid, unsigned long *usemap)#else static unsigned long *__init sparse_early_usemaps_alloc_pgdat_section (struct pglist_data *pgdat, -unsigned long count) +unsigned long size) { -return NULL ; +return alloc_bootmem_node (pgdat, size); }static void __init check_usemap_section_nr (int nid,unsigned long *usemap)
@@ -352,19 +353,7 @@ static void __init sparse_early_usemaps_alloc_node(unsigned long**usemap_map,int size =usemap_size (); usemap =sparse_early_usemaps_alloc_pgdat_section (NODE_DATA (nodeid), - usemap_count); -if (usemap) { -for (pnum = pnum_begin; pnum < pnum_end; pnum++) { -if (!present_section_nr (pnum)) -continue ; - usemap_map[pnum] = usemap; - usemap += size; -check_usemap_section_nr (nodeid, usemap_map[pnum]); - } -return ; - } - - usemap =alloc_bootmem_node (NODE_DATA (nodeid), size * usemap_count); + size * usemap_count);if (usemap) {for (pnum = pnum_begin; pnum < pnum_end; pnum++) {if (!present_section_nr (pnum))
Signature · 8 lines
1.7.7.6 -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Quoted 72 lines · Johannes Weiner
Quoted 35 lines · Nishanth Aravamudan
While testing AMS (Active Memory Sharing) / CMO (Cooperative Memory Overcommit) on powerpc, we tripped the following:
kernel BUG at
mm/bootmem.c:483!
cpu0x0: Vector: 700 (Program Check) at [c000000000c03940]pc: c000000000a62bd8: .alloc_bootmem_core+0x90/0x39c lr: c000000000a64bcc: .sparse_early_usemaps_alloc_node+0x84/0x29c sp: c000000000c03bc0msr: 8000000000021032
current =0xc000000000b0cce0
paca =0xc000000001d80000
pid = 0, comm = swapper
kernel BUG atmm/bootmem.c:483!
enter ? for help
[c000000000c03c80]c000000000a64bcc.sparse_early_usemaps_alloc_node+0x84/0x29c [c000000000c03d50] c000000000a64f10 .sparse_init+0x12c/0x28c [c000000000c03e20] c000000000a474f4 .setup_arch+0x20c/0x294 [c000000000c03ee0] c000000000a4079c .start_kernel+0xb4/0x460 [c000000000c03f90] c000000000009670 .start_here_common+0x1c/0x2cThis is
BUG_ON(limit && goal + size > limit);and after some debugging, it seems that
goal = 0x7ffff000000 limit = 0x80000000000and
sparse_early_usemaps_alloc_node->sparse_early_usemaps_alloc_pgdat_section->alloc_bootmem_sectioncallsreturn alloc_bootmem_section(usemap_size() * count, section_nr);Quoted 8 lines
This is on a system with 8TB available via the AMS pool, and as a quirk of AMS in firmware, all of that memory shows up in node 0. So, we end up with an allocation that will fail the goal/limit constraints. In theory, we could "fall-back" to
alloc_bootmem_node()insparse_early_usemaps_alloc_node(), but since we actually have HOTREMOVE defined, we'llBUG_ON()instead. A simple solution appears to be to disable the limit check if the size of the allocation inalloc_bootmem_secitionexceeds the section size.It makes sense to allow the usemaps to spill over to subsequent sections instead of panicking, so FWIW:
Acked-by Johannes Weiner <[email protected]>That being said, it would be good if
check_usemap_section_nr()printed the cross-dependencies between pgdats and sections when the usemaps of a node spilled over to other sections than the ones holding the pgdat.How about this?
---
From: Johannes Weiner <[email protected]>
Subject: sparsemem/bootmem: catch greater than section size allocations fixIf
alloc_bootmem_section()no longer guarantees section-locality, we needcheck_usemap_section_nr()to print possible cross-dependencies between node descriptors and the usemaps allocated through it.Signed-off-by Johannes Weiner <[email protected]>---
mm/sparse.c +0
-0
mm/sparse.c static void __init sparse_early_usemaps_alloc_node(unsigned long**usemap_map, · line 365
Quoted 7 lines
mm/sparse.c +1
-0@@ -359,6 +359,7 @@ static void __init sparse_early_usemaps_alloc_node(unsigned long**usemap_map,continue ; usemap_map[pnum] = usemap; usemap += size; +check_usemap_section_nr (nodeid, usemap_map[pnum]); }return ; }
This makes sense to me -- ok if I fold it into the re-worked patch (based upon Mel's comments)?
Quoted 16 lines
---
Furthermore, I wonder if we can remove the sparse-specific stuff from
bootmem.cas well, as now even more so than before, calculating the desired area is really none of bootmem's business.Would something like this be okay?
---
From: Johannes Weiner <[email protected]>
Subject: [patch] mm: remove sparsemem allocation details from the bootmem allocator
alloc_bootmem_section()derives allocation area constraints from the specified sparsemem section. This is a bit specific for a generic memory allocator like bootmem, though, so move it over to sparsemem.
Quoted 5 lines
Since
__alloc_bootmem_node()already retries failed allocations with relaxed area constraints, the fallback code insparsemem.ccan be removed and the code becomes a bit more compact overall.Signed-off-by Johannes Weiner <[email protected]>
I've not tested it, but the intention seems sensible. I think it should remain a separate change.
Thanks,
Nish
Signature · 9 lines
Nishanth Aravamudan <[email protected]> IBM Linux Technology Center -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Quoted 68 lines · Nishanth Aravamudan
Quoted 67 lines · Johannes Weiner
Quoted 35 lines · Nishanth Aravamudan
While testing AMS (Active Memory Sharing) / CMO (Cooperative Memory Overcommit) on powerpc, we tripped the following:
kernel BUG at
mm/bootmem.c:483!
cpu0x0: Vector: 700 (Program Check) at [c000000000c03940]pc: c000000000a62bd8: .alloc_bootmem_core+0x90/0x39c lr: c000000000a64bcc: .sparse_early_usemaps_alloc_node+0x84/0x29c sp: c000000000c03bc0msr: 8000000000021032
current =0xc000000000b0cce0
paca =0xc000000001d80000
pid = 0, comm = swapper
kernel BUG atmm/bootmem.c:483!
enter ? for help
[c000000000c03c80]c000000000a64bcc.sparse_early_usemaps_alloc_node+0x84/0x29c [c000000000c03d50] c000000000a64f10 .sparse_init+0x12c/0x28c [c000000000c03e20] c000000000a474f4 .setup_arch+0x20c/0x294 [c000000000c03ee0] c000000000a4079c .start_kernel+0xb4/0x460 [c000000000c03f90] c000000000009670 .start_here_common+0x1c/0x2cThis is
BUG_ON(limit && goal + size > limit);and after some debugging, it seems that
goal = 0x7ffff000000 limit = 0x80000000000and
sparse_early_usemaps_alloc_node->sparse_early_usemaps_alloc_pgdat_section->alloc_bootmem_sectioncallsreturn alloc_bootmem_section(usemap_size() * count, section_nr);Quoted 8 lines
This is on a system with 8TB available via the AMS pool, and as a quirk of AMS in firmware, all of that memory shows up in node 0. So, we end up with an allocation that will fail the goal/limit constraints. In theory, we could "fall-back" to
alloc_bootmem_node()insparse_early_usemaps_alloc_node(), but since we actually have HOTREMOVE defined, we'llBUG_ON()instead. A simple solution appears to be to disable the limit check if the size of the allocation inalloc_bootmem_secitionexceeds the section size.It makes sense to allow the usemaps to spill over to subsequent sections instead of panicking, so FWIW:
Acked-by Johannes Weiner <[email protected]>That being said, it would be good if
check_usemap_section_nr()printed the cross-dependencies between pgdats and sections when the usemaps of a node spilled over to other sections than the ones holding the pgdat.How about this?
---
From: Johannes Weiner <[email protected]>
Subject: sparsemem/bootmem: catch greater than section size allocations fixIf
alloc_bootmem_section()no longer guarantees section-locality, we needcheck_usemap_section_nr()to print possible cross-dependencies between node descriptors and the usemaps allocated through it.Signed-off-by Johannes Weiner <[email protected]>---
Quoted 15 lines
mm/sparse.c static void __init sparse_early_usemaps_alloc_node(unsigned long**usemap_map, · line 365
Quoted 12 lines
mm/sparse.c +1
-0@@ -359,6 +359,7 @@ static void __init sparse_early_usemaps_alloc_node(unsigned long**usemap_map,continue ; usemap_map[pnum] = usemap; usemap += size; +check_usemap_section_nr (nodeid, usemap_map[pnum]); }return ; }This makes sense to me -- ok if I fold it into the re-worked patch (based upon Mel's comments)?
Sure thing!
Quoted 14 lines
Quoted 14 lines
Furthermore, I wonder if we can remove the sparse-specific stuff from
bootmem.cas well, as now even more so than before, calculating the desired area is really none of bootmem's business.Would something like this be okay?
---
From: Johannes Weiner <[email protected]>
Subject: [patch] mm: remove sparsemem allocation details from the bootmem allocator
alloc_bootmem_section()derives allocation area constraints from the specified sparsemem section. This is a bit specific for a generic memory allocator like bootmem, though, so move it over to sparsemem.
Quoted 8 lines
Quoted 5 lines
Since
__alloc_bootmem_node()already retries failed allocations with relaxed area constraints, the fallback code insparsemem.ccan be removed and the code becomes a bit more compact overall.Signed-off-by Johannes Weiner <[email protected]>I've not tested it, but the intention seems sensible. I think it should remain a separate change.
Yes, I agree. I'll resend it in a bit as stand-alone patch.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Quoted 75 lines · Nishanth Aravamudan
While testing AMS (Active Memory Sharing) / CMO (Cooperative Memory Overcommit) on powerpc, we tripped the following:
kernel BUG at
mm/bootmem.c:483!
cpu0x0: Vector: 700 (Program Check) at [c000000000c03940]pc: c000000000a62bd8: .alloc_bootmem_core+0x90/0x39c lr: c000000000a64bcc: .sparse_early_usemaps_alloc_node+0x84/0x29c sp: c000000000c03bc0msr: 8000000000021032
current =0xc000000000b0cce0
paca =0xc000000001d80000
pid = 0, comm = swapper
kernel BUG atmm/bootmem.c:483!
enter ? for help
[c000000000c03c80]c000000000a64bcc.sparse_early_usemaps_alloc_node+0x84/0x29c [c000000000c03d50] c000000000a64f10 .sparse_init+0x12c/0x28c [c000000000c03e20] c000000000a474f4 .setup_arch+0x20c/0x294 [c000000000c03ee0] c000000000a4079c .start_kernel+0xb4/0x460 [c000000000c03f90] c000000000009670 .start_here_common+0x1c/0x2cThis is
BUG_ON(limit && goal + size > limit);and after some debugging, it seems that
goal = 0x7ffff000000 limit = 0x80000000000and
sparse_early_usemaps_alloc_node->sparse_early_usemaps_alloc_pgdat_section->alloc_bootmem_sectioncallsreturn alloc_bootmem_section(usemap_size() * count, section_nr);This is on a system with 8TB available via the AMS pool, and as a quirk of AMS in firmware, all of that memory shows up in node 0. So, we end up with an allocation that will fail the goal/limit constraints. In theory, we could "fall-back" to
alloc_bootmem_node()insparse_early_usemaps_alloc_node(), but since we actually have HOTREMOVE defined, we'llBUG_ON()instead. A simple solution appears to be to disable the limit check if the size of the allocation inalloc_bootmem_secitionexceeds the section size.Signed-off-by Nishanth Aravamudan <[email protected]>Cc Dave Hansen <[email protected]>Cc Anton Blanchard <[email protected]>Cc Paul Mackerras <[email protected]>Cc Ben Herrenschmidt <[email protected]>Cc Robert Jennings <[email protected]>--- include/linux/mmzone.h | 2 ++ mm/bootmem.c | 5 ++++- 2 files changed, 6 insertions(+), 1 deletions(-)include/linux/mmzone.h +2
-0@@ -967,6 +967,8 @@ static inline unsigned long early_pfn_to_nid(unsigned long pfn)* PA_SECTION_SHIFT physical address to/from section number * PFN_SECTION_SHIFT pfn to/from section number */ +#define BYTES_PER_SECTION (1UL << SECTION_SIZE_BITS) +#define SECTIONS_SHIFT (MAX_PHYSMEM_BITS - SECTION_SIZE_BITS)#define PA_SECTION_SHIFT (SECTION_SIZE_BITS)mm/bootmem.c +0
-0
mm/bootmem.c void * __init alloc_bootmem_section(unsigned long size, · line 776
Quoted 8 lines
mm/bootmem.c +4
-1@@ -770,7 +770,10 @@ void * __init alloc_bootmem_section(unsigned long size, pfn =section_nr_to_pfn (section_nr); goal = pfn <<PAGE_SHIFT ; - limit =section_nr_to_pfn (section_nr +1 ) <<PAGE_SHIFT ; +if (size >BYTES_PER_SECTION ) + limit =0 ; +else + limit =section_nr_to_pfn (section_nr +1 ) <<PAGE_SHIFT ;
As it's ok to spill the allocation over to an adjacent section, why not just make limit==0 unconditionally. That would avoid defining BYTES_PER_SECTION.
Signature · 9 lines
Mel Gorman SUSE Labs -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
PATCH v2 1/1 bootmem/sparsemem: remove limit constraint in alloc_bootmem_section
Quoted 77 lines · Mel Gorman
Quoted 76 lines · Nishanth Aravamudan
While testing AMS (Active Memory Sharing) / CMO (Cooperative Memory Overcommit) on powerpc, we tripped the following:
kernel BUG at
mm/bootmem.c:483!
cpu0x0: Vector: 700 (Program Check) at [c000000000c03940]pc: c000000000a62bd8: .alloc_bootmem_core+0x90/0x39c lr: c000000000a64bcc: .sparse_early_usemaps_alloc_node+0x84/0x29c sp: c000000000c03bc0msr: 8000000000021032
current =0xc000000000b0cce0
paca =0xc000000001d80000
pid = 0, comm = swapper
kernel BUG atmm/bootmem.c:483!
enter ? for help
[c000000000c03c80]c000000000a64bcc.sparse_early_usemaps_alloc_node+0x84/0x29c [c000000000c03d50] c000000000a64f10 .sparse_init+0x12c/0x28c [c000000000c03e20] c000000000a474f4 .setup_arch+0x20c/0x294 [c000000000c03ee0] c000000000a4079c .start_kernel+0xb4/0x460 [c000000000c03f90] c000000000009670 .start_here_common+0x1c/0x2cThis is
BUG_ON(limit && goal + size > limit);and after some debugging, it seems that
goal = 0x7ffff000000 limit = 0x80000000000and
sparse_early_usemaps_alloc_node->sparse_early_usemaps_alloc_pgdat_section->alloc_bootmem_sectioncallsreturn alloc_bootmem_section(usemap_size() * count, section_nr);This is on a system with 8TB available via the AMS pool, and as a quirk of AMS in firmware, all of that memory shows up in node 0. So, we end up with an allocation that will fail the goal/limit constraints. In theory, we could "fall-back" to
alloc_bootmem_node()insparse_early_usemaps_alloc_node(), but since we actually have HOTREMOVE defined, we'llBUG_ON()instead. A simple solution appears to be to disable the limit check if the size of the allocation inalloc_bootmem_secitionexceeds the section size.Signed-off-by Nishanth Aravamudan <[email protected]>Cc Dave Hansen <[email protected]>Cc Anton Blanchard <[email protected]>Cc Paul Mackerras <[email protected]>Cc Ben Herrenschmidt <[email protected]>Cc Robert Jennings <[email protected]>--- include/linux/mmzone.h | 2 ++ mm/bootmem.c | 5 ++++- 2 files changed, 6 insertions(+), 1 deletions(-)include/linux/mmzone.h +2
-0@@ -967,6 +967,8 @@ static inline unsigned long early_pfn_to_nid(unsigned long pfn)* PA_SECTION_SHIFT physical address to/from section number * PFN_SECTION_SHIFT pfn to/from section number */ +#define BYTES_PER_SECTION (1UL << SECTION_SIZE_BITS) +#define SECTIONS_SHIFT (MAX_PHYSMEM_BITS - SECTION_SIZE_BITS)#define PA_SECTION_SHIFT (SECTION_SIZE_BITS)mm/bootmem.c +0
-0@@ -770,7 +770,10 @@ void * __init alloc_bootmem_section(unsigned long size,
Quoted 11 lines
mm/bootmem.c void * __init alloc_bootmem_section(unsigned long size, · line 776
Quoted 7 lines
mm/bootmem.c +4
-1pfn =section_nr_to_pfn (section_nr); goal = pfn <<PAGE_SHIFT ; - limit =section_nr_to_pfn (section_nr +1 ) <<PAGE_SHIFT ; +if (size >BYTES_PER_SECTION ) + limit =0 ; +else + limit =section_nr_to_pfn (section_nr +1 ) <<PAGE_SHIFT ;As it's ok to spill the allocation over to an adjacent section, why not just make limit==0 unconditionally. That would avoid defining
BYTES_PER_SECTION.
Something like this?
Andrew, presuming Mel & Johannes give their, ack this should presumably supersede the patch you pulled into -mm.
Thanks,
Nish
-------
While testing AMS (Active Memory Sharing) / CMO (Cooperative Memory Overcommit) on powerpc, we tripped the following:
kernel BUG at mm/bootmem.c:483!
cpu 0x0: Vector: 700 (Program Check) at [c000000000c03940]
pc: c000000000a62bd8: .alloc_bootmem_core+0x90/0x39c
lr: c000000000a64bcc: .sparse_early_usemaps_alloc_node+0x84/0x29c
sp: c000000000c03bc0 msr: 8000000000021032
current = 0xc000000000b0cce0
paca = 0xc000000001d80000
pid = 0, comm = swapper
kernel BUG at mm/bootmem.c:483!
enter ? for help
[c000000000c03c80] c000000000a64bcc
.sparse_early_usemaps_alloc_node+0x84/0x29c [c000000000c03d50] c000000000a64f10 .sparse_init+0x12c/0x28c [c000000000c03e20] c000000000a474f4 .setup_arch+0x20c/0x294 [c000000000c03ee0] c000000000a4079c .start_kernel+0xb4/0x460 [c000000000c03f90] c000000000009670 .start_here_common+0x1c/0x2c
This is
BUG_ON(limit && goal + size > limit);
and after some debugging, it seems that
goal = 0x7ffff000000 limit = 0x80000000000
and sparse_early_usemaps_alloc_node ->sparse_early_usemaps_alloc_pgdat_section calls
return alloc_bootmem_section(usemap_size() * count, section_nr);
This is on a system with 8TB available via the AMS pool, and as a quirk of AMS in firmware, all of that memory shows up in node 0. So, we end up with an allocation that will fail the goal/limit constraints. In theory, we could "fall-back" to alloc_bootmem_node() in sparse_early_usemaps_alloc_node(), but since we actually have HOTREMOVE defined, we'll BUG_ON() instead. A simple solution appears to be to unconditionally remove the limit condition in alloc_bootmem_section, meaning allocations are allowed to cross section boundaries (necessary for systems of this size).
Johannes Weiner pointed out that if alloc_bootmem_section() no longer guarantees section-locality, we need check_usemap_section_nr() to print possible cross-dependencies between node descriptors and the usemaps allocated through it. That makes the two loops in sparse_early_usemaps_alloc_node() identical, so re-factor the code a bit.
---
v2: Unconditionally set limit to 0. Fold in Johannes' changes to sparse_early_usemaps_alloc_node.
mm/bootmem.c +1 -1
@@ -770,7 +770,7 @@ void * __init alloc_bootmem_section(unsigned long size, pfn =section_nr_to_pfn (section_nr); goal = pfn <<PAGE_SHIFT ; - limit =section_nr_to_pfn (section_nr +1 ) <<PAGE_SHIFT ; + limit =0 ; bdata = &bootmem_node_data[early_pfn_to_nid (pfn)];return alloc_bootmem_core (bdata, size,SMP_CACHE_BYTES , goal, limit);
mm/sparse.c +11 -19
@@ -353,29 +353,21 @@ static void __init sparse_early_usemaps_alloc_node(unsigned long**usemap_map, usemap =sparse_early_usemaps_alloc_pgdat_section (NODE_DATA (nodeid), usemap_count); -if (usemap) { -for (pnum = pnum_begin; pnum < pnum_end; pnum++) { -if (!present_section_nr (pnum)) -continue ; - usemap_map[pnum] = usemap; - usemap += size; +if (!usemap) { + usemap =alloc_bootmem_node (NODE_DATA (nodeid), size * usemap_count); +if (!usemap) { +printk (KERN_WARNING "%s: allocation failed\n" , __func__); +return ; } -return ; } - usemap =alloc_bootmem_node (NODE_DATA (nodeid), size * usemap_count); -if (usemap) { -for (pnum = pnum_begin; pnum < pnum_end; pnum++) { -if (!present_section_nr (pnum)) -continue ; - usemap_map[pnum] = usemap; - usemap += size; -check_usemap_section_nr (nodeid, usemap_map[pnum]); - } -return ; +for (pnum = pnum_begin; pnum < pnum_end; pnum++) { +if (!present_section_nr (pnum)) +continue ; + usemap_map[pnum] = usemap; + usemap += size; +check_usemap_section_nr (nodeid, usemap_map[pnum]); } - -printk (KERN_WARNING "%s: allocation failed\n" , __func__); }#ifndef CONFIG_SPARSEMEM_VMEMMAP
Signature · 2 lines
Nishanth Aravamudan <[email protected]> IBM Linux Technology Center
Re: [PATCH v2] bootmem/sparsemem: remove limit constraint in alloc_bootmem_section
Quoted 145 lines · Nishanth Aravamudan
Quoted 77 lines · Mel Gorman
Quoted 76 lines · Nishanth Aravamudan
While testing AMS (Active Memory Sharing) / CMO (Cooperative Memory Overcommit) on powerpc, we tripped the following:
kernel BUG at
mm/bootmem.c:483!
cpu0x0: Vector: 700 (Program Check) at [c000000000c03940]pc: c000000000a62bd8: .alloc_bootmem_core+0x90/0x39c lr: c000000000a64bcc: .sparse_early_usemaps_alloc_node+0x84/0x29c sp: c000000000c03bc0msr: 8000000000021032
current =0xc000000000b0cce0
paca =0xc000000001d80000
pid = 0, comm = swapper
kernel BUG atmm/bootmem.c:483!
enter ? for help
[c000000000c03c80]c000000000a64bcc.sparse_early_usemaps_alloc_node+0x84/0x29c [c000000000c03d50] c000000000a64f10 .sparse_init+0x12c/0x28c [c000000000c03e20] c000000000a474f4 .setup_arch+0x20c/0x294 [c000000000c03ee0] c000000000a4079c .start_kernel+0xb4/0x460 [c000000000c03f90] c000000000009670 .start_here_common+0x1c/0x2cThis is
BUG_ON(limit && goal + size > limit);and after some debugging, it seems that
goal = 0x7ffff000000 limit = 0x80000000000and
sparse_early_usemaps_alloc_node->sparse_early_usemaps_alloc_pgdat_section->alloc_bootmem_sectioncallsreturn alloc_bootmem_section(usemap_size() * count, section_nr);This is on a system with 8TB available via the AMS pool, and as a quirk of AMS in firmware, all of that memory shows up in node 0. So, we end up with an allocation that will fail the goal/limit constraints. In theory, we could "fall-back" to
alloc_bootmem_node()insparse_early_usemaps_alloc_node(), but since we actually have HOTREMOVE defined, we'llBUG_ON()instead. A simple solution appears to be to disable the limit check if the size of the allocation inalloc_bootmem_secitionexceeds the section size.Signed-off-by Nishanth Aravamudan <[email protected]>Cc Dave Hansen <[email protected]>Cc Anton Blanchard <[email protected]>Cc Paul Mackerras <[email protected]>Cc Ben Herrenschmidt <[email protected]>Cc Robert Jennings <[email protected]>--- include/linux/mmzone.h | 2 ++ mm/bootmem.c | 5 ++++- 2 files changed, 6 insertions(+), 1 deletions(-)include/linux/mmzone.h +2
-0@@ -967,6 +967,8 @@ static inline unsigned long early_pfn_to_nid(unsigned long pfn)* PA_SECTION_SHIFT physical address to/from section number * PFN_SECTION_SHIFT pfn to/from section number */ +#define BYTES_PER_SECTION (1UL << SECTION_SIZE_BITS) +#define SECTIONS_SHIFT (MAX_PHYSMEM_BITS - SECTION_SIZE_BITS)#define PA_SECTION_SHIFT (SECTION_SIZE_BITS)mm/bootmem.c +0
-0@@ -770,7 +770,10 @@ void * __init alloc_bootmem_section(unsigned long size,Quoted 11 lines
mm/bootmem.c void * __init alloc_bootmem_section(unsigned long size,
Quoted 7 lines
mm/bootmem.c +4
-1pfn =section_nr_to_pfn (section_nr); goal = pfn <<PAGE_SHIFT ; - limit =section_nr_to_pfn (section_nr +1 ) <<PAGE_SHIFT ; +if (size >BYTES_PER_SECTION ) + limit =0 ; +else + limit =section_nr_to_pfn (section_nr +1 ) <<PAGE_SHIFT ;As it's ok to spill the allocation over to an adjacent section, why not just make limit==0 unconditionally. That would avoid defining
BYTES_PER_SECTION.Something like this?
Andrew, presuming Mel & Johannes give their, ack this should presumably supersede the patch you pulled into -mm.
Thanks,
Nish-------
While testing AMS (Active Memory Sharing) / CMO (Cooperative Memory Overcommit) on powerpc, we tripped the following:
kernel BUG at
mm/bootmem.c:483!
cpu0x0: Vector: 700 (Program Check) at [c000000000c03940]pc: c000000000a62bd8: .alloc_bootmem_core+0x90/0x39c lr: c000000000a64bcc: .sparse_early_usemaps_alloc_node+0x84/0x29c sp: c000000000c03bc0msr: 8000000000021032
current =0xc000000000b0cce0
paca =0xc000000001d80000
pid = 0, comm = swapper
kernel BUG atmm/bootmem.c:483!
enter ? for help
[c000000000c03c80]c000000000a64bcc.sparse_early_usemaps_alloc_node+0x84/0x29c [c000000000c03d50] c000000000a64f10 .sparse_init+0x12c/0x28c [c000000000c03e20] c000000000a474f4 .setup_arch+0x20c/0x294 [c000000000c03ee0] c000000000a4079c .start_kernel+0xb4/0x460 [c000000000c03f90] c000000000009670 .start_here_common+0x1c/0x2cThis is
BUG_ON(limit && goal + size > limit);and after some debugging, it seems that
goal = 0x7ffff000000 limit = 0x80000000000and
sparse_early_usemaps_alloc_node->sparse_early_usemaps_alloc_pgdat_sectioncallsreturn alloc_bootmem_section(usemap_size() * count, section_nr);This is on a system with 8TB available via the AMS pool, and as a quirk of AMS in firmware, all of that memory shows up in node 0. So, we end up with an allocation that will fail the goal/limit constraints. In theory, we could "fall-back" to
alloc_bootmem_node()insparse_early_usemaps_alloc_node(), but since we actually have HOTREMOVE defined, we'llBUG_ON()instead. A simple solution appears to be to unconditionally remove the limit condition inalloc_bootmem_section, meaning allocations are allowed to cross section boundaries (necessary for systems of this size).
Quoted 8 lines
Johannes Weiner pointed out that if
alloc_bootmem_section()no longer guarantees section-locality, we needcheck_usemap_section_nr()to print possible cross-dependencies between node descriptors and the usemaps allocated through it. That makes the two loops insparse_early_usemaps_alloc_node()identical, so re-factor the code a bit.Signed-off-by Nishanth Aravamudan <[email protected]>
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Andrew Morton
maintainerRe: [PATCH v2] bootmem/sparsemem: remove limit constraint in alloc_bootmem_section
In reply to Nishanth AravamudanQuoted 31 lines · On Wed, 29 Feb 2012 10:12:33 -0800 Nishanth Aravamudan…
While testing AMS (Active Memory Sharing) / CMO (Cooperative Memory Overcommit) on powerpc, we tripped the following:
kernel BUG at
mm/bootmem.c:483!...
This is
BUG_ON(limit && goal + size > limit);and after some debugging, it seems that
goal = 0x7ffff000000 limit = 0x80000000000and
sparse_early_usemaps_alloc_node->sparse_early_usemaps_alloc_pgdat_sectioncallsreturn alloc_bootmem_section(usemap_size() * count, section_nr);This is on a system with 8TB available via the AMS pool, and as a quirk of AMS in firmware, all of that memory shows up in node 0. So, we end up with an allocation that will fail the goal/limit constraints. In theory, we could "fall-back" to
alloc_bootmem_node()insparse_early_usemaps_alloc_node(), but since we actually have HOTREMOVE defined, we'llBUG_ON()instead. A simple solution appears to be to unconditionally remove the limit condition inalloc_bootmem_section, meaning allocations are allowed to cross section boundaries (necessary for systems of this size).
Quoted 6 lines
Johannes Weiner pointed out that if
alloc_bootmem_section()no longer guarantees section-locality, we needcheck_usemap_section_nr()to print possible cross-dependencies between node descriptors and the usemaps allocated through it. That makes the two loops insparse_early_usemaps_alloc_node()identical, so re-factor the code a bit.
The patch is a bit scary now, so I think we should merge it into 3.4-rc1 and then backport it into 3.3.1 if nothing blows up.
Do you think it should be backported into 3.3.x? Earlier kernels?
Also, this?
mm/bootmem.c renamed from mm/bootmem.c~bootmem-sparsemem-remove-limit-constraint-in-alloc_bootmem_section-fix +2 -3
@@ -766,14 +766,13 @@ void * __init alloc_bootmem_section(unsi unsigned long section_nr) {bootmem_data_t *bdata; -unsigned long pfn, goal, limit; +unsigned long pfn, goal; pfn =section_nr_to_pfn (section_nr); goal = pfn <<PAGE_SHIFT ; - limit =0 ; bdata = &bootmem_node_data[early_pfn_to_nid (pfn)]; -return alloc_bootmem_core (bdata, size,SMP_CACHE_BYTES , goal, limit); +return alloc_bootmem_core (bdata, size,SMP_CACHE_BYTES , goal,0 ); }#endif
_
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Nishanth Aravamudan
Acked-byRe: [PATCH v2] bootmem/sparsemem: remove limit constraint in alloc_bootmem_section
Quoted 41 lines · Andrew Morton
Quoted 31 lines · On Wed, 29 Feb 2012 10:12:33 -0800 Nishanth Aravamudan…
While testing AMS (Active Memory Sharing) / CMO (Cooperative Memory Overcommit) on powerpc, we tripped the following:
kernel BUG at
mm/bootmem.c:483!...
This is
BUG_ON(limit && goal + size > limit);and after some debugging, it seems that
goal = 0x7ffff000000 limit = 0x80000000000and
sparse_early_usemaps_alloc_node->sparse_early_usemaps_alloc_pgdat_sectioncallsreturn alloc_bootmem_section(usemap_size() * count, section_nr);This is on a system with 8TB available via the AMS pool, and as a quirk of AMS in firmware, all of that memory shows up in node 0. So, we end up with an allocation that will fail the goal/limit constraints. In theory, we could "fall-back" to
alloc_bootmem_node()insparse_early_usemaps_alloc_node(), but since we actually have HOTREMOVE defined, we'llBUG_ON()instead. A simple solution appears to be to unconditionally remove the limit condition inalloc_bootmem_section, meaning allocations are allowed to cross section boundaries (necessary for systems of this size).Quoted 6 lines
Johannes Weiner pointed out that if
alloc_bootmem_section()no longer guarantees section-locality, we needcheck_usemap_section_nr()to print possible cross-dependencies between node descriptors and the usemaps allocated through it. That makes the two loops insparse_early_usemaps_alloc_node()identical, so re-factor the code a bit.
Quoted 2 lines
The patch is a bit scary now, so I think we should merge it into 3.4-rc1 and then backport it into 3.3.1 if nothing blows up.
I think that's fair.
Quoted 1 line
Do you think it should be backported into 3.3.x? Earlier kernels?
3.3.x seems reasonable. If I had to guess, I think this could be hit on any kernels with this functionality -- that is, sparsemem in general? Not sure how far back it's worth backporting.
Quoted 1 line
Also, this?
Urgh, yeah, that's way better.
Quoted 8 lines
mm/bootmem.c renamed from mm/bootmem.c~bootmem-sparsemem-remove-limit-constraint-in-alloc_bootmem_section-fix +1
-1@@ -766,14 +766,13 @@ void * __init alloc_bootmem_section(unsi unsigned long section_nr) {bootmem_data_t *bdata; -unsigned long pfn, goal, limit; +unsigned long pfn, goal;
mm/bootmem.c void * __init alloc_bootmem_section(unsi · line 777
Quoted 10 lines
mm/bootmem.c renamed from mm/bootmem.c~bootmem-sparsemem-remove-limit-constraint-in-alloc_bootmem_section-fix +1
-2pfn =section_nr_to_pfn (section_nr); goal = pfn <<PAGE_SHIFT ; - limit =0 ; bdata = &bootmem_node_data[early_pfn_to_nid (pfn)]; - return alloc_bootmem_core(bdata, size, SMP_CACHE_BYTES, goal, limit); + return alloc_bootmem_core(bdata, size, SMP_CACHE_BYTES, goal, 0); }#endif
Thanks for all the feedback!
-Nish
Signature · 2 lines
Nishanth Aravamudan <[email protected]> IBM Linux Technology Center
Re: [PATCH v2] bootmem/sparsemem: remove limit constraint in alloc_bootmem_section
In reply to Andrew MortonQuoted 41 lines · Andrew Morton
Quoted 31 lines · On Wed, 29 Feb 2012 10:12:33 -0800 Nishanth Aravamudan…
While testing AMS (Active Memory Sharing) / CMO (Cooperative Memory Overcommit) on powerpc, we tripped the following:
kernel BUG at
mm/bootmem.c:483!...
This is
BUG_ON(limit && goal + size > limit);and after some debugging, it seems that
goal = 0x7ffff000000 limit = 0x80000000000and
sparse_early_usemaps_alloc_node->sparse_early_usemaps_alloc_pgdat_sectioncallsreturn alloc_bootmem_section(usemap_size() * count, section_nr);This is on a system with 8TB available via the AMS pool, and as a quirk of AMS in firmware, all of that memory shows up in node 0. So, we end up with an allocation that will fail the goal/limit constraints. In theory, we could "fall-back" to
alloc_bootmem_node()insparse_early_usemaps_alloc_node(), but since we actually have HOTREMOVE defined, we'llBUG_ON()instead. A simple solution appears to be to unconditionally remove the limit condition inalloc_bootmem_section, meaning allocations are allowed to cross section boundaries (necessary for systems of this size).Quoted 6 lines
Johannes Weiner pointed out that if
alloc_bootmem_section()no longer guarantees section-locality, we needcheck_usemap_section_nr()to print possible cross-dependencies between node descriptors and the usemaps allocated through it. That makes the two loops insparse_early_usemaps_alloc_node()identical, so re-factor the code a bit.
Quoted 4 lines
The patch is a bit scary now, so I think we should merge it into 3.4-rc1 and then backport it into 3.3.1 if nothing blows up.
Do you think it should be backported into 3.3.x? Earlier kernels?
Upon review, it would be good if we can get it pushed back to kernels 3.0.x, 3.1.x and 3.2.x.
Thanks,
Nish
Signature · 9 lines
Nishanth Aravamudan <[email protected]> IBM Linux Technology Center -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Re: [PATCH v2] bootmem/sparsemem: remove limit constraint in alloc_bootmem_section
In reply to Nishanth AravamudanQuoted 4 lines · Nishanth Aravamudan
<SNIP>
Signed-off-by Nishanth Aravamudan <[email protected]>
Signature · 9 lines
Mel Gorman SUSE Labs -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"[email protected]"> [email protected] </a>