Re: [PATCH] mm: page_alloc: fix watermark check in __zone_watermark_ok()

[Kyungmin Park <kmpark@infradead.org>]

[-- Attachment #1: Type: text/plain, Size: 3399 bytes --]

On Sat, May 25, 2013 at 5:23 AM, Laura Abbott <lauraa@codeaurora.org> wrote:

> On 5/9/2013 12:50 AM, Tomasz Stanislawski wrote:
>
>> The watermark check consists of two sub-checks.
>> The first one is:
>>
>>         if (free_pages <= min + lowmem_reserve)
>>                 return false;
>>
>> The check assures that there is minimal amount of RAM in the zone.  If
>> CMA is
>> used then the free_pages is reduced by the number of free pages in CMA
>> prior
>> to the over-mentioned check.
>>
>>         if (!(alloc_flags & ALLOC_CMA))
>>                 free_pages -= zone_page_state(z, NR_FREE_CMA_PAGES);
>>
>> This prevents the zone from being drained from pages available for
>> non-movable
>> allocations.
>>
>> The second check prevents the zone from getting too fragmented.
>>
>>         for (o = 0; o < order; o++) {
>>                 free_pages -= z->free_area[o].nr_free << o;
>>                 min >>= 1;
>>                 if (free_pages <= min)
>>                         return false;
>>         }
>>
>> The field z->free_area[o].nr_free is equal to the number of free pages
>> including free CMA pages.  Therefore the CMA pages are subtracted twice.
>>  This
>> may cause a false positive fail of __zone_watermark_ok() if the CMA area
>> gets
>> strongly fragmented.  In such a case there are many 0-order free pages
>> located
>> in CMA. Those pages are subtracted twice therefore they will quickly drain
>> free_pages during the check against fragmentation.  The test fails even
>> though
>> there are many free non-cma pages in the zone.
>>
>> This patch fixes this issue by subtracting CMA pages only for a purpose of
>> (free_pages <= min + lowmem_reserve) check.
>>
>> Signed-off-by: Tomasz Stanislawski <t.stanislaws@samsung.com>
>> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
>> ---
>>   mm/page_alloc.c |    6 ++++--
>>   1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
>> index 8fcced7..0d4fef2 100644
>> --- a/mm/page_alloc.c
>> +++ b/mm/page_alloc.c
>> @@ -1626,6 +1626,7 @@ static bool __zone_watermark_ok(struct zone *z, int
>> order, unsigned long mark,
>>         long min = mark;
>>         long lowmem_reserve = z->lowmem_reserve[classzone_**idx];
>>         int o;
>> +       long free_cma = 0;
>>
>>         free_pages -= (1 << order) - 1;
>>         if (alloc_flags & ALLOC_HIGH)
>> @@ -1635,9 +1636,10 @@ static bool __zone_watermark_ok(struct zone *z,
>> int order, unsigned long mark,
>>   #ifdef CONFIG_CMA
>>         /* If allocation can't use CMA areas don't use free CMA pages */
>>         if (!(alloc_flags & ALLOC_CMA))
>> -               free_pages -= zone_page_state(z, NR_FREE_CMA_PAGES);
>> +               free_cma = zone_page_state(z, NR_FREE_CMA_PAGES);
>>   #endif
>> -       if (free_pages <= min + lowmem_reserve)
>> +
>> +       if (free_pages - free_cma <= min + lowmem_reserve)
>>                 return false;
>>         for (o = 0; o < order; o++) {
>>                 /* At the next order, this order's pages become
>> unavailable */
>>
>>
> I haven't seen any response to this patch but it has been of some benefit
> to some of our use cases. You're welcome to add
>
> Tested-by: Laura Abbott <lauraa@codeaurora.org>
>

Thanks Laura,
We already got mail from Andrew, it's merged mm tree.

Thank you,
Kyungmin Park


>
> if the patch hasn't been  picked up yet.
>
>

[-- Attachment #2: Type: text/html, Size: 4664 bytes --]