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

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

[Tomasz Stanislawski]

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

--
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: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

[Laura Abbott]

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>

if the patch hasn't been  picked up yet.

Thanks,
Laura
-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

--
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: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

[Kyungmin Park]

[-- 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 --]

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

[Andrew Morton]

On Sat, 25 May 2013 13:32:02 +0900 Kyungmin Park <kmpark@infradead.org> wrote:

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

Yes, but I have it scheduled for 3.11 with no -stable backport.

This is because the patch changelog didn't tell me about the
userspace-visible impact of the bug.  Judging from Laura's comments, this
was a mistake.

So please: details.  What problems were observable to Laura and do we
think this bug should be fixed in 3.10 and earlier?

--
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: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

[Marek Szyprowski]

Hello,

On 5/30/2013 12:08 AM, Andrew Morton wrote:
> On Sat, 25 May 2013 13:32:02 +0900 Kyungmin Park <kmpark@infradead.org> wrote:
>
> > > 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.
>
> Yes, but I have it scheduled for 3.11 with no -stable backport.
>
> This is because the patch changelog didn't tell me about the
> userspace-visible impact of the bug.  Judging from Laura's comments, this
> was a mistake.
>
> So please: details.  What problems were observable to Laura and do we
> think this bug should be fixed in 3.10 and earlier?

This patch fixes issue introduced in commit 
d95ea5d18e699515468368415c93ed49b1a3221b,
so if possible, I suggest to get it backported to v3.7-v3.10 releases as 
well.

Best regards
-- 
Marek Szyprowski
Samsung R&D Institute Poland


--
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: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

[Laura Abbott]

On 5/29/2013 3:08 PM, Andrew Morton wrote:
> On Sat, 25 May 2013 13:32:02 +0900 Kyungmin Park <kmpark@infradead.org> wrote:
>
>>> 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.
>
> Yes, but I have it scheduled for 3.11 with no -stable backport.
>
> This is because the patch changelog didn't tell me about the
> userspace-visible impact of the bug.  Judging from Laura's comments, this
> was a mistake.
>
> So please: details.  What problems were observable to Laura and do we
> think this bug should be fixed in 3.10 and earlier?
>

We were observing allocation failures of higher order pages (order 5 = 
128K typically) under tight memory conditions resulting in driver 
failure. The output from the page allocation failure showed plenty of 
free pages of the appropriate order/type/zone and mostly CMA pages in 
the lower orders.

For full disclosure, we still observed some page allocation failures 
even after applying the patch but the number was drastically reduced and 
those failures were attributed to fragmentation/other system issues.

Thanks,
Laura

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

--
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: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

[Minchan Kim]

Hello,

On Thu, May 09, 2013 at 09:50:46AM +0200, 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 description is rather confusing.
It was not to prevent the zone from being drained but for considering a right
number of allocatable pages from the zone because !ALLOC_CMA pages couldn't be
allocated from CMA migrate type from the zone so if we didn't have such patch,
zone_watermark_ok could return true but allocation couldn't allocate a page.

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

Right.

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

How about this?

  The !ALLOC_CMA higher order allocation fails even though there are many
  free high order pages in the zone.

This patch should go to stable so I'd like to include description with detail
numbers of zone's stat to describe the situation when the higher allocation
is failed so stable maintainers and other distro maintainers could parse the
problem easily.

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

Otherwise, looks good to me.

Acked-by: Minchan Kim <minchan@kernel.org>

-- 
Kind regards,
Minchan Kim

--
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: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>