[PATCH] vmscan: Make move_active_pages_to_lru more generic

[PATCH] vmscan: Make move_active_pages_to_lru more generic

[Minchan Kim]

Now move_active_pages_to_lru can move pages into active or inactive.
if it moves the pages into inactive, it itself can clear PG_acive.
It makes the function more generic.

Signed-off-by: Minchan Kim <minchan.kim@gmail.com>
Cc: Wu Fengguang <fengguang.wu@intel.com>
Cc: Rik van Riel <riel@redhat.com>
Cc: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
Cc: Johannes Weiner <hannes@cmpxchg.org>

---
 mm/vmscan.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/mm/vmscan.c b/mm/vmscan.c
index aa4f1cb..bd408b3 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -1457,6 +1457,10 @@ static void move_active_pages_to_lru(struct zone *zone,
 		VM_BUG_ON(PageLRU(page));
 		SetPageLRU(page);
 
+		/* we are de-activating */
+		if (!is_active_lru(lru))
+			ClearPageActive(page);
+
 		list_move(&page->lru, &zone->lru[lru].list);
 		mem_cgroup_add_lru_list(page, lru);
 		pgmoved++;
@@ -1543,7 +1547,6 @@ static void shrink_active_list(unsigned long nr_pages, struct zone *zone,
 			}
 		}
 
-		ClearPageActive(page);  /* we are de-activating */
 		list_add(&page->lru, &l_inactive);
 	}
 
-- 
1.7.0.4

--
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/ .
Fight unfair telecom policy in Canada: sign http://dissolvethecrtc.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] vmscan: Make move_active_pages_to_lru more generic

[Rik van Riel]

On 11/21/2010 09:24 AM, Minchan Kim wrote:
> Now move_active_pages_to_lru can move pages into active or inactive.
> if it moves the pages into inactive, it itself can clear PG_acive.
> It makes the function more generic.

> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index aa4f1cb..bd408b3 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -1457,6 +1457,10 @@ static void move_active_pages_to_lru(struct zone *zone,
>   		VM_BUG_ON(PageLRU(page));
>   		SetPageLRU(page);
>
> +		/* we are de-activating */
> +		if (!is_active_lru(lru))
> +			ClearPageActive(page);
> +

Does that mean we also want code to ensure that pages have
the PG_active bit set when we add them to an active list?

-- 
All rights reversed

--
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/ .
Fight unfair telecom policy in Canada: sign http://dissolvethecrtc.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] vmscan: Make move_active_pages_to_lru more generic

[Minchan Kim]

On Mon, Nov 22, 2010 at 3:07 AM, Rik van Riel <riel@redhat.com> wrote:
> On 11/21/2010 09:24 AM, Minchan Kim wrote:
>>
>> Now move_active_pages_to_lru can move pages into active or inactive.
>> if it moves the pages into inactive, it itself can clear PG_acive.
>> It makes the function more generic.
>
>> diff --git a/mm/vmscan.c b/mm/vmscan.c
>> index aa4f1cb..bd408b3 100644
>> --- a/mm/vmscan.c
>> +++ b/mm/vmscan.c
>> @@ -1457,6 +1457,10 @@ static void move_active_pages_to_lru(struct zone
>> *zone,
>>                VM_BUG_ON(PageLRU(page));
>>                SetPageLRU(page);
>>
>> +               /* we are de-activating */
>> +               if (!is_active_lru(lru))
>> +                       ClearPageActive(page);
>> +
>
> Does that mean we also want code to ensure that pages have
> the PG_active bit set when we add them to an active list?

Yes. the function name is move_"active"_pages_to_lru.
So  caller have to make sure pages have PG_active.

>
> --
> All rights reversed
>



-- 
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/ .
Fight unfair telecom policy in Canada: sign http://dissolvethecrtc.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] vmscan: Make move_active_pages_to_lru more generic

[Rik van Riel]

On 11/21/2010 07:49 PM, Minchan Kim wrote:
> On Mon, Nov 22, 2010 at 3:07 AM, Rik van Riel<riel@redhat.com>  wrote:
>> On 11/21/2010 09:24 AM, Minchan Kim wrote:
>>>
>>> Now move_active_pages_to_lru can move pages into active or inactive.
>>> if it moves the pages into inactive, it itself can clear PG_acive.
>>> It makes the function more generic.
>>
>>> diff --git a/mm/vmscan.c b/mm/vmscan.c
>>> index aa4f1cb..bd408b3 100644
>>> --- a/mm/vmscan.c
>>> +++ b/mm/vmscan.c
>>> @@ -1457,6 +1457,10 @@ static void move_active_pages_to_lru(struct zone
>>> *zone,
>>>                 VM_BUG_ON(PageLRU(page));
>>>                 SetPageLRU(page);
>>>
>>> +               /* we are de-activating */
>>> +               if (!is_active_lru(lru))
>>> +                       ClearPageActive(page);
>>> +
>>
>> Does that mean we also want code to ensure that pages have
>> the PG_active bit set when we add them to an active list?
>
> Yes. the function name is move_"active"_pages_to_lru.
> So  caller have to make sure pages have PG_active.

Good point.

Acked-by: Rik van Riel <riel@redhat.com>

-- 
All rights reversed

--
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/ .
Fight unfair telecom policy in Canada: sign http://dissolvethecrtc.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] vmscan: Make move_active_pages_to_lru more generic

[Wu Fengguang]

On Sun, Nov 21, 2010 at 10:24:56PM +0800, Minchan Kim wrote:
> Now move_active_pages_to_lru can move pages into active or inactive.
> if it moves the pages into inactive, it itself can clear PG_acive.
> It makes the function more generic.

Do you plan to use this "more generic" function? Because the patch in
itself makes code slightly less efficient. It adds one "if" test, and
moves one operation into the spin lock.

Thanks,
Fengguang

--
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/ .
Fight unfair telecom policy in Canada: sign http://dissolvethecrtc.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] vmscan: Make move_active_pages_to_lru more generic

[Minchan Kim]

Hi Wu,

On Mon, Nov 22, 2010 at 10:36 AM, Wu Fengguang <fengguang.wu@intel.com> wrote:
> On Sun, Nov 21, 2010 at 10:24:56PM +0800, Minchan Kim wrote:
>> Now move_active_pages_to_lru can move pages into active or inactive.
>> if it moves the pages into inactive, it itself can clear PG_acive.
>> It makes the function more generic.
>
> Do you plan to use this "more generic" function? Because the patch in
> itself makes code slightly less efficient. It adds one "if" test, and
> moves one operation into the spin lock.

I tried to use it in my deactivate page series but couldn't use it due
some problem.
So I don't have a plan to use it. It's just my preference(and I hope
others think like me :) )

I hope function itself would be generic and clear because some people
in future might use it and reference it.
If my change makes big cost, I am not against you. otherwise, I hope
move_active_pages_to_lru become self-contained.

>
> Thanks,
> Fengguang
>



-- 
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/ .
Fight unfair telecom policy in Canada: sign http://dissolvethecrtc.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>