Re: [PATCH, V2] i386: instead of poisoning .init zone, change protection bits to force a fault

[Eric Dumazet <dada1@cosmosbay.com>]

Andrew Morton a écrit :
> Eric Dumazet <dada1@cosmosbay.com> wrote:
> > > --- devel/arch/i386/mm/init.c~i386-instead-of-poisoning-init-zone-change-protection-fix	2006-02-04 14:33:33.000000000 -0800
> >  > +++ devel-akpm/arch/i386/mm/init.c	2006-02-04 14:34:07.000000000 -0800
> >  > @@ -751,11 +751,15 @@ void free_init_pages(char *what, unsigne
> >  >  		ClearPageReserved(virt_to_page(addr));
> >  >  		set_page_count(virt_to_page(addr), 1);
> >  >  #ifdef CONFIG_DEBUG_INITDATA
> >  > +		/*
> >  > +		 * Unmap the page, and leak it.  So any further accesses will
> >  > +		 * oops.
> >  > +		 */
> >  >  		change_page_attr(virt_to_page(addr), 1, __pgprot(0));
> >  >  #else
> >  >  		memset((void *)addr, 0xcc, PAGE_SIZE);
> >  > -#endif
> >  >  		free_page(addr);
> >  > +#endif
> >  >  		totalram_pages++;
> >  >  	}
> >  >  	printk(KERN_INFO "Freeing %s: %ldk freed\n", what, (end - begin) >> 10);
> >  > _
> >  > 
> >  > But is there much point in doing this?  Does it offer much more than
> >  > CONFIG_DEBUG_PAGEALLOC?
> >  > 
> >  > 
> >
> >  1) CONFIG_DEBUG_PAGEALLOC is very generic (and expensive), while 
> >  CONFIG_DEBUG_INITDATA only makes sure init data becomes unreadable.
> >
> >  2) If CONFIG_DEBUG_INITDATA is on, the redzone is in action in the virtual 
> >  memory that hold the initdata, while the physical pages that contained the 
> >  initdata where freed and might be reused for  other needs.
> >
> >  I think we have two different things here : Virtual mem redzoning (my patch), 
> >  and physical ram poisoning (your patch).
> >
> >  CONFIG_DEBUG_INITDATA uses only a virtual mem redzoning (no underlying memory 
> >  cost, apart of the page tables), while your solution doesnt free the pages, 
> >  and the poisoining wont catch further accesses, just make some results funny 
> >  or false.
> >
> >  The only bad effect of my patch is about the TLB cost, because of the 
> >  hugepage(s) that should revert to 512 normal 4K pages.
>
> Your patch made my kernel oops!  The oops was prevented by either enabling
> CONFIG_DEBUG_PAGEALLOC or by the above patch.

Oh I got it now ! Sorry for being stupid :(

If pages are freed, they indeed can be reused by kmalloc() and we get page 
faults...
I wrongly assumed these pages would be mapped to different virtual addresses.

So your patch is necessary, unless we can free the pages and let them be used 
only for User context for example.

Thank you
Eric

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/