Re: [Bugme-new] [Bug 36192] New: Kernel panic when boot the 2.6.39+ kernel based off of 2.6.32 kernel

[Mel Gorman <mgorman@suse.de>]

On Tue, Jun 07, 2011 at 06:06:30PM +0900, KAMEZAWA Hiroyuki wrote:
> On Tue, 7 Jun 2011 10:03:13 +0100
> Mel Gorman <mgorman@suse.de> wrote:
> 
> > On Tue, Jun 07, 2011 at 09:57:08AM +0900, KAMEZAWA Hiroyuki wrote:
> > > On Mon, 6 Jun 2011 14:45:19 -0700
> > > Andrew Morton <akpm@linux-foundation.org> wrote:
> > > 
> > > > Hopefully he can test this one for us as well, thanks.
> > > > 
> > > 
> > > A  patch with better description (of mine) is here.
> > > Anyway, I felt I needed a fix for ARM special case.
> > > 
> > > ==
> > > fix-init-page_cgroup-for-sparsemem-taking-care-of-broken-page-flags.patch
> > > Even with SPARSEMEM, there are some magical memmap.
> > > 
> > 
> > Who wants to introduce SPARSEMEM_MAGICAL?
> > 
> 
> ARM guys ;)
> 
> > > If a Node is not aligned to SECTION, memmap of pfn which is out of
> > > Node's range is not initialized. And page->flags contains 0.
> > > 
> > 
> > This is tangential but it might be worth introducing
> > CONFIG_DEBUG_MEMORY_MODEL that WARN_ONs page->flag == 0 in
> > pfn_to_page() to catch some accesses outside node boundaries. Not for
> > this bug though.
> > 
> 
> Hmm, buf if zone == 0 && section == 0 && nid == 0, page->flags is 0.
> 

Sorry, what I meant to suggest was that page->flags outside of
boundaries be initialised to a poison value that is an impossible
combination of flags and check that.

> > > If Node(0) doesn't exist, NODE_DATA(pfn_to_nid(pfn)) causes error.
> > > 
> > 
> > Well, not in itself. It causes a bug when we try allocate memory
> > from node 0 but there is a subtle performance bug here as well. For
> > unaligned nodes, the cgroup information can be allocated from node
> > 0 instead of node-local.
> > 
> > > In another case, for example, ARM frees memmap which is never be used
> > > even under SPARSEMEM. In that case, page->flags will contain broken
> > > value.
> > > 
> > 
> > Again, not as such. In that case, struct page is not valid memory
> > at all.
> 
> Hmm, IIUC, ARM's code frees memmap by free_bootmem().....so, memory used 
> for 'struct page' is valid and can access (but it's not struct page.)
> 
> If my English sounds strange, I'm sorry. Hm
> 
> How about this ?
> == 
>  In another case, for example, ARM frees memmap which is never be used
>  and reuse memory for memmap for other purpose. So, in that case,
>  a page got by pfn_to_page(pfn) may not a struct page.
> ==
> 

Much better.

> 
> 
> > 
> > > This patch does a strict check on nid which is obtained by
> > > pfn_to_page() and use proper NID for page_cgroup allocation.
> > > 
> > > Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> > > 
> > > ---
> > >  mm/page_cgroup.c |   36 +++++++++++++++++++++++++++++++++++-
> > >  1 file changed, 35 insertions(+), 1 deletion(-)
> > > 
> > > Index: linux-3.0-rc1/mm/page_cgroup.c
> > > ===================================================================
> > > --- linux-3.0-rc1.orig/mm/page_cgroup.c
> > > +++ linux-3.0-rc1/mm/page_cgroup.c
> > > @@ -168,6 +168,7 @@ static int __meminit init_section_page_c
> > >  	struct mem_section *section;
> > >  	unsigned long table_size;
> > >  	unsigned long nr;
> > > +	unsigned long tmp;
> > >  	int nid, index;
> > >  
> > >  	nr = pfn_to_section_nr(pfn);
> > > @@ -175,8 +176,41 @@ static int __meminit init_section_page_c
> > >  
> > >  	if (section->page_cgroup)
> > >  		return 0;
> > > +	/*
> > > +	 * check Node-ID. Because we get 'pfn' which is obtained by calculation,
> > > +	 * the pfn may "not exist" or "alreay freed". Even if pfn_valid() returns
> > > +	 * true, page->flags may contain broken value and pfn_to_nid() returns
> > > +	 * bad value.
> > > +	 * (See CONFIG_ARCH_HAS_HOLES_MEMORYMODEL and ARM's free_memmap())
> > > +	 * So, we need to do careful check, here.
> > > +	 */
> > 
> > You don't really need to worry about ARM here as long as you stay
> > within node boundaries and you only care about the first valid page
> > in the node. Why not lookup NODE_DATA(nid) and make sure start and
> > end are within the node boundaries?
> > 
> 
> I thought ARM's code just takes care of MAX_ORDER alignment..

Which is not the same as section alignment and whatever alignment it's
using, the start of the node is still going to be valid.

-- 
Mel Gorman
SUSE Labs

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  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:"dont@kvack.org"> email@kvack.org </a>