linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/5]  move ClearPageActive from move_active_pages() to shrink_active_list()
@ 2009-07-16  8:37 KOSAKI Motohiro
  2009-07-16  8:38 ` [PATCH 2/5] Kill unnecessary page flag test KOSAKI Motohiro
                   ` (4 more replies)
  0 siblings, 5 replies; 10+ messages in thread
From: KOSAKI Motohiro @ 2009-07-16  8:37 UTC (permalink / raw)
  To: LKML, linux-mm, Andrew Morton; +Cc: kosaki.motohiro

This patch series are several vmscan cleanups.

===================
Subject: [PATCH] move ClearPageActive from move_active_pages() to shrink_active_list()

The mvoe_active_pages_to_lru() function is called under irq disabled and
ClearPageActive() doesn't need irq disabling.

Then, this patch move it into shrink_active_list().


Signed-off-by: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
---
 mm/vmscan.c |    5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

Index: b/mm/vmscan.c
===================================================================
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -1230,10 +1230,6 @@ static void move_active_pages_to_lru(str
 		VM_BUG_ON(PageLRU(page));
 		SetPageLRU(page);
 
-		VM_BUG_ON(!PageActive(page));
-		if (!is_active_lru(lru))
-			ClearPageActive(page);	/* we are de-activating */
-
 		list_move(&page->lru, &zone->lru[lru].list);
 		mem_cgroup_add_lru_list(page, lru);
 		pgmoved++;
@@ -1315,6 +1311,7 @@ static void shrink_active_list(unsigned 
 			}
 		}
 
+		ClearPageActive(page);	/* we are de-activating */
 		list_add(&page->lru, &l_inactive);
 	}
 


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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 2/5] Kill unnecessary page flag test
  2009-07-16  8:37 [PATCH 1/5] move ClearPageActive from move_active_pages() to shrink_active_list() KOSAKI Motohiro
@ 2009-07-16  8:38 ` KOSAKI Motohiro
  2009-07-16  8:39 ` [PATCH 3/5] Kill unnecessary prefetch KOSAKI Motohiro
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 10+ messages in thread
From: KOSAKI Motohiro @ 2009-07-16  8:38 UTC (permalink / raw)
  To: LKML; +Cc: kosaki.motohiro, linux-mm, Andrew Morton

Subject: [PATCH] Kill unnecessary page flag test

The page_lru() already evaluate PageActive() and PageSwapBacked(). We
don't need to re-evaluate it.


Signed-off-by: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
---
 mm/vmscan.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Index: b/mm/vmscan.c
===================================================================
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -1160,8 +1160,8 @@ static unsigned long shrink_inactive_lis
 			SetPageLRU(page);
 			lru = page_lru(page);
 			add_page_to_lru_list(zone, page, lru);
