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

[Kyungmin Park <kmpark@xxxxxxxxxxxxx>]

On Sat, May 25, 2013 at 5:23 AM, Laura Abbott <lauraa@xxxxxxxxxxxxxx> 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@xxxxxxxxxxx>
Signed-off-by: Kyungmin Park <kyungmin.park@xxxxxxxxxxx>
---
  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@xxxxxxxxxxxxxx>

 

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.