lkml.org 
[lkml]   [2013]   [Jul]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 1/2] mm: Restructure free-page stealing code and fix a bug
On 07/25/2013 08:40 AM, Wanpeng Li wrote:
> On Tue, Jul 23, 2013 at 12:18:06AM +0530, Srivatsa S. Bhat wrote:
>> The free-page stealing code in __rmqueue_fallback() is somewhat hard to
>> follow, and has an incredible amount of subtlety hidden inside!
>>
>> First off, there is a minor bug in the reporting of change-of-ownership of
>> pageblocks. Under some conditions, we try to move upto 'pageblock_nr_pages'
>> no. of pages to the preferred allocation list. But we change the ownership
>> of that pageblock to the preferred type only if we manage to successfully
>> move atleast half of that pageblock (or if page_group_by_mobility_disabled
>> is set).
>>
>> However, the current code ignores the latter part and sets the 'migratetype'
>> variable to the preferred type, irrespective of whether we actually changed
>> the pageblock migratetype of that block or not. So, the page_alloc_extfrag
>> tracepoint can end up printing incorrect info (i.e., 'change_ownership'
>> might be shown as 1 when it must have been 0).
>>
>> So fixing this involves moving the update of the 'migratetype' variable to
>> the right place. But looking closer, we observe that the 'migratetype' variable
>> is used subsequently for checks such as "is_migrate_cma()". Obviously the
>> intent there is to check if the *fallback* type is MIGRATE_CMA, but since we
>> already set the 'migratetype' variable to start_migratetype, we end up checking
>> if the *preferred* type is MIGRATE_CMA!!
>>
>> To make things more interesting, this actually doesn't cause a bug in practice,
>> because we never change *anything* if the fallback type is CMA.
>>
>> So, restructure the code in such a way that it is trivial to understand what
>> is going on, and also fix the above mentioned bug. And while at it, also add a
>> comment explaining the subtlety behind the migratetype used in the call to
>> expand().
>>
>
> Greate catch!
>

Thank you :-)

Regards,
Srivatsa S. Bhat



\
 
 \ /
  Last update: 2013-07-25 09:01    [W:0.056 / U:0.044 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site