From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from psmtp.com (na3sys010amx122.postini.com [74.125.245.122]) by kanga.kvack.org (Postfix) with SMTP id 0C7C76B0032 for ; Sat, 25 May 2013 00:32:02 -0400 (EDT) Received: by mail-ie0-f172.google.com with SMTP id 16so14216234iea.17 for ; Fri, 24 May 2013 21:32:02 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <519FCC46.2000703@codeaurora.org> References: <518B5556.4010005@samsung.com> <519FCC46.2000703@codeaurora.org> Date: Sat, 25 May 2013 13:32:02 +0900 Message-ID: Subject: Re: [PATCH] mm: page_alloc: fix watermark check in __zone_watermark_ok() From: Kyungmin Park Content-Type: multipart/alternative; boundary=089e01182f669ee7b204dd83659a Sender: owner-linux-mm@kvack.org List-ID: To: Laura Abbott Cc: Tomasz Stanislawski , linux-mm@kvack.org, Marek Szyprowski , Andrew Morton , minchan@kernel.org, mgorman@suse.de, 'Bartlomiej Zolnierkiewicz --089e01182f669ee7b204dd83659a Content-Type: text/plain; charset=ISO-8859-1 On Sat, May 25, 2013 at 5:23 AM, Laura Abbott 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 >> Signed-off-by: Kyungmin Park >> --- >> 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 > 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. > > --089e01182f669ee7b204dd83659a Content-Type: text/html; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable

On Sat, May 25, 2013 at 5:23 AM, Laura A= bbott <lauraa@codeaurora.org> wrote:
On 5/9/2013 12:50 AM, Tomasz Stanis= lawski wrote:
The watermark check consists of two sub-checks.
The first one is:

=A0 =A0 =A0 =A0 if (free_pages <=3D min + lowmem_reserve)
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 return false;

The check assures that there is minimal amount of RAM in the zone. =A0If CM= A is
used then the free_pages is reduced by the number of free pages in CMA prio= r
to the over-mentioned check.

=A0 =A0 =A0 =A0 if (!(alloc_flags & ALLOC_CMA))
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 free_pages -=3D zone_page_state(z, NR_FREE_= CMA_PAGES);

This prevents the zone from being drained from pages available for non-mova= ble
allocations.

The second check prevents the zone from getting too fragmented.

=A0 =A0 =A0 =A0 for (o =3D 0; o < order; o++) {
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 free_pages -=3D z->free_area[o].nr_free = << o;
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 min >>=3D 1;
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (free_pages <=3D min)
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 return false;
=A0 =A0 =A0 =A0 }

The field z->free_area[o].nr_free is equal to the number of free pages including free CMA pages. =A0Therefore the CMA pages are subtracted twice. = =A0This
may cause a false positive fail of __zone_watermark_ok() if the CMA area ge= ts
strongly fragmented. =A0In such a case there are many 0-order free pages lo= cated
in CMA. Those pages are subtracted twice therefore they will quickly drain<= br> free_pages during the check against fragmentation. =A0The test fails even t= hough
there are many free non-cma pages in the zone.

This patch fixes this issue by subtracting CMA pages only for a purpose of<= br> (free_pages <=3D min + lowmem_reserve) check.

Signed-off-by: Tomasz Stanislawski <t.stanislaws@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
=A0 mm/page_alloc.c | =A0 =A06 ++++--
=A0 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 o= rder, unsigned long mark,
=A0 =A0 =A0 =A0 long min =3D mark;
=A0 =A0 =A0 =A0 long lowmem_reserve =3D z->lowmem_reserve[classzone_<= /u>idx];
=A0 =A0 =A0 =A0 int o;
+ =A0 =A0 =A0 long free_cma =3D 0;

=A0 =A0 =A0 =A0 free_pages -=3D (1 << order) - 1;
=A0 =A0 =A0 =A0 if (alloc_flags & ALLOC_HIGH)
@@ -1635,9 +1636,10 @@ static bool __zone_watermark_ok(struct zone *z, int = order, unsigned long mark,
=A0 #ifdef CONFIG_CMA
=A0 =A0 =A0 =A0 /* If allocation can't use CMA areas don't use free= CMA pages */
=A0 =A0 =A0 =A0 if (!(alloc_flags & ALLOC_CMA))
- =A0 =A0 =A0 =A0 =A0 =A0 =A0 free_pages -=3D zone_page_state(z, NR_FREE_CM= A_PAGES);
+ =A0 =A0 =A0 =A0 =A0 =A0 =A0 free_cma =3D zone_page_state(z, NR_FREE_CMA_P= AGES);
=A0 #endif
- =A0 =A0 =A0 if (free_pages <=3D min + lowmem_reserve)
+
+ =A0 =A0 =A0 if (free_pages - free_cma <=3D min + lowmem_reserve)
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 return false;
=A0 =A0 =A0 =A0 for (o =3D 0; o < order; o++) {
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 /* At the next order, this order's page= s become unavailable */


I haven't seen any response to this patch but it has been of some benef= it to some of our use cases. You're welcome to add

Tested-by: Laura Abbott <lauraa@codeaurora.org>
=A0
Thanks Laura,
We already got mail from Andrew, it's merged = mm tree.
=A0
Thank you,
Kyungmin Park
=A0

if the patch hasn't been =A0picked up yet.
=A0
--089e01182f669ee7b204dd83659a-- -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: email@kvack.org