Kernel panic due to page migration accessing memory holes

[Michael Bohan]

On 2/18/2010 2:04 AM, Mel Gorman wrote:
> On Thu, Feb 18, 2010 at 06:36:04PM +0900, KAMEZAWA Hiroyuki wrote:
>    
>>   [Fact]
>>   - There are 2 banks of memory and a memory hole on your machine.
>>     As
>>           0x00200000 - 0x07D00000
>>           0x40000000 - 0x43000000
>>
>>   - Each bancks are in the same zone.
>>   - You use FLATMEM.
>>   - You see panic in move_freepages().
>>   - Your host's MAX_ORDER=11....buddy allocator's alignment is 0x400000
>>     Then, it seems 1st bank is not algined.
>>      
> It's not and assumptions are made about it being aligned.
>    

Would it be prudent to have the ARM mm init code detect unaligned, 
discontiguous banks and print a warning message if 
CONFIG_ARCH_HAS_HOLES_MEMORYMODEL is not configured?  Should we take it 
a step further and even BUG()?

> ARM frees unused portions of memmap to save memory. It's why memmap_valid_within()
> exists when CONFIG_ARCH_HAS_HOLES_MEMORYMODEL although previously only
> reading /proc/pagetypeinfo cared.
>
> In that case, the FLATMEM memory map had unexpected holes which "never"
> happens and that was the workaround. The problem here is that there are
> unaligned zones but no pfn_valid() implementation that can identify
> them as you'd have with SPARSEMEM. My expectation is that you are using
> the pfn_valid() implementation from asm-generic
>
> #define pfn_valid(pfn)          ((pfn)<  max_mapnr)
>
> which is insufficient in your case.
>    

I am actually using the pfn_valid implementation FLATMEM in 
arch/arm/include/asm/memory.h.  This one is very similar to the 
asm-generic, and has no knowledge of the holes.

> I think it's more likely the at the memmap he is accessing has been
> freed and is effectively random data.
>
>    

I also think this is the case.

> SPARSEMEM would give you an implementation of pfn_valid() that you could
> use here. The choices that spring to mind are;
>
> 1. reduce MAX_ORDER so they are aligned (easiest)
>    

Is it safe to assume that reducing MAX_ORDER will hurt performance?

> 2. use SPARSEMEM (easy, but not necessary what you want to do, might
> 	waste memory unless you drop MAX_ORDER as well)
>    

We intend to use SPARSEMEM, but we'd also like to maintain FLATMEM 
compatibility for some configurations.  My guess is that there are other 
ARM users that may want this support as well.

> 3. implement a pfn_valid() that can handle the holes and set
> 	CONFIG_HOLES_IN_ZONE so it's called in move_freepages() to
> 	deal with the holes (should pass this by someone more familiar
> 	with ARM than I)
>    

This option seems the best to me.  We should be able to implement an ARM 
specific pfn_valid() that walks the ARM meminfo struct to ensure the pfn 
is not within a hole.

My only concern with this is a comment in __rmqueue_fallback() after 
calling move_freepages_block()  that states "Claim the whole block if 
over half of it is free".  Suppose only 1 MB is beyond the bank limit.  
That means that over half of the pages of the 4 MB block will be 
reported by move_freepages() as free -- but 1 MB of those pages are 
invalid.  Won't this cause problems if these pages are assumed to be 
part of an active block?

It seems like we should have an additional check in 
move_freepages_block() with pfn_valid_within() to check the last page in 
the block (eg. end_pfn) before calling move_freepages_block().  If the 
last page is not valid, then we shouldn't we return 0 as in the zone 
span check?  This will also skip the extra burden of checking each 
individual page, when we already know the proposed range is invalid.

Assuming we did return 0 in this case, would that sub-block of pages 
ever be usable for anything else, or would it be effectively wasted?  If 
this memory were wasted, then adjusting MAX_ORDER would have an 
advantage in this sense -- ignoring any performance implications.

Thanks,
Michael