arm: mm: Don't free prohibited memmap entries
Subsystems (2)
PATCH 1/1 arm: mm: Don't free prohibited memmap entries
From: Michael Bohan <[email protected]>
The VM subsystem assumes that there are valid memmap entries to the bank end aligned to MAX_ORDER_NR_PAGES. It will try and read these page structs, and so we cannot free any memmap entries that it may inspect.
--- arch/arm/mm/init.c | 19 +++++++++---------- 1 files changed, 9 insertions(+), 10 deletions(-)
arch/arm/mm/init.c +9 -10
@@ -532,28 +532,27 @@ static void __init free_unused_memmap_node(int node, struct meminfo *mi)unsigned int i;/* -* [FIXME] This relies on each bank being in address order. This -* may not be the case, especially if the user has provided the -* information on the command line. +* This relies on each bank being in address order. +* The banks are sorted previously in bootmem_init(). */ for_each_nodebank (i, mi, node) {struct membank *bank = &mi->bank[i]; bank_start =bank_pfn_start (bank); -if (bank_start < prev_bank_end) { -printk (KERN_ERR "MEM: unordered memory banks. " -"Not freeing memmap.\n" ); -break ; - }/* * If we had a previous bank, and there is a space * between the current bank and the previous, free it. */ -if (prev_bank_end && prev_bank_end != bank_start) +if (prev_bank_end && prev_bank_end < bank_start)free_memmap (node, prev_bank_end, bank_start); - prev_bank_end =bank_pfn_end (bank); +/* +* Align up here since the VM subsystem insists that the +* memmap entries are valid from the bank end aligned to +* MAX_ORDER_NR_PAGES. +*/ + prev_bank_end =ALIGN (bank_pfn_end (bank),MAX_ORDER_NR_PAGES ); } }
Signature · 1 line
1.6.2.3
PATCH 1/1 arm: mm: Don't free prohibited memmap entries
Quoted 6 lines · Michael Bohan
From: Michael Bohan <[email protected]>
The VM subsystem assumes that there are valid memmap entries to the bank end aligned to
MAX_ORDER_NR_PAGES. It will try and read these page structs, and so we cannot free any memmap entries that it may inspect.
1. are you enabling ARCH_HAS_HOLES_MEMORYMODEL ?
2. where does it try to access these page structs without trying pfn_valid() to check whether a page struct exists first?
PATCH 1/1 arm: mm: Don't free prohibited memmap entries
Quoted 2 lines · Russell King - ARM Linux
1. are you enabling
ARCH_HAS_HOLES_MEMORYMODEL?
Yes, although this does not impact the problem I'm dealing with. That option is only used for /proc/pagetypeinfo currently. It would be good if we could consolidate ARCH_HAS_HOLES_MEMORYMODEL and HOLES_IN_ZONE, but that may be out of scope for this change.
Quoted 3 lines
2. where does it try to access these page structs without trying
pfn_valid()to check whether a page struct exists first?
The specific piece of code that is causing crashes in my scenario is in vm/page_alloc.c:move_freepages(), called from move_freepages_block(). The code in move_freepages_block aligns the end_pfn to the closest page block, which may take us to invalid memmap entries.
The macro that conditionally saves us in this case is the pfn_valid_within(), called from move_freepages(). If HOLES_IN_ZONE is configured, this option calls down to pfn_valid() to make sure the page has a valid memmap entry. There are likely other cases where this is an issue as well that I haven't run into.
Michael
PATCH 1/1 arm: mm: Don't free prohibited memmap entries
Quoted 18 lines · Michael Bohan
Quoted 2 lines · Russell King - ARM Linux
1. are you enabling
ARCH_HAS_HOLES_MEMORYMODEL?Yes, although this does not impact the problem I'm dealing with. That option is only used for
/proc/pagetypeinfocurrently. It would be good if we could consolidateARCH_HAS_HOLES_MEMORYMODELandHOLES_IN_ZONE, but that may be out of scope for this change.Quoted 3 lines
2. where does it try to access these page structs without trying
pfn_valid()to check whether a page struct exists first?The specific piece of code that is causing crashes in my scenario is in vm/page_alloc.c:move_freepages(), called from
move_freepages_block(). The code inmove_freepages_blockaligns theend_pfnto the closest page block, which may take us to invalid memmap entries.
Quoted 5 lines
The macro that conditionally saves us in this case is the
pfn_valid_within(), called frommove_freepages(). IfHOLES_IN_ZONEis configured, this option calls down topfn_valid()to make sure the page has a valid memmap entry. There are likely other cases where this is an issue as well that I haven't run into.
Well, there's two ways to look at this - either you should ensure that memory is available up to MAX_ORDER_NR_PAGES, possibly reducing this number of that's necessary to achive this.
Or, we need to have ARCH_HAS_HOLES_MEMORYMODEL needs to select HOLES_IN_ZONE so we get proper checking of PFNs - maybe conditional on MSM. I think most users of ARCH_HAS_HOLES_MEMORYMODEL align memory to a power-of-two amount of memory, and so MAX_ORDER_NR_PAGES doesn't cause them a problem.
PATCH 1/1 arm: mm: Don't free prohibited memmap entries
Quoted 32 lines · Russell King - ARM Linux
Quoted 20 lines · Michael Bohan
Quoted 3 lines · Russell King - ARM Linux
1. are you enabling
ARCH_HAS_HOLES_MEMORYMODEL?Yes, although this does not impact the problem I'm dealing with. That option is only used for
/proc/pagetypeinfocurrently. It would be good if we could consolidateARCH_HAS_HOLES_MEMORYMODELandHOLES_IN_ZONE, but that may be out of scope for this change.Quoted 4 lines
2. where does it try to access these page structs without trying
pfn_valid()to check whether a page struct exists first?The specific piece of code that is causing crashes in my scenario is in vm/page_alloc.c:move_freepages(), called from
move_freepages_block(). The code inmove_freepages_blockaligns theend_pfnto the closest page block, which may take us to invalid memmap entries.Quoted 6 lines
The macro that conditionally saves us in this case is the
pfn_valid_within(), called frommove_freepages(). IfHOLES_IN_ZONEis configured, this option calls down topfn_valid()to make sure the page has a valid memmap entry. There are likely other cases where this is an issue as well that I haven't run into.Well, there's two ways to look at this - either you should ensure that memory is available up to
MAX_ORDER_NR_PAGES, possibly reducing this number of that's necessary to achive this.
Quoted 6 lines
Or, we need to have
ARCH_HAS_HOLES_MEMORYMODELneeds to selectHOLES_IN_ZONEso we get proper checking of PFNs - maybe conditional on MSM. I think most users ofARCH_HAS_HOLES_MEMORYMODELalign memory to a power-of-two amount of memory, and soMAX_ORDER_NR_PAGESdoesn't cause them a problem.
The solution proposed in this patch does not include HOLES_IN_ZONE or ARCH_HAS_HOLES_MEMORYMODEL. I proposed HOLES_IN_ZONE in a previous patch, but per Mel's suggestion here we simply don't free any necessary memmap entries that are required by the VM subsystem. This has the following benefits:
-No extra run time overhead.
-No extra memory used for platforms with memory hole end addresses aligned to MAX_ORDER_NR_PAGES. This means that there should be no impact at all for users who are not currently running into this problem. For users that don't align to MAX_ORDER_NR_PAGES, there would be a loss of 8KB, 16KB, or 24KB of memory per memory hole (assuming MAX_ORDER == 11), depending on where their hole end address lands. This is somewhat undesirable, but better than losing the full 1MB, 2MB, or 3MB, respectfully, that would occur to achieve alignment. Some platforms are not flexible with respect to their memory map.
I'd also like to point out that this problem is not limited to MSM. Any platform in the future that changes its memory map in such a fashion will likely run into this bug.
Thanks,
Michael