-			if (PageActive(page)) {
-				int file = !!page_is_file_cache(page);
+			if (is_active_lru(lru)) {
+				int file = !!is_file_lru(lru);
 				reclaim_stat->recent_rotated[file]++;
 			}
 			if (!pagevec_add(&pvec, page)) {


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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 3/5]  Kill unnecessary prefetch
  2009-07-16  8:37 [PATCH 1/5] move ClearPageActive from move_active_pages() to shrink_active_list() KOSAKI Motohiro
  2009-07-16  8:38 ` [PATCH 2/5] Kill unnecessary page flag test KOSAKI Motohiro
@ 2009-07-16  8:39 ` KOSAKI Motohiro
  2009-07-16  8:40 ` [PATCH 4/5] Use add_page_to_lru_list() helper function KOSAKI Motohiro
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 10+ messages in thread
From: KOSAKI Motohiro @ 2009-07-16  8:39 UTC (permalink / raw)
  To: LKML; +Cc: kosaki.motohiro, linux-mm, Andrew Morton

Subject: Kill unnecessary prefetch

The pages in the list passed move_active_pages_to_lru() are
already touched by shrink_active_list(). IOW the prefetch in
move_active_pages_to_lru() don't populate any cache. it's pointless.

This patch remove it.


Signed-off-by: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
---
 mm/vmscan.c |    1 -
 1 file changed, 1 deletion(-)

Index: b/mm/vmscan.c
===================================================================
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -1225,7 +1225,6 @@ static void move_active_pages_to_lru(str
 
 	while (!list_empty(list)) {
 		page = lru_to_page(list);
-		prefetchw_prev_lru_page(page, list, flags);
 
 		VM_BUG_ON(PageLRU(page));
 		SetPageLRU(page);


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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 4/5] Use add_page_to_lru_list() helper function
  2009-07-16  8:37 [PATCH 1/5] move ClearPageActive from move_active_pages() to shrink_active_list() KOSAKI Motohiro
  2009-07-16  8:38 ` [PATCH 2/5] Kill unnecessary page flag test KOSAKI Motohiro
  2009-07-16  8:39 ` [PATCH 3/5] Kill unnecessary prefetch KOSAKI Motohiro
@ 2009-07-16  8:40 ` KOSAKI Motohiro
  2009-07-17 12:18   ` Peter Zijlstra
  2009-07-16  8:40 ` [PATCH 5/5] move PGDEACTIVATE modification to shrink_active_list() KOSAKI Motohiro
  2009-07-16 17:14 ` [PATCH 1/5] move ClearPageActive from move_active_pages() " Johannes Weiner
  4 siblings, 1 reply; 10+ messages in thread
From: KOSAKI Motohiro @ 2009-07-16  8:40 UTC (permalink / raw)
  To: LKML; +Cc: kosaki.motohiro, linux-mm, Andrew Morton

Subject: Use add_page_to_lru_list() helper function

add_page_to_lru_list() is equivalent to
  - add lru list (global)
  - add lru list (mem-cgroup)
  - modify zone stat

We can use it.

Signed-off-by: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
---
 mm/vmscan.c |    5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

Index: b/mm/vmscan.c
===================================================================
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -1225,12 +1225,12 @@ static void move_active_pages_to_lru(str
 
 	while (!list_empty(list)) {
 		page = lru_to_page(list);
+		list_del(&page->lru);
 
 		VM_BUG_ON(PageLRU(page));
 		SetPageLRU(page);
 
-		list_move(&page->lru, &zone->lru[lru].list);
-		mem_cgroup_add_lru_list(page, lru);
+		add_page_to_lru_list(zone, page, lru);
 		pgmoved++;
 
 		if (!pagevec_add(&pvec, page) || list_empty(list)) {
@@ -1241,7 +1241,6 @@ static void move_active_pages_to_lru(str
 			spin_lock_irq(&zone->lru_lock);
 		}
 	}
-	__mod_zone_page_state(zone, NR_LRU_BASE + lru, pgmoved);
 	if (!is_active_lru(lru))
 		__count_vm_events(PGDEACTIVATE, pgmoved);
 }


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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 5/5] move PGDEACTIVATE modification to shrink_active_list()
  2009-07-16  8:37 [PATCH 1/5] move ClearPageActive from move_active_pages() to shrink_active_list() KOSAKI Motohiro
                   ` (2 preceding siblings ...)
  2009-07-16  8:40 ` [PATCH 4/5] Use add_page_to_lru_list() helper function KOSAKI Motohiro
@ 2009-07-16  8:40 ` KOSAKI Motohiro
  2009-07-16 17:14 ` [PATCH 1/5] move ClearPageActive from move_active_pages() " Johannes Weiner
  4 siblings, 0 replies; 10+ messages in thread
From: KOSAKI Motohiro @ 2009-07-16  8:40 UTC (permalink / raw)
  To: LKML; +Cc: kosaki.motohiro, linux-mm, Andrew Morton

Subject: move PGDEACTIVATE modification to shrink_active_list()

Pgmoved accounting in move_active_pages_to_lru() doesn't make any sense.
it can be calculated in irq enabled area.

This patch move #-of-deactivating-pages calcution to shrink_active_list().

Signed-off-by: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
---
 mm/vmscan.c |    7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

Index: b/mm/vmscan.c
===================================================================
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -1217,7 +1217,6 @@ static void move_active_pages_to_lru(str
 				     struct list_head *list,
 				     enum lru_list lru)
 {
-	unsigned long pgmoved = 0;
 	struct pagevec pvec;
 	struct page *page;
 
@@ -1231,7 +1230,6 @@ static void move_active_pages_to_lru(str
 		SetPageLRU(page);
 
 		add_page_to_lru_list(zone, page, lru);
-		pgmoved++;
 
 		if (!pagevec_add(&pvec, page) || list_empty(list)) {
 			spin_unlock_irq(&zone->lru_lock);
@@ -1241,8 +1239,6 @@ static void move_active_pages_to_lru(str
 			spin_lock_irq(&zone->lru_lock);
 		}
 	}
-	if (!is_active_lru(lru))
-		__count_vm_events(PGDEACTIVATE, pgmoved);
 }
 
 static void shrink_active_list(unsigned long nr_pages, struct zone *zone,
@@ -1257,6 +1253,7 @@ static void shrink_active_list(unsigned 
 	struct page *page;
 	struct zone_reclaim_stat *reclaim_stat = get_reclaim_stat(zone, sc);
 	unsigned long nr_rotated = 0;
+	unsigned long nr_deactivate = 0;
 
 	lru_add_drain();
 	spin_lock_irq(&zone->lru_lock);
@@ -1311,6 +1308,7 @@ static void shrink_active_list(unsigned 
 
 		ClearPageActive(page);	/* we are de-activating */
 		list_add(&page->lru, &l_inactive);
+		nr_deactivate++;
 	}
 
 	/*
@@ -1324,6 +1322,7 @@ static void shrink_active_list(unsigned 
 	 * get_scan_ratio.
 	 */
 	reclaim_stat->recent_rotated[!!file] += nr_rotated;
+	__count_vm_events(PGDEACTIVATE, nr_deactivate);
 
 	move_active_pages_to_lru(zone, &l_active,
 						LRU_ACTIVE + file * LRU_FILE);


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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/5]  move ClearPageActive from move_active_pages() to shrink_active_list()
  2009-07-16  8:37 [PATCH 1/5] move ClearPageActive from move_active_pages() to shrink_active_list() KOSAKI Motohiro
                   ` (3 preceding siblings ...)
  2009-07-16  8:40 ` [PATCH 5/5] move PGDEACTIVATE modification to shrink_active_list() KOSAKI Motohiro
@ 2009-07-16 17:14 ` Johannes Weiner
  4 siblings, 0 replies; 10+ messages in thread
From: Johannes Weiner @ 2009-07-16 17:14 UTC (permalink / raw)
  To: KOSAKI Motohiro; +Cc: LKML, linux-mm, Andrew Morton

On Thu, Jul 16, 2009 at 05:37:34PM +0900, KOSAKI Motohiro wrote:
> This patch series are several vmscan cleanups.

Reviewed-by: Johannes Weiner <hannes@cmpxchg.org>

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 4/5] Use add_page_to_lru_list() helper function
  2009-07-16  8:40 ` [PATCH 4/5] Use add_page_to_lru_list() helper function KOSAKI Motohiro
@ 2009-07-17 12:18   ` Peter Zijlstra
  2009-07-20  5:37     ` KOSAKI Motohiro
  0 siblings, 1 reply; 10+ messages in thread
From: Peter Zijlstra @ 2009-07-17 12:18 UTC (permalink / raw)
  To: KOSAKI Motohiro; +Cc: LKML, linux-mm, Andrew Morton

On Thu, 2009-07-16 at 17:40 +0900, KOSAKI Motohiro wrote:
> Subject: Use add_page_to_lru_list() helper function
> 
> add_page_to_lru_list() is equivalent to
>   - add lru list (global)
>   - add lru list (mem-cgroup)
>   - modify zone stat
> 
> We can use it.
> 
> Signed-off-by: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
> ---
>  mm/vmscan.c |    5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> Index: b/mm/vmscan.c
> ===================================================================
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -1225,12 +1225,12 @@ static void move_active_pages_to_lru(str
>  
>  	while (!list_empty(list)) {
>  		page = lru_to_page(list);
> +		list_del(&page->lru);
>  
>  		VM_BUG_ON(PageLRU(page));
>  		SetPageLRU(page);
>  
> -		list_move(&page->lru, &zone->lru[lru].list);
> -		mem_cgroup_add_lru_list(page, lru);
> +		add_page_to_lru_list(zone, page, lru);
>  		pgmoved++;
>  
>  		if (!pagevec_add(&pvec, page) || list_empty(list)) {
> @@ -1241,7 +1241,6 @@ static void move_active_pages_to_lru(str
>  			spin_lock_irq(&zone->lru_lock);
>  		}
>  	}
> -	__mod_zone_page_state(zone, NR_LRU_BASE + lru, pgmoved);
>  	if (!is_active_lru(lru))
>  		__count_vm_events(PGDEACTIVATE, pgmoved);
>  }

This is a net loss, you introduce pgmoved calls to __inc_zone_state,
instead of the one __mod_zone_page_state() call.

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 4/5] Use add_page_to_lru_list() helper function
  2009-07-17 12:18   ` Peter Zijlstra
