lkml.org 
[lkml]   [2012]   [Nov]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] mm: compaction: Fix return value of capture_free_page
On Mon, Nov 26, 2012 at 07:06:53AM -0800, Dave Hansen wrote:
> On 11/26/2012 03:23 AM, Mel Gorman wrote:
> > On Wed, Nov 21, 2012 at 02:21:51PM -0500, Dave Hansen wrote:
> >>
> >> This needs to make it in before 3.7 is released.
> >>
> >
> > This is also required. Dave, can you double check? The surprise is that
> > this does not blow up very obviously.
> ...
> > @@ -1422,7 +1422,7 @@ int capture_free_page(struct page *page, int alloc_order, int migratetype)
> > }
> > }
> >
> > - return 1UL << order;
> > + return 1UL << alloc_order;
> > }
>
> compact_capture_page() only looks at the boolean return value out of
> capture_free_page(), so it wouldn't notice. split_free_page() does.
> But, when it calls capture_free_page(), order==alloc_order, so it
> wouldn't make a difference. So, there's probably no actual bug here,
> but it's certainly a wrong return value.
>

I don't think it is fine in this case.

isolate_freepages_block
isolated = split_free_page(page);
-> split_free_page
nr_pages = capture_free_page(page, order, 0);
-> capture_free_page (returns wrong value of too many pages)
return nr_pages;

so now isolate_freepages_block has the wrong value with nr_pages holding
a value for a larger number of pages than are really isolated and does
this

for (i = 0; i < isolated; i++) {
list_add(&page->lru, freelist);
page++;
}

so potentially that is now adding pages that are already on the buddy list
to the local free list and "fun" ensues.

--
Mel Gorman
SUSE Labs


\
 
 \ /
  Last update: 2012-11-27 12:41    [W:0.035 / U:1.044 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site