Re: [PATCH v2 2/8] mm/page_alloc: correct to clear guard attribute in DEBUG_PAGEALLOC

From: Joonsoo Kim
Date: Wed Aug 13 2014 - 04:21:06 EST


On Tue, Aug 12, 2014 at 01:45:23AM +0000, Minchan Kim wrote:
> On Wed, Aug 06, 2014 at 04:18:30PM +0900, Joonsoo Kim wrote:
> > In __free_one_page(), we check the buddy page if it is guard page.
> > And, if so, we should clear guard attribute on the buddy page. But,
> > currently, we clear original page's order rather than buddy one's.
> > This doesn't have any problem, because resetting buddy's order
> > is useless and the original page's order is re-assigned soon.
> > But, it is better to correct code.
> >
> > Additionally, I change (set/clear)_page_guard_flag() to
> > (set/clear)_page_guard() and makes these functions do all works
> > needed for guard page. This may make code more understandable.
> >
> > One more thing, I did in this patch, is that fixing freepage accounting.
> > If we clear guard page and link it onto isolate buddy list, we should
> > not increase freepage count.
>
> You are saying just "shouldn't do that" but don't say "why" and "result"
> I know the reason but as you know, I'm one of the person who is rather
> familiar with this part but I guess others should spend some time to get.
> Kind detail description is never to look down on person. :)

Hmm. In fact, the reason is already mentioned in cover letter, but,
it is better to write it here.

Will do.

> >
>
> Nice catch, Joonsoo! But what make me worry is is this patch makes 3 thing
> all at once.
>
> 1. fix - no candidate for stable
> 2. clean up
> 3. fix - candidate for stable.
>
> Could you separate 3 and (1,2) in next spin?
>

Okay!

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