@ 2009-07-20  5:37     ` KOSAKI Motohiro
  2009-07-20  7:28       ` Peter Zijlstra
  0 siblings, 1 reply; 10+ messages in thread
From: KOSAKI Motohiro @ 2009-07-20  5:37 UTC (permalink / raw)
  To: Peter Zijlstra; +Cc: kosaki.motohiro, LKML, linux-mm, Andrew Morton

> > @@ -1241,7 +1241,6 @@ static void move_active_pages_to_lru(str
> >  			spin_lock_irq(&zone->lru_lock);
> >  		}
> >  	}
> > -	__mod_zone_page_state(zone, NR_LRU_BASE + lru, pgmoved);
> >  	if (!is_active_lru(lru))
> >  		__count_vm_events(PGDEACTIVATE, pgmoved);
> >  }
> 
> This is a net loss, you introduce pgmoved calls to __inc_zone_state,
> instead of the one __mod_zone_page_state() call.

max pgmoved is 32. 32 times __inc_zone_state() make 0 or 1 time
atomic operation (not much than two).
I don't think it reduce performance.




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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 4/5] Use add_page_to_lru_list() helper function
  2009-07-20  5:37     ` KOSAKI Motohiro
@ 2009-07-20  7:28       ` Peter Zijlstra
  2009-07-20 14:30         ` KOSAKI Motohiro
  0 siblings, 1 reply; 10+ messages in thread
From: Peter Zijlstra @ 2009-07-20  7:28 UTC (permalink / raw)
  To: KOSAKI Motohiro; +Cc: LKML, linux-mm, Andrew Morton

On Mon, 2009-07-20 at 14:37 +0900, KOSAKI Motohiro wrote:
> > > @@ -1241,7 +1241,6 @@ static void move_active_pages_to_lru(str
> > >  			spin_lock_irq(&zone->lru_lock);
> > >  		}
> > >  	}
> > > -	__mod_zone_page_state(zone, NR_LRU_BASE + lru, pgmoved);
> > >  	if (!is_active_lru(lru))
> > >  		__count_vm_events(PGDEACTIVATE, pgmoved);
> > >  }
> > 
> > This is a net loss, you introduce pgmoved calls to __inc_zone_state,
> > instead of the one __mod_zone_page_state() call.
> 
> max pgmoved is 32. 32 times __inc_zone_state() make 0 or 1 time
> atomic operation (not much than two).
> I don't think it reduce performance.

its not just atomics, count calls and branches too. It simply adds a ton
of code for no particular reason.

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 4/5] Use add_page_to_lru_list() helper function
  2009-07-20  7:28       ` Peter Zijlstra
