[PATCH 0/3] misc fix around vmscan/isolate_lru_pages

[PATCH 0/3] misc fix around vmscan/isolate_lru_pages

[KAMEZAWA Hiroyuki]

Thank you for all helps. I finally wrote 3 small pathces.

All 3 patches are for vmscan.c::isolate_lru_pages().

[1/3] ... remove unnecessary/wrong lru rotation in lumpy reclaim.
[2/3] ... check PG_unevictable at lumpy reclaim
[3/3] ... fix memcg's lru rotation logic.

All 3 are just for fixes and don't do any other.
I'll revisit this area while working for memcg.

Thanks,
-Kame

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

[PATCH 1/3] remove wrong rotation at lumpy reclaim

[KAMEZAWA Hiroyuki]

From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>

At lumpy reclaim, a page failed to be taken by __isolate_lru_page() can
be pushed back to "src" list by list_move(). But the page may not be from
"src" list. And list_move() itself is unnecessary because the page is
not on top of LRU. Then, leave it as it is if __isolate_lru_page() fails.

This patch doesn't change the logic as "we should exit loop or not" and
just fixes buggy list_move().

Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
---
 mm/vmscan.c |    9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

Index: lumpy-reclaim-trial/mm/vmscan.c
===================================================================
--- lumpy-reclaim-trial.orig/mm/vmscan.c
+++ lumpy-reclaim-trial/mm/vmscan.c
@@ -936,18 +936,11 @@ static unsigned long isolate_lru_pages(u
 			/* Check that we have not crossed a zone boundary. */
 			if (unlikely(page_zone_id(cursor_page) != zone_id))
 				continue;
-			switch (__isolate_lru_page(cursor_page, mode, file)) {
-			case 0:
+			if (__isolate_lru_page(cursor_page, mode, file) == 0) {
 				list_move(&cursor_page->lru, dst);
 				nr_taken++;
 				scan++;
 				break;
-
-			case -EBUSY:
-				/* else it is being freed elsewhere */
-				list_move(&cursor_page->lru, src);
-			default:
-				break;	/* ! on LRU or wrong list */
 			}
 		}
 	}

--
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 1/3] remove wrong rotation at lumpy reclaim

[KOSAKI Motohiro]

> From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> 
> At lumpy reclaim, a page failed to be taken by __isolate_lru_page() can
> be pushed back to "src" list by list_move(). But the page may not be from
> "src" list. And list_move() itself is unnecessary because the page is
> not on top of LRU. Then, leave it as it is if __isolate_lru_page() fails.
> 
> This patch doesn't change the logic as "we should exit loop or not" and
> just fixes buggy list_move().
> 
> Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> ---
>  mm/vmscan.c |    9 +--------
>  1 file changed, 1 insertion(+), 8 deletions(-)
> 
> Index: lumpy-reclaim-trial/mm/vmscan.c
> ===================================================================
> --- lumpy-reclaim-trial.orig/mm/vmscan.c
> +++ lumpy-reclaim-trial/mm/vmscan.c
> @@ -936,18 +936,11 @@ static unsigned long isolate_lru_pages(u
>  			/* Check that we have not crossed a zone boundary. */
>  			if (unlikely(page_zone_id(cursor_page) != zone_id))
>  				continue;
> -			switch (__isolate_lru_page(cursor_page, mode, file)) {
> -			case 0:
> +			if (__isolate_lru_page(cursor_page, mode, file) == 0) {
>  				list_move(&cursor_page->lru, dst);
>  				nr_taken++;
>  				scan++;
>  				break;
> -
> -			case -EBUSY:
> -				/* else it is being freed elsewhere */
> -				list_move(&cursor_page->lru, src);
> -			default:
> -				break;	/* ! on LRU or wrong list */
>  			}
>  		}
>  	}
> 

Looks goold. Thanks fixing annoy bug.

	Reviewed-by: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>



--
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 1/3] remove wrong rotation at lumpy reclaim

[Minchan Kim]

