Message-ID: <50B3858D.2060404@linux.vnet.ibm.com>
Date: Mon, 26 Nov 2012 07:06:53 -0800
From: Dave Hansen <dave@linux.vnet.ibm.com>
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:16.0) Gecko/20121028 Thunderbird/16.0.2
MIME-Version: 1.0
To: Mel Gorman <mgorman@suse.de>
CC: akpm@osdl.org, linux-kernel@vger.kernel.org, torvalds@linux-foundation.org,
        linux-mm@kvack.org
Subject: Re: [PATCH] mm: compaction: Fix return value of capture_free_page
References: <20121121192151.3FFE0A9A@kernel.stglabs.ibm.com> <20121126112350.GI8218@suse.de>
In-Reply-To: <20121126112350.GI8218@suse.de>
Content-Type: text/plain; charset=ISO-8859-15
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1291
Lines: 33

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.

We should probably also fix the set_pageblock_migratetype() loop in
there while we're at it.  I think it's potentially trampling on the
migration type of pages currently in the allocator.  I _think_ that
completes the list of things that need to get audited in there.

--
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/