@ 2009-07-20 14:30         ` KOSAKI Motohiro
  0 siblings, 0 replies; 10+ messages in thread
From: KOSAKI Motohiro @ 2009-07-20 14:30 UTC (permalink / raw)
  To: Peter Zijlstra; +Cc: kosaki.motohiro, LKML, linux-mm, Andrew Morton

> On Mon, 2009-07-20 at 14:37 +0900, KOSAKI Motohiro wrote:
> > > > @@ -1241,7 +1241,6 @@ static void move_active_pages_to_lru(str
> > > >  			spin_lock_irq(&zone->lru_lock);
> > > >  		}
> > > >  	}
> > > > -	__mod_zone_page_state(zone, NR_LRU_BASE + lru, pgmoved);
> > > >  	if (!is_active_lru(lru))
> > > >  		__count_vm_events(PGDEACTIVATE, pgmoved);
> > > >  }
> > > 
> > > This is a net loss, you introduce pgmoved calls to __inc_zone_state,
> > > instead of the one __mod_zone_page_state() call.
> > 
> > max pgmoved is 32. 32 times __inc_zone_state() make 0 or 1 time
> > atomic operation (not much than two).
> > I don't think it reduce performance.
> 
> its not just atomics, count calls and branches too. It simply adds a ton
> of code for no particular reason.

hm, I don't think it's tons penalty. I mean vmscan makes tons cache miss
and function calling penalty is hidden by it.
But, I agreed this patch doesn't have strong motivation. I can drop this.

Andrew, Can you please drop this patch?





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

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2009-07-20 14:31 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-07-16  8:37 [PATCH 1/5] move ClearPageActive from move_active_pages() to shrink_active_list() KOSAKI Motohiro
2009-07-16  8:38 ` [PATCH 2/5] Kill unnecessary page flag test KOSAKI Motohiro
2009-07-16  8:39 ` [PATCH 3/5] Kill unnecessary prefetch KOSAKI Motohiro
2009-07-16  8:40 ` [PATCH 4/5] Use add_page_to_lru_list() helper function KOSAKI Motohiro
2009-07-17 12:18   ` Peter Zijlstra
2009-07-20  5:37     ` KOSAKI Motohiro
2009-07-20  7:28       ` Peter Zijlstra
2009-07-20 14:30         ` KOSAKI Motohiro
2009-07-16  8:40 ` [PATCH 5/5] move PGDEACTIVATE modification to shrink_active_list() KOSAKI Motohiro
2009-07-16 17:14 ` [PATCH 1/5] move ClearPageActive from move_active_pages() " Johannes Weiner

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).