On Thu, Jun 11, 2009 at 5:00 PM, KAMEZAWA
Hiroyuki<kamezawa.hiroyu@jp.fujitsu.com> wrote:
> From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
>
> At lumpy reclaim, a page failed to be taken by __isolate_lru_page() can
> be pushed back to "src" list by list_move(). But the page may not be from
> "src" list. And list_move() itself is unnecessary because the page is
> not on top of LRU. Then, leave it as it is if __isolate_lru_page() fails.
>
> This patch doesn't change the logic as "we should exit loop or not" and
> just fixes buggy list_move().
>
> Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> ---
>  mm/vmscan.c |    9 +--------
>  1 file changed, 1 insertion(+), 8 deletions(-)
>
> Index: lumpy-reclaim-trial/mm/vmscan.c
> ===================================================================
> --- lumpy-reclaim-trial.orig/mm/vmscan.c
> +++ lumpy-reclaim-trial/mm/vmscan.c
> @@ -936,18 +936,11 @@ static unsigned long isolate_lru_pages(u
>                        /* Check that we have not crossed a zone boundary. */
>                        if (unlikely(page_zone_id(cursor_page) != zone_id))
>                                continue;
> -                       switch (__isolate_lru_page(cursor_page, mode, file)) {
> -                       case 0:
> +                       if (__isolate_lru_page(cursor_page, mode, file) == 0) {
>                                list_move(&cursor_page->lru, dst);
>                                nr_taken++;
>                                scan++;
>                                break;

break ??

-- 
Kinds regards,
Minchan Kim

Re: [PATCH 1/3] remove wrong rotation at lumpy reclaim

[KAMEZAWA Hiroyuki]

Minchan Kim wrote:
> On Thu, Jun 11, 2009 at 5:00 PM, KAMEZAWA
> Hiroyuki<kamezawa.hiroyu@jp.fujitsu.com> wrote:
>> From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
>>
>> At lumpy reclaim, a page failed to be taken by __isolate_lru_page() can
>> be pushed back to "src" list by list_move(). But the page may not be
>> from
>> "src" list. And list_move() itself is unnecessary because the page is
>> not on top of LRU. Then, leave it as it is if __isolate_lru_page()
>> fails.
>>
>> This patch doesn't change the logic as "we should exit loop or not" and
>> just fixes buggy list_move().
>>
>> Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
>> ---
>> ?mm/vmscan.c | ? ?9 +--------
>> ?1 file changed, 1 insertion(+), 8 deletions(-)
>>
>> Index: lumpy-reclaim-trial/mm/vmscan.c
>> ===================================================================
>> --- lumpy-reclaim-trial.orig/mm/vmscan.c
>> +++ lumpy-reclaim-trial/mm/vmscan.c
>> @@ -936,18 +936,11 @@ static unsigned long isolate_lru_pages(u
>> ? ? ? ? ? ? ? ? ? ? ? ?/* Check that we have not crossed a zone
>> boundary. */
>> ? ? ? ? ? ? ? ? ? ? ? ?if (unlikely(page_zone_id(cursor_page) !=
>> zone_id))
>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?continue;
>> - ? ? ? ? ? ? ? ? ? ? ? switch (__isolate_lru_page(cursor_page, mode,
>> file)) {
>> - ? ? ? ? ? ? ? ? ? ? ? case 0:
>> + ? ? ? ? ? ? ? ? ? ? ? if (__isolate_lru_page(cursor_page, mode, file)
>> == 0) {
>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?list_move(&cursor_page->lru, dst);
>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?nr_taken++;
>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?scan++;
>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?break;
>
> break ??
>
good catch. I'll post updated one tomorrow.
I'm very sorry ;(

Thanks,
-Kame


--
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 1/3] remove wrong rotation at lumpy reclaim

[Minchan Kim]

2009/6/11 KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>:
> Minchan Kim wrote:
>> On Thu, Jun 11, 2009 at 5:00 PM, KAMEZAWA
>> Hiroyuki<kamezawa.hiroyu@jp.fujitsu.com> wrote:
>>> From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
>>>
>>> At lumpy reclaim, a page failed to be taken by __isolate_lru_page() can
>>> be pushed back to "src" list by list_move(). But the page may not be
>>> from
>>> "src" list. And list_move() itself is unnecessary because the page is
>>> not on top of LRU. Then, leave it as it is if __isolate_lru_page()
>>> fails.
>>>
>>> This patch doesn't change the logic as "we should exit loop or not" and
>>> just fixes buggy list_move().
>>>
>>> Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
>>> ---
>>> ?mm/vmscan.c | ? ?9 +--------
>>> ?1 file changed, 1 insertion(+), 8 deletions(-)
>>>
>>> Index: lumpy-reclaim-trial/mm/vmscan.c
>>> ===================================================================
>>> --- lumpy-reclaim-trial.orig/mm/vmscan.c
>>> +++ lumpy-reclaim-trial/mm/vmscan.c
>>> @@ -936,18 +936,11 @@ static unsigned long isolate_lru_pages(u
>>> ? ? ? ? ? ? ? ? ? ? ? ?/* Check that we have not crossed a zone
>>> boundary. */
>>> ? ? ? ? ? ? ? ? ? ? ? ?if (unlikely(page_zone_id(cursor_page) !=
>>> zone_id))
>>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?continue;
>>> - ? ? ? ? ? ? ? ? ? ? ? switch (__isolate_lru_page(cursor_page, mode,
>>> file)) {
>>> - ? ? ? ? ? ? ? ? ? ? ? case 0:
>>> + ? ? ? ? ? ? ? ? ? ? ? if (__isolate_lru_page(cursor_page, mode, file)
>>> == 0) {
>>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?list_move(&cursor_page->lru, dst);
>>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?nr_taken++;
>>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?scan++;
>>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?break;
>>
>> break ??
>>
> good catch. I'll post updated one tomorrow.
> I'm very sorry ;(

Never mind.  :)

> Thanks,
> -Kame
>
>
> --
> 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>
>



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

Re: [PATCH 1/3] remove wrong rotation at lumpy reclaim

[Minchan Kim]

Otherwise, Looks good to me.
Reviewed-by: Minchan Kim <minchan.kim@gmail.com>

On Thu, Jun 11, 2009 at 6:18 PM, Minchan Kim<minchan.kim@gmail.com> wrote:
> On Thu, Jun 11, 2009 at 5:00 PM, KAMEZAWA
> Hiroyuki<kamezawa.hiroyu@jp.fujitsu.com> wrote:
>> From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
>>
>> At lumpy reclaim, a page failed to be taken by __isolate_lru_page() can
>> be pushed back to "src" list by list_move(). But the page may not be from
>> "src" list. And list_move() itself is unnecessary because the page is
>> not on top of LRU. Then, leave it as it is if __isolate_lru_page() fails.
>>
>> This patch doesn't change the logic as "we should exit loop or not" and
>> just fixes buggy list_move().
>>
>> Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
>> ---
>>  mm/vmscan.c |    9 +--------
>>  1 file changed, 1 insertion(+), 8 deletions(-)
>>
>> Index: lumpy-reclaim-trial/mm/vmscan.c
>> ===================================================================
>> --- lumpy-reclaim-trial.orig/mm/vmscan.c
>> +++ lumpy-reclaim-trial/mm/vmscan.c
>> @@ -936,18 +936,11 @@ static unsigned long isolate_lru_pages(u
>>                        /* Check that we have not crossed a zone boundary. */
>>                        if (unlikely(page_zone_id(cursor_page) != zone_id))
>>                                continue;
>> -                       switch (__isolate_lru_page(cursor_page, mode, file)) {
>> -                       case 0:
>> +                       if (__isolate_lru_page(cursor_page, mode, file) == 0) {
>>                                list_move(&cursor_page->lru, dst);
>>                                nr_taken++;
>>                                scan++;
>>                                break;
>
> break ??
>
> Kinds regards,
> Minchan Kim
>



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

Re: [PATCH 1/3] remove wrong rotation at lumpy reclaim

[Mel Gorman]

On Thu, Jun 11, 2009 at 05:00:18PM +0900, KAMEZAWA Hiroyuki wrote:
> From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> 
> At lumpy reclaim, a page failed to be taken by __isolate_lru_page() can
> be pushed back to "src" list by list_move(). But the page may not be from
> "src" list. And list_move() itself is unnecessary because the page is
> not on top of LRU. Then, leave it as it is if __isolate_lru_page() fails.
> 
> This patch doesn't change the logic as "we should exit loop or not" and
> just fixes buggy list_move().
> 
> Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> ---
>  mm/vmscan.c |    9 +--------
>  1 file changed, 1 insertion(+), 8 deletions(-)
> 
> Index: lumpy-reclaim-trial/mm/vmscan.c
> ===================================================================
> --- lumpy-reclaim-trial.orig/mm/vmscan.c
> +++ lumpy-reclaim-trial/mm/vmscan.c
> @@ -936,18 +936,11 @@ static unsigned long isolate_lru_pages(u
>  			/* Check that we have not crossed a zone boundary. */
>  			if (unlikely(page_zone_id(cursor_page) != zone_id))
>  				continue;
> -			switch (__isolate_lru_page(cursor_page, mode, file)) {
> -			case 0:
> +			if (__isolate_lru_page(cursor_page, mode, file) == 0) {
>  				list_move(&cursor_page->lru, dst);
>  				nr_taken++;
>  				scan++;
>  				break;
> -
> -			case -EBUSY:
> -				/* else it is being freed elsewhere */
> -				list_move(&cursor_page->lru, src);
> -			default:
> -				break;	/* ! on LRU or wrong list */
>  			}
>  		}
>  	}
> 

At very minimum, this avoids an unnecessary reshuffling of the LRU lists
during lumpy reclaim. Thanks

Acked-by: Mel Gorman <mel@csn.ul.ie>

-- 
Mel Gorman
Part-time Phd Student                          Linux Technology Center
University of Limerick                         IBM Dublin Software Lab

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

[PATCH 2/3] check unevictable flag in lumy reclaim

[KAMEZAWA Hiroyuki]

From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>

Lumpy reclaim scans pages from their pfn. Then, it can find unevictable pages
in its loop. Abort lumpy reclaim when we find Unevictable page, we never get a
block of pages for requested order.

Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
---
Index: lumpy-reclaim-trial/mm/vmscan.c
===================================================================
--- lumpy-reclaim-trial.orig/mm/vmscan.c
+++ lumpy-reclaim-trial/mm/vmscan.c
@@ -936,6 +936,9 @@ static unsigned long isolate_lru_pages(u
 			/* Check that we have not crossed a zone boundary. */
 			if (unlikely(page_zone_id(cursor_page) != zone_id))
 				continue;
+			/* Abort when the page is mlocked */
+			if (unlikely(PageUnevictable(cursor_page)))
+				break;
 			if (__isolate_lru_page(cursor_page, mode, file) == 0) {
 				list_move(&cursor_page->lru, dst);
 				nr_taken++;

--
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 2/3] check unevictable flag in lumy reclaim

[KOSAKI Motohiro]

> From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> 
> Lumpy reclaim scans pages from their pfn. Then, it can find unevictable pages
> in its loop. Abort lumpy reclaim when we find Unevictable page, we never get a
> block of pages for requested order.
> 
> Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> ---
> Index: lumpy-reclaim-trial/mm/vmscan.c
> ===================================================================
> --- lumpy-reclaim-trial.orig/mm/vmscan.c
> +++ lumpy-reclaim-trial/mm/vmscan.c
> @@ -936,6 +936,9 @@ static unsigned long isolate_lru_pages(u
>  			/* Check that we have not crossed a zone boundary. */
>  			if (unlikely(page_zone_id(cursor_page) != zone_id))
>  				continue;
> +			/* Abort when the page is mlocked */
> +			if (unlikely(PageUnevictable(cursor_page)))
> +				break;
>  			if (__isolate_lru_page(cursor_page, mode, file) == 0) {
>  				list_move(&cursor_page->lru, dst);
>  				nr_taken++;
> 

The code is good. thanks.

But please comment adding more. plus, unevictable is made by multiple reason.
not only mlock. please fix misleading comment.


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

[PATCH 2/3] check unevictable flag in lumy reclaim v2

[KAMEZAWA Hiroyuki]

How about this ?

From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>

Lumpy reclaim check pages from their pfn. Then, it can find unevictable pages
in its loop.
Abort lumpy reclaim when we find Unevictable page, we never get a lump
of pages for requested order.

Changelog: v1->v2
 - rewrote commet.

Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
---
 mm/vmscan.c |    9 +++++++++
 1 file changed, 9 insertions(+)

Index: lumpy-reclaim-trial/mm/vmscan.c
===================================================================
--- lumpy-reclaim-trial.orig/mm/vmscan.c
+++ lumpy-reclaim-trial/mm/vmscan.c
@@ -936,6 +936,15 @@ static unsigned long isolate_lru_pages(u
 			/* Check that we have not crossed a zone boundary. */
 			if (unlikely(page_zone_id(cursor_page) != zone_id))
 				continue;
+			/*
+			 * We tries to free all pages in this range to create
+			 * a free large page. Then, if the range includes a page
+			 * never be reclaimed, we have no reason to do more.
+			 * PageUnevictable page is not a page which can be
+			 * easily freed. Abort this scan now.
+			 */
+			if (unlikely(PageUnevictable(cursor_page)))
+				break;
 			if (__isolate_lru_page(cursor_page, mode, file) == 0) {
 				list_move(&cursor_page->lru, dst);
 				nr_taken++;

--
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 2/3] check unevictable flag in lumy reclaim v2

[KOSAKI Motohiro]

> How about this ?
> 
> From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> 
> Lumpy reclaim check pages from their pfn. Then, it can find unevictable pages
> in its loop.
> Abort lumpy reclaim when we find Unevictable page, we never get a lump
> of pages for requested order.
> 
> Changelog: v1->v2
>  - rewrote commet.

Great.
	Reviewed-by: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>

> 
> Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> ---
>  mm/vmscan.c |    9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> Index: lumpy-reclaim-trial/mm/vmscan.c
> ===================================================================
> --- lumpy-reclaim-trial.orig/mm/vmscan.c
> +++ lumpy-reclaim-trial/mm/vmscan.c
> @@ -936,6 +936,15 @@ static unsigned long isolate_lru_pages(u
>  			/* Check that we have not crossed a zone boundary. */
>  			if (unlikely(page_zone_id(cursor_page) != zone_id))
>  				continue;
> +			/*
> +			 * We tries to free all pages in this range to create
> +			 * a free large page. Then, if the range includes a page
> +			 * never be reclaimed, we have no reason to do more.
> +			 * PageUnevictable page is not a page which can be
> +			 * easily freed. Abort this scan now.
> +			 */
> +			if (unlikely(PageUnevictable(cursor_page)))
> +				break;
>  			if (__isolate_lru_page(cursor_page, mode, file) == 0) {
>  				list_move(&cursor_page->lru, dst);
>  				nr_taken++;
> 
> --
> 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>



--
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 2/3] check unevictable flag in lumy reclaim v2

[Minchan Kim]

On Thu, Jun 11, 2009 at 5:38 PM, KAMEZAWA
Hiroyuki<kamezawa.hiroyu@jp.fujitsu.com> wrote:
> How about this ?
>
> From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
>
> Lumpy reclaim check pages from their pfn. Then, it can find unevictable pages
> in its loop.
> Abort lumpy reclaim when we find Unevictable page, we never get a lump
> of pages for requested order.
>
> Changelog: v1->v2
>  - rewrote commet.
>
> Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> ---
>  mm/vmscan.c |    9 +++++++++
>  1 file changed, 9 insertions(+)
>
> Index: lumpy-reclaim-trial/mm/vmscan.c
> ===================================================================
> --- lumpy-reclaim-trial.orig/mm/vmscan.c
> +++ lumpy-reclaim-trial/mm/vmscan.c
> @@ -936,6 +936,15 @@ static unsigned long isolate_lru_pages(u
>                        /* Check that we have not crossed a zone boundary. */
>                        if (unlikely(page_zone_id(cursor_page) != zone_id))
>                                continue;
> +                       /*
> +                        * We tries to free all pages in this range to create
> +                        * a free large page. Then, if the range includes a page
> +                        * never be reclaimed, we have no reason to do more.
> +                        * PageUnevictable page is not a page which can be
> +                        * easily freed. Abort this scan now.
> +                        */
> +                       if (unlikely(PageUnevictable(cursor_page)))
> +                               break;

__isolate_lru_pages already checked PageUnevictable to return error.
I want to remove repeated check although it is trivial.

By your patch, It seems to remove PageUnevictable check in __isolate_lru_pages.

But I know that. If we remove PageUnevictable check in
__isolate_lru_pages, it can't go into BUG in non-lumpy case. ( I
mentioned following as code)

                case -EBUSY:
                        /* else it is being freed elsewhere */
                        list_move(&page->lru, src);
                        continue;

                default:
                        BUG();
                }


It means we can remove BUG in non-lumpy case and then add BUG into
__isolate_lru_pages directly.

If we can do it, we can remove unnecessary PageUnevictable check in
__isolate_lru_page.

I am not sure this is right in case of memcg.

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

Re: [PATCH 2/3] check unevictable flag in lumy reclaim v2

[Minchan Kim]

On Thu, Jun 11, 2009 at 6:37 PM, Minchan Kim<minchan.kim@gmail.com> wrote:
> On Thu, Jun 11, 2009 at 5:38 PM, KAMEZAWA
> Hiroyuki<kamezawa.hiroyu@jp.fujitsu.com> wrote:
>> How about this ?
>>
>> From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
>>
>> Lumpy reclaim check pages from their pfn. Then, it can find unevictable pages
>> in its loop.
>> Abort lumpy reclaim when we find Unevictable page, we never get a lump
>> of pages for requested order.
>>
>> Changelog: v1->v2
>>  - rewrote commet.
>>
>> Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
>> ---
>>  mm/vmscan.c |    9 +++++++++
>>  1 file changed, 9 insertions(+)
>>
>> Index: lumpy-reclaim-trial/mm/vmscan.c
>> ===================================================================
>> --- lumpy-reclaim-trial.orig/mm/vmscan.c
>> +++ lumpy-reclaim-trial/mm/vmscan.c
>> @@ -936,6 +936,15 @@ static unsigned long isolate_lru_pages(u
>>                        /* Check that we have not crossed a zone boundary. */
>>                        if (unlikely(page_zone_id(cursor_page) != zone_id))
>>                                continue;
>> +                       /*
>> +                        * We tries to free all pages in this range to create
>> +                        * a free large page. Then, if the range includes a page
>> +                        * never be reclaimed, we have no reason to do more.
>> +                        * PageUnevictable page is not a page which can be
>> +                        * easily freed. Abort this scan now.
>> +                        */
>> +                       if (unlikely(PageUnevictable(cursor_page)))
>> +                               break;
>
> __isolate_lru_pages already checked PageUnevictable to return error.
> I want to remove repeated check although it is trivial.
>
> By your patch, It seems to remove PageUnevictable check in __isolate_lru_pages.

typo.

It seems we can remove PageUnevictable check in __isolate_lru_pages.
Sorry for noise.

> But I know that. If we remove PageUnevictable check in
> __isolate_lru_pages, it can't go into BUG in non-lumpy case. ( I
> mentioned following as code)
>
>                case -EBUSY:
>                        /* else it is being freed elsewhere */
>                        list_move(&page->lru, src);
>                        continue;
>
>                default:
>                        BUG();
>                }
>
>
> It means we can remove BUG in non-lumpy case and then add BUG into
> __isolate_lru_pages directly.
>
> If we can do it, we can remove unnecessary PageUnevictable check in
> __isolate_lru_page.
>
> I am not sure this is right in case of memcg.
>
> --
> Kinds regards,
> Minchan Kim
>



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

Re: [PATCH 2/3] check unevictable flag in lumy reclaim v2

[KAMEZAWA Hiroyuki]

Minchan Kim さん wrote:
> On Thu, Jun 11, 2009 at 5:38 PM, KAMEZAWA
> Hiroyuki<kamezawa.hiroyu@jp.fujitsu.com> wrote:
>> How about this ?
>>
>> From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
>>
>> Lumpy reclaim check pages from their pfn. Then, it can find unevictable
>> pages
>> in its loop.
>> Abort lumpy reclaim when we find Unevictable page, we never get a lump
>> of pages for requested order.
>>
>> Changelog: v1->v2
>> ?- rewrote commet.
>>
>> Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
>> ---
>> ?mm/vmscan.c | ? ?9 +++++++++
>> ?1 file changed, 9 insertions(+)
>>
>> Index: lumpy-reclaim-trial/mm/vmscan.c
>> ===================================================================
>> --- lumpy-reclaim-trial.orig/mm/vmscan.c
>> +++ lumpy-reclaim-trial/mm/vmscan.c
>> @@ -936,6 +936,15 @@ static unsigned long isolate_lru_pages(u
>> ? ? ? ? ? ? ? ? ? ? ? ?/* Check that we have not crossed a zone
>> boundary. */
>> ? ? ? ? ? ? ? ? ? ? ? ?if (unlikely(page_zone_id(cursor_page) !=
>> zone_id))
>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?continue;
>> + ? ? ? ? ? ? ? ? ? ? ? /*
>> + ? ? ? ? ? ? ? ? ? ? ? ?* We tries to free all pages in this range to
>> create
>> + ? ? ? ? ? ? ? ? ? ? ? ?* a free large page. Then, if the range
>> includes a page
>> + ? ? ? ? ? ? ? ? ? ? ? ?* never be reclaimed, we have no reason to do
>> more.
>> + ? ? ? ? ? ? ? ? ? ? ? ?* PageUnevictable page is not a page which can
>> be
>> + ? ? ? ? ? ? ? ? ? ? ? ?* easily freed. Abort this scan now.
>> + ? ? ? ? ? ? ? ? ? ? ? ?*/
>> + ? ? ? ? ? ? ? ? ? ? ? if (unlikely(PageUnevictable(cursor_page)))
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? break;
>
> __isolate_lru_pages already checked PageUnevictable to return error.
> I want to remove repeated check although it is trivial.
>
> By your patch, It seems to remove PageUnevictable check in
> __isolate_lru_pages.
>
yes.

> But I know that. If we remove PageUnevictable check in
> __isolate_lru_pages, it can't go into BUG in non-lumpy case. ( I
> mentioned following as code)
>
In non-lumpy case, we'll never see Unevictable, maybe.

>                 case -EBUSY:
>                         /* else it is being freed elsewhere */
>                         list_move(&page->lru, src);
>                         continue;
>
>                 default:
>                         BUG();
>                 }
>
>
> It means we can remove BUG in non-lumpy case and then add BUG into
> __isolate_lru_pages directly.
>
> If we can do it, we can remove unnecessary PageUnevictable check in
> __isolate_lru_page.
>
Hmm, but Unevicable check had tons of troubles at its implementation
and I don't want to do it at once.

> I am not sure this is right in case of memcg.
>
I think we don't see Unevictable in memcg's path if my memcg-lru code
works as designed.

I'll postpone this patch for a while until my brain works well.

Thanks,
-Kame

--
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 2/3] check unevictable flag in lumy reclaim v2

[Minchan Kim]

2009/6/11 KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>:
> Minchan Kim さん wrote:
>> On Thu, Jun 11, 2009 at 5:38 PM, KAMEZAWA
>> Hiroyuki<kamezawa.hiroyu@jp.fujitsu.com> wrote:
>>> How about this ?
>>>
>>> From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
>>>
>>> Lumpy reclaim check pages from their pfn. Then, it can find unevictable
>>> pages
>>> in its loop.
>>> Abort lumpy reclaim when we find Unevictable page, we never get a lump
>>> of pages for requested order.
>>>
>>> Changelog: v1->v2
>>> ?- rewrote commet.
>>>
>>> Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
>>> ---
>>> ?mm/vmscan.c | ? ?9 +++++++++
>>> ?1 file changed, 9 insertions(+)
>>>
>>> Index: lumpy-reclaim-trial/mm/vmscan.c
>>> ===================================================================
>>> --- lumpy-reclaim-trial.orig/mm/vmscan.c
>>> +++ lumpy-reclaim-trial/mm/vmscan.c
>>> @@ -936,6 +936,15 @@ static unsigned long isolate_lru_pages(u
>>> ? ? ? ? ? ? ? ? ? ? ? ?/* Check that we have not crossed a zone
>>> boundary. */
>>> ? ? ? ? ? ? ? ? ? ? ? ?if (unlikely(page_zone_id(cursor_page) !=
>>> zone_id))
>>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?continue;
>>> + ? ? ? ? ? ? ? ? ? ? ? /*
>>> + ? ? ? ? ? ? ? ? ? ? ? ?* We tries to free all pages in this range to
>>> create
>>> + ? ? ? ? ? ? ? ? ? ? ? ?* a free large page. Then, if the range
>>> includes a page
>>> + ? ? ? ? ? ? ? ? ? ? ? ?* never be reclaimed, we have no reason to do
>>> more.
>>> + ? ? ? ? ? ? ? ? ? ? ? ?* PageUnevictable page is not a page which can
>>> be
>>> + ? ? ? ? ? ? ? ? ? ? ? ?* easily freed. Abort this scan now.
>>> + ? ? ? ? ? ? ? ? ? ? ? ?*/
>>> + ? ? ? ? ? ? ? ? ? ? ? if (unlikely(PageUnevictable(cursor_page)))
>>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? break;
>>
>> __isolate_lru_pages already checked PageUnevictable to return error.
>> I want to remove repeated check although it is trivial.
>>
>> By your patch, It seems to remove PageUnevictable check in
>> __isolate_lru_pages.
>>
> yes.
>
>> But I know that. If we remove PageUnevictable check in
>> __isolate_lru_pages, it can't go into BUG in non-lumpy case. ( I
>> mentioned following as code)
>>
> In non-lumpy case, we'll never see Unevictable, maybe.

I think so if it doesn't happen RAM failure.
AFAIK, Unevictable check didn't related with RAM failure.

>
>>                 case -EBUSY:
>>                         /* else it is being freed elsewhere */
>>                         list_move(&page->lru, src);
>>                         continue;
>>
>>                 default:
>>                         BUG();
>>                 }
>>
>>
>> It means we can remove BUG in non-lumpy case and then add BUG into
>> __isolate_lru_pages directly.
>>
>> If we can do it, we can remove unnecessary PageUnevictable check in
>> __isolate_lru_page.
>>
> Hmm, but Unevicable check had tons of troubles at its implementation
> and I don't want to do it at once.

I think it's not a big problem.
As comment said, the check's goal is to prevent in lumpy case.
        /*
         * When this function is being called for lumpy reclaim, we
         * initially look into all LRU pages, active, inactive and
         * unevictable; only give shrink_page_list evictable pages.
         */
        if (PageUnevictable(page))
                return ret;

So I think we can remove this check.

>> I am not sure this is right in case of memcg.
>>
> I think we don't see Unevictable in memcg's path if my memcg-lru code
> works as designed.
>
> I'll postpone this patch for a while until my brain works well.

If you have a concern about that, how about this ?
(This code will be hunk since gmail webserver always mangle. Pz,forgive me)
Also, we can CC original authors.

--- a/mm/vmscan.c
++ b/mm/vmscan.c
@@ -936,19 +936,20 @@ static unsigned long isolate_lru_pages(unsigned
long nr_to_scan,
                        /* Check that we have not crossed a zone boundary. */
                        if (unlikely(page_zone_id(cursor_page) != zone_id))
                                continue;
-                       switch (__isolate_lru_page(cursor_page, mode, file)) {
-                       case 0:
+                       if (__isolate_lru_page(cursor_page, mode, file) == 0) {
                                list_move(&cursor_page->lru, dst);
                                nr_taken++;
                                scan++;
-                               break;
-
-                       case -EBUSY:
-                               /* else it is being freed elsewhere */
-                               list_move(&cursor_page->lru, src);
-                       default:
-                               break;  /* ! on LRU or wrong list */
                        }
+                       else if (PageUnevictable(cursor_page))
+                                       /*
+                                * We tries to free all pages in this
range to create
+                                * a free large page. Then, if the
range includes a page
+                                * never be reclaimed, we have no
reason to do more.
+                                * PageUnevictable page is not a page
which can be
+                                * easily freed. Abort this scan now.
+                                */
+                               break
                }
        }


> Thanks,
> -Kame
>
>



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

Re: [PATCH 2/3] check unevictable flag in lumy reclaim v2

[KAMEZAWA Hiroyuki]

Minchan Kim wrote:
> 2009/6/11 KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>:
>> Minchan Kim さん wrote:
>>> On Thu, Jun 11, 2009 at 5:38 PM, KAMEZAWA
>>> Hiroyuki<kamezawa.hiroyu@jp.fujitsu.com> wrote:
>>>> How about this ?
>>>>
>>>> From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
>>>>
>>>> Lumpy reclaim check pages from their pfn. Then, it can find
>>>> unevictable
>>>> pages
>>>> in its loop.
>>>> Abort lumpy reclaim when we find Unevictable page, we never get a lump
>>>> of pages for requested order.
>>>>
>>>> Changelog: v1->v2
>>>> ?- rewrote commet.
>>>>
>>>> Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
>>>> ---
>>>> ?mm/vmscan.c | ? ?9 +++++++++
>>>> ?1 file changed, 9 insertions(+)
>>>>
>>>> Index: lumpy-reclaim-trial/mm/vmscan.c
>>>> ===================================================================
>>>> --- lumpy-reclaim-trial.orig/mm/vmscan.c
>>>> +++ lumpy-reclaim-trial/mm/vmscan.c
>>>> @@ -936,6 +936,15 @@ static unsigned long isolate_lru_pages(u
>>>> ? ? ? ? ? ? ? ? ? ? ? ?/* Check that we have not crossed a zone
>>>> boundary. */
>>>> ? ? ? ? ? ? ? ? ? ? ? ?if (unlikely(page_zone_id(cursor_page) !=
>>>> zone_id))
>>>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?continue;
>>>> + ? ? ? ? ? ? ? ? ? ? ? /*
>>>> + ? ? ? ? ? ? ? ? ? ? ? ?* We tries to free all pages in this range to
>>>> create
>>>> + ? ? ? ? ? ? ? ? ? ? ? ?* a free large page. Then, if the range
>>>> includes a page
>>>> + ? ? ? ? ? ? ? ? ? ? ? ?* never be reclaimed, we have no reason to do
>>>> more.
>>>> + ? ? ? ? ? ? ? ? ? ? ? ?* PageUnevictable page is not a page which
>>>> can
>>>> be
>>>> + ? ? ? ? ? ? ? ? ? ? ? ?* easily freed. Abort this scan now.
>>>> + ? ? ? ? ? ? ? ? ? ? ? ?*/
>>>> + ? ? ? ? ? ? ? ? ? ? ? if (unlikely(PageUnevictable(cursor_page)))
>>>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? break;
>>>
>>> __isolate_lru_pages already checked PageUnevictable to return error.
>>> I want to remove repeated check although it is trivial.
>>>
>>> By your patch, It seems to remove PageUnevictable check in
>>> __isolate_lru_pages.
>>>
>> yes.
>>
>>> But I know that. If we remove PageUnevictable check in
>>> __isolate_lru_pages, it can't go into BUG in non-lumpy case. ( I
>>> mentioned following as code)
>>>
>> In non-lumpy case, we'll never see Unevictable, maybe.
>
> I think so if it doesn't happen RAM failure.
> AFAIK, Unevictable check didn't related with RAM failure.
>
>>
>>> ? ? ? ? ? ? ? ? case -EBUSY:
>>> ? ? ? ? ? ? ? ? ? ? ? ? /* else it is being freed elsewhere */
>>> ? ? ? ? ? ? ? ? ? ? ? ? list_move(&page->lru, src);
>>> ? ? ? ? ? ? ? ? ? ? ? ? continue;
>>>
>>> ? ? ? ? ? ? ? ? default:
>>> ? ? ? ? ? ? ? ? ? ? ? ? BUG();
>>> ? ? ? ? ? ? ? ? }
>>>
>>>
>>> It means we can remove BUG in non-lumpy case and then add BUG into
>>> __isolate_lru_pages directly.
>>>
>>> If we can do it, we can remove unnecessary PageUnevictable check in
>>> __isolate_lru_page.
>>>
>> Hmm, but Unevicable check had tons of troubles at its implementation
>> and I don't want to do it at once.
>
> I think it's not a big problem.
> As comment said, the check's goal is to prevent in lumpy case.
>         /*
>          * When this function is being called for lumpy reclaim, we
>          * initially look into all LRU pages, active, inactive and
>          * unevictable; only give shrink_page_list evictable pages.
>          */
>         if (PageUnevictable(page))
>                 return ret;
>
> So I think we can remove this check.
>
agreed.

>>> I am not sure this is right in case of memcg.
>>>
>> I think we don't see Unevictable in memcg's path if my memcg-lru code
>> works as designed.
>>
>> I'll postpone this patch for a while until my brain works well.
>
> If you have a concern about that, how about this ?
> (This code will be hunk since gmail webserver always mangle. Pz,forgive
> me)
> Also, we can CC original authors.
>
I'll schedule this optimization/clean up for unevictable case in queue.
Thank you for inputs.

But it's now merge-window, I'd like to push bugfix first.(1/3 and 3/3)
I'd like to scheule Unevictable case fix after rc1(when mmotm stack seems
to be pushed out to Linus.)
And I'll add
int __isolate_lru_page(...)
{
     VM_BUG_ON(PageUnevictable(page));
}
as sanity check for mmotm test time.

Thank you for all your help.
-Kame

--
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 2/3] check unevictable flag in lumy reclaim v2

[Minchan Kim]

2009/6/11 KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>:
> Minchan Kim wrote:
>> 2009/6/11 KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>:
>>> Minchan Kim さん wrote:
>>>> On Thu, Jun 11, 2009 at 5:38 PM, KAMEZAWA
>>>> Hiroyuki<kamezawa.hiroyu@jp.fujitsu.com> wrote:
>>>>> How about this ?
>>>>>
>>>>> From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
>>>>>
>>>>> Lumpy reclaim check pages from their pfn. Then, it can find
>>>>> unevictable
>>>>> pages
>>>>> in its loop.
>>>>> Abort lumpy reclaim when we find Unevictable page, we never get a lump
>>>>> of pages for requested order.
>>>>>
>>>>> Changelog: v1->v2
>>>>> ?- rewrote commet.
>>>>>
>>>>> Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
>>>>> ---
>>>>> ?mm/vmscan.c | ? ?9 +++++++++
>>>>> ?1 file changed, 9 insertions(+)
>>>>>
>>>>> Index: lumpy-reclaim-trial/mm/vmscan.c
>>>>> ===================================================================
>>>>> --- lumpy-reclaim-trial.orig/mm/vmscan.c
>>>>> +++ lumpy-reclaim-trial/mm/vmscan.c
>>>>> @@ -936,6 +936,15 @@ static unsigned long isolate_lru_pages(u
>>>>> ? ? ? ? ? ? ? ? ? ? ? ?/* Check that we have not crossed a zone
>>>>> boundary. */
>>>>> ? ? ? ? ? ? ? ? ? ? ? ?if (unlikely(page_zone_id(cursor_page) !=
>>>>> zone_id))
>>>>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?continue;
>>>>> + ? ? ? ? ? ? ? ? ? ? ? /*
>>>>> + ? ? ? ? ? ? ? ? ? ? ? ?* We tries to free all pages in this range to
>>>>> create
>>>>> + ? ? ? ? ? ? ? ? ? ? ? ?* a free large page. Then, if the range
>>>>> includes a page
>>>>> + ? ? ? ? ? ? ? ? ? ? ? ?* never be reclaimed, we have no reason to do
>>>>> more.
>>>>> + ? ? ? ? ? ? ? ? ? ? ? ?* PageUnevictable page is not a page which
>>>>> can
>>>>> be
>>>>> + ? ? ? ? ? ? ? ? ? ? ? ?* easily freed. Abort this scan now.
>>>>> + ? ? ? ? ? ? ? ? ? ? ? ?*/
>>>>> + ? ? ? ? ? ? ? ? ? ? ? if (unlikely(PageUnevictable(cursor_page)))
>>>>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? break;
>>>>
>>>> __isolate_lru_pages already checked PageUnevictable to return error.
>>>> I want to remove repeated check although it is trivial.
>>>>
>>>> By your patch, It seems to remove PageUnevictable check in
>>>> __isolate_lru_pages.
>>>>
>>> yes.
>>>
>>>> But I know that. If we remove PageUnevictable check in
>>>> __isolate_lru_pages, it can't go into BUG in non-lumpy case. ( I
>>>> mentioned following as code)
>>>>
>>> In non-lumpy case, we'll never see Unevictable, maybe.
>>
>> I think so if it doesn't happen RAM failure.
>> AFAIK, Unevictable check didn't related with RAM failure.
>>
>>>
>>>> ? ? ? ? ? ? ? ? case -EBUSY:
>>>> ? ? ? ? ? ? ? ? ? ? ? ? /* else it is being freed elsewhere */
>>>> ? ? ? ? ? ? ? ? ? ? ? ? list_move(&page->lru, src);
>>>> ? ? ? ? ? ? ? ? ? ? ? ? continue;
>>>>
>>>> ? ? ? ? ? ? ? ? default:
>>>> ? ? ? ? ? ? ? ? ? ? ? ? BUG();
>>>> ? ? ? ? ? ? ? ? }
>>>>
>>>>
>>>> It means we can remove BUG in non-lumpy case and then add BUG into
>>>> __isolate_lru_pages directly.
>>>>
>>>> If we can do it, we can remove unnecessary PageUnevictable check in
>>>> __isolate_lru_page.
>>>>
>>> Hmm, but Unevicable check had tons of troubles at its implementation
>>> and I don't want to do it at once.
>>
>> I think it's not a big problem.
>> As comment said, the check's goal is to prevent in lumpy case.
>>         /*
>>          * When this function is being called for lumpy reclaim, we
>>          * initially look into all LRU pages, active, inactive and
>>          * unevictable; only give shrink_page_list evictable pages.
>>          */
>>         if (PageUnevictable(page))
>>                 return ret;
>>
>> So I think we can remove this check.
>>
> agreed.
>
>>>> I am not sure this is right in case of memcg.
>>>>
>>> I think we don't see Unevictable in memcg's path if my memcg-lru code
>>> works as designed.
>>>
>>> I'll postpone this patch for a while until my brain works well.
>>
>> If you have a concern about that, how about this ?
>> (This code will be hunk since gmail webserver always mangle. Pz,forgive
>> me)
>> Also, we can CC original authors.
>>
> I'll schedule this optimization/clean up for unevictable case in queue.
> Thank you for inputs.
>
> But it's now merge-window, I'd like to push bugfix first.(1/3 and 3/3)

I agree. It's more important now.

> I'd like to scheule Unevictable case fix after rc1(when mmotm stack seems
> to be pushed out to Linus.)
> And I'll add
> int __isolate_lru_page(...)
> {
>     VM_BUG_ON(PageUnevictable(page));
> }
> as sanity check for mmotm test time.
>
> Thank you for all your help.

I also thanks you for considering my comment.

You may add my review sign. :)
Reviewed-by: Minchan Kim <minchan.kim@gmail.com>

> -Kame
>
>



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

Re: [PATCH 2/3] check unevictable flag in lumy reclaim

[Mel Gorman]

On Thu, Jun 11, 2009 at 05:01:52PM +0900, KAMEZAWA Hiroyuki wrote:
> From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> 
> Lumpy reclaim scans pages from their pfn. Then, it can find unevictable pages
> in its loop. Abort lumpy reclaim when we find Unevictable page, we never get a
> block of pages for requested order.
> 
> Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>

Acked-by: Mel Gorman <mel@csn.ul.ie>

> ---
> Index: lumpy-reclaim-trial/mm/vmscan.c
> ===================================================================
> --- lumpy-reclaim-trial.orig/mm/vmscan.c
> +++ lumpy-reclaim-trial/mm/vmscan.c
> @@ -936,6 +936,9 @@ static unsigned long isolate_lru_pages(u
>  			/* Check that we have not crossed a zone boundary. */
>  			if (unlikely(page_zone_id(cursor_page) != zone_id))
>  				continue;
> +			/* Abort when the page is mlocked */
> +			if (unlikely(PageUnevictable(cursor_page)))
> +				break;
>  			if (__isolate_lru_page(cursor_page, mode, file) == 0) {
>  				list_move(&cursor_page->lru, dst);
>  				nr_taken++;
> 

-- 
Mel Gorman
Part-time Phd Student                          Linux Technology Center
University of Limerick                         IBM Dublin Software Lab

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

[PATCH 3/3] memcg: fix LRU rotation of isolate_lru_pages with memcg

[KAMEZAWA Hiroyuki]

From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>

This patch tries to fix memcg's lru rotation logic to make memcg use
the same logic as global LRU does.

Now, at __isolate_lru_page() retruns -EBUSY, the page is rotated to
the tail of LRU in global LRU's isolate LRU pages. But in memcg,
it's not handled. This makes memcg do the same behavior as global LRU
and rotate LRU in the page is busy.

Acked-by: Balbir Singh <balbir@linux.vnet.ibm.com>
Reviewed-by: Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp>
Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>

---
Index: lumpy-reclaim-trial/mm/vmscan.c
===================================================================
--- lumpy-reclaim-trial.orig/mm/vmscan.c	2009-06-10 19:48:28.000000000 +0900
+++ lumpy-reclaim-trial/mm/vmscan.c	2009-06-10 20:06:55.000000000 +0900
@@ -844,7 +844,6 @@
 		 */
 		ClearPageLRU(page);
 		ret = 0;
-		mem_cgroup_del_lru(page);
 	}
 
 	return ret;
@@ -892,12 +891,14 @@
 		switch (__isolate_lru_page(page, mode, file)) {
 		case 0:
 			list_move(&page->lru, dst);
+			mem_cgroup_del_lru(page);
 			nr_taken++;
 			break;
 
 		case -EBUSY:
 			/* else it is being freed elsewhere */
 			list_move(&page->lru, src);
+			mem_cgroup_rotate_lru_list(page, page_lru(page));
 			continue;
 
 		default:
@@ -941,6 +942,7 @@
 				break;
 			if (__isolate_lru_page(cursor_page, mode, file) == 0) {
 				list_move(&cursor_page->lru, dst);
+				mem_cgroup_del_lru(page);
 				nr_taken++;
 				scan++;
 				break;
Index: lumpy-reclaim-trial/mm/memcontrol.c
===================================================================
--- lumpy-reclaim-trial.orig/mm/memcontrol.c	2009-06-10 17:30:23.000000000 +0900
+++ lumpy-reclaim-trial/mm/memcontrol.c	2009-06-10 20:05:21.000000000 +0900
@@ -649,6 +649,7 @@
 	int zid = zone_idx(z);
 	struct mem_cgroup_per_zone *mz;
 	int lru = LRU_FILE * !!file + !!active;
+	int ret;
 
 	BUG_ON(!mem_cont);
 	mz = mem_cgroup_zoneinfo(mem_cont, nid, zid);
@@ -666,9 +667,19 @@
 			continue;
 
 		scan++;
-		if (__isolate_lru_page(page, mode, file) == 0) {
+		ret = __isolate_lru_page(page, mode, file);
+		switch (ret) {
+		case 0:
 			list_move(&page->lru, dst);
+			mem_cgroup_del_lru(page);
 			nr_taken++;
+			break;
+		case -EBUSY:
+			/* we don't affect global LRU but rotate in our LRU */
+			mem_cgroup_rotate_lru_list(page, page_lru(page));
+			break;
+		default:
+			break;
 		}
 	}
 

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