[PATCH 0/3] mm: three minor vmscan improvements

[PATCH 0/3] mm: three minor vmscan improvements

[Hugh Dickins]

Here are three minor improvements in vmscan.c,
based on 3.2.0-rc6-next-20111222 minus Mel's 11/11
"mm: isolate pages for immediate reclaim on their own LRU"
and its two corrections.

[PATCH 1/3] mm: test PageSwapBacked in lumpy reclaim
[PATCH 2/3] mm: cond_resched in scan_mapping_unevictable_pages
[PATCH 3/3] mm: take pagevecs off reclaim stack

 include/linux/pagevec.h |    2 -
 mm/swap.c               |   19 -----------
 mm/vmscan.c             |   62 +++++++++++++++++++++++++-------------
 3 files changed, 42 insertions(+), 41 deletions(-)

Hugh

--
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 internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH 1/3] mm: test PageSwapBacked in lumpy reclaim

[Hugh Dickins]

Lumpy reclaim does well to stop at a PageAnon when there's no swap, but
better is to stop at any PageSwapBacked, which includes shmem/tmpfs too.

Signed-off-by: Hugh Dickins <hughd@google.com>
---
 mm/vmscan.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- mmotm.orig/mm/vmscan.c	2011-12-28 12:32:02.000000000 -0800
+++ mmotm/mm/vmscan.c	2011-12-28 16:49:36.463201033 -0800
@@ -1222,7 +1222,7 @@ static unsigned long isolate_lru_pages(u
 			 * anon page which don't already have a swap slot is
 			 * pointless.
 			 */
-			if (nr_swap_pages <= 0 && PageAnon(cursor_page) &&
+			if (nr_swap_pages <= 0 && PageSwapBacked(cursor_page) &&
 			    !PageSwapCache(cursor_page))
 				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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 1/3] mm: test PageSwapBacked in lumpy reclaim

[KOSAKI Motohiro]

(12/28/11 11:35 PM), Hugh Dickins wrote:
> Lumpy reclaim does well to stop at a PageAnon when there's no swap, but
> better is to stop at any PageSwapBacked, which includes shmem/tmpfs too.
>
> Signed-off-by: Hugh Dickins<hughd@google.com>
> ---
>   mm/vmscan.c |    2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> --- mmotm.orig/mm/vmscan.c	2011-12-28 12:32:02.000000000 -0800
> +++ mmotm/mm/vmscan.c	2011-12-28 16:49:36.463201033 -0800
> @@ -1222,7 +1222,7 @@ static unsigned long isolate_lru_pages(u
>   			 * anon page which don't already have a swap slot is
>   			 * pointless.
>   			 */
> -			if (nr_swap_pages<= 0&&  PageAnon(cursor_page)&&
> +			if (nr_swap_pages<= 0&&  PageSwapBacked(cursor_page)&&
>   			!PageSwapCache(cursor_page))
>   				break;

It seems obvious.

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

Re: [PATCH 1/3] mm: test PageSwapBacked in lumpy reclaim

[Minchan Kim]

On Wed, Dec 28, 2011 at 08:35:13PM -0800, Hugh Dickins wrote:
> Lumpy reclaim does well to stop at a PageAnon when there's no swap, but
> better is to stop at any PageSwapBacked, which includes shmem/tmpfs too.
> 
> Signed-off-by: Hugh Dickins <hughd@google.com>
Reviewed-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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 1/3] mm: test PageSwapBacked in lumpy reclaim

[KAMEZAWA Hiroyuki]

On Wed, 28 Dec 2011 20:35:13 -0800 (PST)
Hugh Dickins <hughd@google.com> wrote:

> Lumpy reclaim does well to stop at a PageAnon when there's no swap, but
> better is to stop at any PageSwapBacked, which includes shmem/tmpfs too.
> 
> Signed-off-by: Hugh Dickins <hughd@google.com>

Reviewed-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH 2/3] mm: cond_resched in scan_mapping_unevictable_pages

[Hugh Dickins]

scan_mapping_unevictable_pages() is used to make SysV SHM_LOCKed pages
evictable again once the shared memory is unlocked or destroyed (the
latter seems rather a waste of time, but meets internal expectations).
It does pagevec_lookup()s across the whole object: methinks a
cond_resched() every PAGEVEC_SIZE pages would be worthwhile.

Signed-off-by: Hugh Dickins <hughd@google.com>
---
 mm/vmscan.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- mmotm.orig/mm/vmscan.c	2011-12-28 16:49:36.000000000 -0800
+++ mmotm/mm/vmscan.c	2011-12-28 17:03:07.647220248 -0800
@@ -3583,8 +3583,8 @@ void scan_mapping_unevictable_pages(stru
 		pagevec_release(&pvec);
 
 		count_vm_events(UNEVICTABLE_PGSCANNED, pg_scanned);
+		cond_resched();
 	}
-
 }
 
 static void warn_scan_unevictable_pages(void)

--
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 internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 2/3] mm: cond_resched in scan_mapping_unevictable_pages

[KOSAKI Motohiro]

(12/28/11 11:36 PM), Hugh Dickins wrote:
> scan_mapping_unevictable_pages() is used to make SysV SHM_LOCKed pages
> evictable again once the shared memory is unlocked or destroyed (the
> latter seems rather a waste of time, but meets internal expectations).
> It does pagevec_lookup()s across the whole object: methinks a
> cond_resched() every PAGEVEC_SIZE pages would be worthwhile.
>
> Signed-off-by: Hugh Dickins<hughd@google.com>
> ---
>   mm/vmscan.c |    2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> --- mmotm.orig/mm/vmscan.c	2011-12-28 16:49:36.000000000 -0800
> +++ mmotm/mm/vmscan.c	2011-12-28 17:03:07.647220248 -0800
> @@ -3583,8 +3583,8 @@ void scan_mapping_unevictable_pages(stru
>   		pagevec_release(&pvec);
>
>   		count_vm_events(UNEVICTABLE_PGSCANNED, pg_scanned);
> +		cond_resched();
>   	}

Hmm...
scan_mapping_unevictable_pages() is always under spinlock?



int shmem_lock(struct file *file, int lock, struct user_struct *user)
{
         spin_lock(&info->lock);
	(snip)
                 smp_mb__after_clear_bit();
                 scan_mapping_unevictable_pages(file->f_mapping);
	(snip)
out_nomem:
         spin_unlock(&info->lock);
         return retval;
}

--
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 internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 2/3] mm: cond_resched in scan_mapping_unevictable_pages

[Hugh Dickins]

On Thu, 29 Dec 2011, KOSAKI Motohiro wrote:
> (12/28/11 11:36 PM), Hugh Dickins wrote:
> > scan_mapping_unevictable_pages() is used to make SysV SHM_LOCKed pages
> > evictable again once the shared memory is unlocked or destroyed (the
> > latter seems rather a waste of time, but meets internal expectations).
> > It does pagevec_lookup()s across the whole object: methinks a
> > cond_resched() every PAGEVEC_SIZE pages would be worthwhile.
> > 
> > Signed-off-by: Hugh Dickins<hughd@google.com>
> > ---
> >   mm/vmscan.c |    2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > --- mmotm.orig/mm/vmscan.c	2011-12-28 16:49:36.000000000 -0800
> > +++ mmotm/mm/vmscan.c	2011-12-28 17:03:07.647220248 -0800
> > @@ -3583,8 +3583,8 @@ void scan_mapping_unevictable_pages(stru
> >   		pagevec_release(&pvec);
> > 
> >   		count_vm_events(UNEVICTABLE_PGSCANNED, pg_scanned);
> > +		cond_resched();
> >   	}
> 
> Hmm...
> scan_mapping_unevictable_pages() is always under spinlock?

Yikes, how dreadful!  Dreadful that it's like that, and dreadful
that I didn't notice.  Many thanks for spotting, consider this
patch summarily withdrawn.  All the more need for some patch like
this, but no doubt it was "easier" to do it all under the spinlock,
so the right replacement patch may not be so obvious.
 
Thanks again,
Hugh

--
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 internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 2/3] mm: cond_resched in scan_mapping_unevictable_pages

[Hugh Dickins]

On Wed, 28 Dec 2011, Hugh Dickins wrote:
> On Thu, 29 Dec 2011, KOSAKI Motohiro wrote:
> > 
> > Hmm...
> > scan_mapping_unevictable_pages() is always under spinlock?
> 
> Yikes, how dreadful!  Dreadful that it's like that, and dreadful
> that I didn't notice.  Many thanks for spotting, consider this
> patch summarily withdrawn.  All the more need for some patch like
> this, but no doubt it was "easier" to do it all under the spinlock,
> so the right replacement patch may not be so obvious.

It's not so bad, I think: that info->lock isn't really needed across
scan_mapping_unevictable_pages(), which has to deal with races on a
page by page basis anyway.

But it's not the only spinlock, there is also ipc_lock() (often) held
in the level above: so I'll have to restructure it a little.  And now
that it's no longer a one-liner, I really ought to add a patch fixing
another bug I introduced here - if there were swapped pages when the
area was SHM_LOCKed, find_get_pages() will give up if it hits a row
of PAGEVEC_SIZE swap entries, leaving subsequent pages unevictable.

But I'd better empty my queue of trivia before going on to that.

Hugh

--
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 internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH 3/3] mm: take pagevecs off reclaim stack

[Hugh Dickins]

Replace pagevecs in putback_lru_pages() and move_active_pages_to_lru()
by lists of pages_to_free: then apply Konstantin Khlebnikov's
free_hot_cold_page_list() to them instead of pagevec_release().

Which simplifies the flow (no need to drop and retake lock whenever
pagevec fills up) and reduces stale addresses in stack backtraces
(which often showed through the pagevecs); but more importantly,
removes another 120 bytes from the deepest stacks in page reclaim.
Although I've not recently seen an actual stack overflow here with
a vanilla kernel, move_active_pages_to_lru() has often featured in
deep backtraces.

However, free_hot_cold_page_list() does not handle compound pages
(nor need it: a Transparent HugePage would have been split by the
time it reaches the call in shrink_page_list()), but it is possible
for putback_lru_pages() or move_active_pages_to_lru() to be left
holding the last reference on a THP, so must exclude the unlikely
compound case before putting on pages_to_free.

Remove pagevec_strip(), its work now done in move_active_pages_to_lru().
The pagevec in scan_mapping_unevictable_pages() remains in mm/vmscan.c,
but that is never on the reclaim path, and cannot be replaced by a list.

Signed-off-by: Hugh Dickins <hughd@google.com>
---
 include/linux/pagevec.h |    2 -
 mm/swap.c               |   19 ------------
 mm/vmscan.c             |   58 ++++++++++++++++++++++++++------------
 3 files changed, 40 insertions(+), 39 deletions(-)

--- mmotm.orig/include/linux/pagevec.h	2011-12-22 02:53:31.000000000 -0800
+++ mmotm/include/linux/pagevec.h	2011-12-28 17:33:21.855263356 -0800
@@ -22,7 +22,6 @@ struct pagevec {
 
 void __pagevec_release(struct pagevec *pvec);
 void ____pagevec_lru_add(struct pagevec *pvec, enum lru_list lru);
-void pagevec_strip(struct pagevec *pvec);
 unsigned pagevec_lookup(struct pagevec *pvec, struct address_space *mapping,
 		pgoff_t start, unsigned nr_pages);
 unsigned pagevec_lookup_tag(struct pagevec *pvec,
@@ -59,7 +58,6 @@ static inline unsigned pagevec_add(struc
 	return pagevec_space(pvec);
 }
 
-
 static inline void pagevec_release(struct pagevec *pvec)
 {
 	if (pagevec_count(pvec))
--- mmotm.orig/mm/swap.c	2011-12-28 12:53:23.000000000 -0800
+++ mmotm/mm/swap.c	2011-12-28 17:37:21.071268545 -0800
@@ -23,7 +23,6 @@
 #include <linux/init.h>
 #include <linux/export.h>
 #include <linux/mm_inline.h>
-#include <linux/buffer_head.h>	/* for try_to_release_page() */
 #include <linux/percpu_counter.h>
 #include <linux/percpu.h>
 #include <linux/cpu.h>
@@ -730,24 +729,6 @@ void ____pagevec_lru_add(struct pagevec
 
 EXPORT_SYMBOL(____pagevec_lru_add);
 
-/*
- * Try to drop buffers from the pages in a pagevec
- */
-void pagevec_strip(struct pagevec *pvec)
-{
-	int i;
-
-	for (i = 0; i < pagevec_count(pvec); i++) {
-		struct page *page = pvec->pages[i];
-
-		if (page_has_private(page) && trylock_page(page)) {
-			if (page_has_private(page))
-				try_to_release_page(page, 0);
-			unlock_page(page);
-		}
-	}
-}
-
 /**
  * pagevec_lookup - gang pagecache lookup
  * @pvec:	Where the resulting pages are placed
--- mmotm.orig/mm/vmscan.c	2011-12-28 17:03:07.000000000 -0800
+++ mmotm/mm/vmscan.c	2011-12-28 17:59:24.811300757 -0800
@@ -1398,12 +1398,10 @@ putback_lru_pages(struct mem_cgroup_zone
 		  struct list_head *page_list)
 {
 	struct page *page;
-	struct pagevec pvec;
+	LIST_HEAD(pages_to_free);
 	struct zone *zone = mz->zone;
 	struct zone_reclaim_stat *reclaim_stat = get_reclaim_stat(mz);
 
-	pagevec_init(&pvec, 1);
-
 	/*
 	 * Put back any unfreeable pages.
 	 */
@@ -1427,17 +1425,24 @@ putback_lru_pages(struct mem_cgroup_zone
 			int numpages = hpage_nr_pages(page);
 			reclaim_stat->recent_rotated[file] += numpages;
 		}
-		if (!pagevec_add(&pvec, page)) {
-			spin_unlock_irq(&zone->lru_lock);
-			__pagevec_release(&pvec);
-			spin_lock_irq(&zone->lru_lock);
+		if (put_page_testzero(page)) {
+			__ClearPageLRU(page);
+			__ClearPageActive(page);
+			del_page_from_lru_list(zone, page, lru);
+
+			if (unlikely(PageCompound(page))) {
+				spin_unlock_irq(&zone->lru_lock);
+				(*get_compound_page_dtor(page))(page);
+				spin_lock_irq(&zone->lru_lock);
+			} else
+				list_add(&page->lru, &pages_to_free);
 		}
 	}
 	__mod_zone_page_state(zone, NR_ISOLATED_ANON, -nr_anon);
 	__mod_zone_page_state(zone, NR_ISOLATED_FILE, -nr_file);
 
 	spin_unlock_irq(&zone->lru_lock);
-	pagevec_release(&pvec);
+	free_hot_cold_page_list(&pages_to_free, 1);
 }
 
 static noinline_for_stack void
@@ -1647,13 +1652,23 @@ shrink_inactive_list(unsigned long nr_to
 
 static void move_active_pages_to_lru(struct zone *zone,
 				     struct list_head *list,
+				     struct list_head *pages_to_free,
 				     enum lru_list lru)
 {
 	unsigned long pgmoved = 0;
-	struct pagevec pvec;
 	struct page *page;
 
-	pagevec_init(&pvec, 1);
+	if (buffer_heads_over_limit) {
+		spin_unlock_irq(&zone->lru_lock);
+		list_for_each_entry(page, list, lru) {
+			if (page_has_private(page) && trylock_page(page)) {
+				if (page_has_private(page))
+					try_to_release_page(page, 0);
+				unlock_page(page);
+			}
+		}
+		spin_lock_irq(&zone->lru_lock);
+	}
 
 	while (!list_empty(list)) {
 		struct lruvec *lruvec;
@@ -1667,12 +1682,17 @@ static void move_active_pages_to_lru(str
 		list_move(&page->lru, &lruvec->lists[lru]);
 		pgmoved += hpage_nr_pages(page);
 
-		if (!pagevec_add(&pvec, page) || list_empty(list)) {
-			spin_unlock_irq(&zone->lru_lock);
-			if (buffer_heads_over_limit)
-				pagevec_strip(&pvec);
-			__pagevec_release(&pvec);
-			spin_lock_irq(&zone->lru_lock);
+		if (put_page_testzero(page)) {
+			__ClearPageLRU(page);
+			__ClearPageActive(page);
+			del_page_from_lru_list(zone, page, lru);
+
+			if (unlikely(PageCompound(page))) {
+				spin_unlock_irq(&zone->lru_lock);
+				(*get_compound_page_dtor(page))(page);
+				spin_lock_irq(&zone->lru_lock);
+			} else
+				list_add(&page->lru, pages_to_free);
 		}
 	}
 	__mod_zone_page_state(zone, NR_LRU_BASE + lru, pgmoved);
@@ -1766,12 +1786,14 @@ static void shrink_active_list(unsigned
 	 */
 	reclaim_stat->recent_rotated[file] += nr_rotated;
 
-	move_active_pages_to_lru(zone, &l_active,
+	move_active_pages_to_lru(zone, &l_active, &l_hold,
 						LRU_ACTIVE + file * LRU_FILE);
-	move_active_pages_to_lru(zone, &l_inactive,
+	move_active_pages_to_lru(zone, &l_inactive, &l_hold,
 						LRU_BASE   + file * LRU_FILE);
 	__mod_zone_page_state(zone, NR_ISOLATED_ANON + file, -nr_taken);
 	spin_unlock_irq(&zone->lru_lock);
+
+	free_hot_cold_page_list(&l_hold, 1);
 }
 
 #ifdef CONFIG_SWAP

--
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 internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 3/3] mm: take pagevecs off reclaim stack

[KOSAKI Motohiro]

(12/28/11 11:39 PM), Hugh Dickins wrote:
> Replace pagevecs in putback_lru_pages() and move_active_pages_to_lru()
> by lists of pages_to_free: then apply Konstantin Khlebnikov's
> free_hot_cold_page_list() to them instead of pagevec_release().
>
> Which simplifies the flow (no need to drop and retake lock whenever
> pagevec fills up) and reduces stale addresses in stack backtraces
> (which often showed through the pagevecs); but more importantly,
> removes another 120 bytes from the deepest stacks in page reclaim.
> Although I've not recently seen an actual stack overflow here with
> a vanilla kernel, move_active_pages_to_lru() has often featured in
> deep backtraces.
>
> However, free_hot_cold_page_list() does not handle compound pages
> (nor need it: a Transparent HugePage would have been split by the
> time it reaches the call in shrink_page_list()), but it is possible
> for putback_lru_pages() or move_active_pages_to_lru() to be left
> holding the last reference on a THP, so must exclude the unlikely
> compound case before putting on pages_to_free.
>
> Remove pagevec_strip(), its work now done in move_active_pages_to_lru().
> The pagevec in scan_mapping_unevictable_pages() remains in mm/vmscan.c,
> but that is never on the reclaim path, and cannot be replaced by a list.
>
> Signed-off-by: Hugh Dickins<hughd@google.com>

I haven't found any incorrect.

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

Re: [PATCH 3/3] mm: take pagevecs off reclaim stack

[Konstantin Khlebnikov]

Hugh Dickins wrote:
> Replace pagevecs in putback_lru_pages() and move_active_pages_to_lru()
> by lists of pages_to_free: then apply Konstantin Khlebnikov's
> free_hot_cold_page_list() to them instead of pagevec_release().
>
> Which simplifies the flow (no need to drop and retake lock whenever
> pagevec fills up) and reduces stale addresses in stack backtraces
> (which often showed through the pagevecs); but more importantly,
> removes another 120 bytes from the deepest stacks in page reclaim.
> Although I've not recently seen an actual stack overflow here with
> a vanilla kernel, move_active_pages_to_lru() has often featured in
> deep backtraces.
>
> However, free_hot_cold_page_list() does not handle compound pages
> (nor need it: a Transparent HugePage would have been split by the
> time it reaches the call in shrink_page_list()), but it is possible
> for putback_lru_pages() or move_active_pages_to_lru() to be left
> holding the last reference on a THP, so must exclude the unlikely
> compound case before putting on pages_to_free.
>
> Remove pagevec_strip(), its work now done in move_active_pages_to_lru().
> The pagevec in scan_mapping_unevictable_pages() remains in mm/vmscan.c,
> but that is never on the reclaim path, and cannot be replaced by a list.
>
> Signed-off-by: Hugh Dickins<hughd@google.com>

Nice patch

Reviewed-by: Konstantin Khlebnikov <khlebnikov@openvz.org>

> @@ -1398,12 +1398,10 @@ putback_lru_pages(struct mem_cgroup_zone
>   		  struct list_head *page_list)
>   {
>   	struct page *page;
> -	struct pagevec pvec;
> +	LIST_HEAD(pages_to_free);
>   	struct zone *zone = mz->zone;

As I see, this patch is on top "memcg naturalization" patchset,
it does not apply clearly against Linus tree.

 > +		if (put_page_testzero(page)) {
 > +			__ClearPageLRU(page);
 > +			__ClearPageActive(page);
 > +			del_page_from_lru_list(zone, page, lru);
 > +
 > +			if (unlikely(PageCompound(page))) {
 > +				spin_unlock_irq(&zone->lru_lock);

There is good place for VM_BUG_ON(!PageHead(page));

 > +				(*get_compound_page_dtor(page))(page);
 > +				spin_lock_irq(&zone->lru_lock);
 > +			} else
 > +				list_add(&page->lru,&pages_to_free);
 >   		}
 >   	}
 >   	__mod_zone_page_state(zone, NR_ISOLATED_ANON, -nr_anon);
 >   	__mod_zone_page_state(zone, NR_ISOLATED_FILE, -nr_file);
 >
 >   	spin_unlock_irq(&zone->lru_lock);
 > -	pagevec_release(&pvec);
 > +	free_hot_cold_page_list(&pages_to_free, 1);
 >   }
 >
 >   static noinline_for_stack void

--
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 internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 3/3] mm: take pagevecs off reclaim stack

[Hugh Dickins]

On Thu, 29 Dec 2011, Konstantin Khlebnikov wrote:
> 
> Nice patch
> 
> Reviewed-by: Konstantin Khlebnikov <khlebnikov@openvz.org>

Thanks.

> As I see, this patch is on top "memcg naturalization" patchset,
> it does not apply clearly against Linus tree.

Right, it's certainly not intended as a last-minute "fix" to 3.2,
but as a patch for mmotm and linux-next then 3.3.  Linus doesn't
even have your free_hot_cold_page_list() yet.

> 
> > +		if (put_page_testzero(page)) {
> > +			__ClearPageLRU(page);
> > +			__ClearPageActive(page);
> > +			del_page_from_lru_list(zone, page, lru);
> > +
> > +			if (unlikely(PageCompound(page))) {
> > +				spin_unlock_irq(&zone->lru_lock);
> 
> There is good place for VM_BUG_ON(!PageHead(page));

Well, my inertia wanted to find a reason to disagree with you on that,
and indeed I found one!  If this were a tail page, the preceding
put_page_testzero() should already have hit its
	VM_BUG_ON(atomic_read(&page->_count) == 0);
(since Andrea changed the THP refcounting to respect get_page_unless_zero).

Hugh

--
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 internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 3/3] mm: take pagevecs off reclaim stack

[Andrew Morton]

On Wed, 28 Dec 2011 20:39:36 -0800 (PST)
Hugh Dickins <hughd@google.com> wrote:

> Replace pagevecs in putback_lru_pages() and move_active_pages_to_lru()
> by lists of pages_to_free

One effect of the pagevec handling was to limit lru_lock hold times and
interrupt-disabled times.

This patch removes that upper bound and has the potential to cause
various latency problems when processing large numbers of pages.

The affected functions have rather a lot of callers.  I don't think
that auditing all these callers and convincing ourselves that none of
them pass in 10,000 pages is sufficient, because that doesn't prevent us
from introducing such latency problems as the MM code evolves.

--
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 internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 3/3] mm: take pagevecs off reclaim stack

[Hugh Dickins]

On Thu, 29 Dec 2011, Andrew Morton wrote:
> On Wed, 28 Dec 2011 20:39:36 -0800 (PST)
> Hugh Dickins <hughd@google.com> wrote:
> 
> > Replace pagevecs in putback_lru_pages() and move_active_pages_to_lru()
> > by lists of pages_to_free
> 
> One effect of the pagevec handling was to limit lru_lock hold times and
> interrupt-disabled times.
> 
> This patch removes that upper bound and has the potential to cause
> various latency problems when processing large numbers of pages.
> 
> The affected functions have rather a lot of callers.  I don't think
> that auditing all these callers and convincing ourselves that none of
> them pass in 10,000 pages is sufficient, because that doesn't prevent us
> from introducing such latency problems as the MM code evolves.

That's an interesting slant on it, that hadn't crossed my mind;
but it looks like intervening changes have answered that concern.

putback_lru_pages() has one caller, shrink_inactive_list();
move_active_pages_to_lru() has one caller, shrink_active_list().
Following those back, they're in all cases capped to SWAP_CLUSTER_MAX
pages per call.  That's 32 pages, not so very much more than the 14
page limit the pagevecs were imposing.

And both shrink_inactive_list() and shrink_active_list() gather these
pages with isolate_lru_pages(), which does not drop lock or enable
interrupts at all - probably why the SWAP_CLUSTER_MAX cap got imposed.

(Don't be deceived by mm/migrate.c's putback_lru_pages()!
That's a distinct function, unaffected by this patch.)

Hugh

--
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 internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 3/3] mm: take pagevecs off reclaim stack

[KOSAKI Motohiro]

(12/29/11 6:27 PM), Hugh Dickins wrote:
> On Thu, 29 Dec 2011, Andrew Morton wrote:
>> On Wed, 28 Dec 2011 20:39:36 -0800 (PST)
>> Hugh Dickins<hughd@google.com>  wrote:
>>
>>> Replace pagevecs in putback_lru_pages() and move_active_pages_to_lru()
>>> by lists of pages_to_free
>>
>> One effect of the pagevec handling was to limit lru_lock hold times and
>> interrupt-disabled times.
>>
>> This patch removes that upper bound and has the potential to cause
>> various latency problems when processing large numbers of pages.
>>
>> The affected functions have rather a lot of callers.  I don't think
>> that auditing all these callers and convincing ourselves that none of
>> them pass in 10,000 pages is sufficient, because that doesn't prevent us
>> from introducing such latency problems as the MM code evolves.
>
> That's an interesting slant on it, that hadn't crossed my mind;
> but it looks like intervening changes have answered that concern.
>
> putback_lru_pages() has one caller, shrink_inactive_list();
> move_active_pages_to_lru() has one caller, shrink_active_list().
> Following those back, they're in all cases capped to SWAP_CLUSTER_MAX
> pages per call.  That's 32 pages, not so very much more than the 14
> page limit the pagevecs were imposing.
>
> And both shrink_inactive_list() and shrink_active_list() gather these
> pages with isolate_lru_pages(), which does not drop lock or enable
> interrupts at all - probably why the SWAP_CLUSTER_MAX cap got imposed.

When lumpy reclaim occur, isolate_lru_pages() gather much pages than
SWAP_CLUSTER_MAX. However, at that time, I think this patch behave
better than old. If we release and retake zone lock per 14 pages,
other tasks can easily steal a part of lumpy reclaimed pages. and then
long latency wrongness will be happen when system is under large page
memory allocation pressure. That's the reason why I posted very similar 
patch a long time ago.


--
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 internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 3/3] mm: take pagevecs off reclaim stack

[Hugh Dickins]

On Thu, 29 Dec 2011, KOSAKI Motohiro wrote:
> 
> When lumpy reclaim occur, isolate_lru_pages() gather much pages than
> SWAP_CLUSTER_MAX.

Oh, good point, I hadn't thought of that.

> However, at that time, I think this patch behave
> better than old. If we release and retake zone lock per 14 pages,
> other tasks can easily steal a part of lumpy reclaimed pages. and then
> long latency wrongness will be happen when system is under large page
> memory allocation pressure. That's the reason why I posted very similar patch
> a long time ago.

Aha, and another good point.  Thank you.

Hugh

--
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 internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 3/3] mm: take pagevecs off reclaim stack

[Andrew Morton]

On Thu, 29 Dec 2011 17:55:14 -0800 (PST) Hugh Dickins <hughd@google.com> wrote:

> > However, at that time, I think this patch behave
> > better than old. If we release and retake zone lock per 14 pages,
> > other tasks can easily steal a part of lumpy reclaimed pages. and then
> > long latency wrongness will be happen when system is under large page
> > memory allocation pressure. That's the reason why I posted very similar patch
> > a long time ago.
> 
> Aha, and another good point.  Thank you.

I hope you understand it better than I :(

Long lock hold times and long irq-off times are demonstrable problems
which have hurt people in the past.  Whereas the someone-stole-my-page
issue is theoretical, undemonstrated and unquantified.  And for
some people, lengthy worst-case latency is a serious problem, doesn't
matter whether the system is under memory pressure or not - that simply
determines when the worst-case hits.

This is not all some handwavy theoretical thing either.  If we've gone
and introduced serious latency issues, people *will* hit them and treat
it as a regression.


Now, a way out here is to remove lumpy reclaim (please).  And make the
problem not come back by promising to never call putback_lru_pages(lots
of pages) (how do we do this?).

So I think the best way ahead here is to distribute this patch in the
same release in which we remove lumpy reclaim (pokes Mel).

--
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 internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 3/3] mm: take pagevecs off reclaim stack

[Mel Gorman]

On Thu, Dec 29, 2011 at 07:59:17PM -0800, Andrew Morton wrote:
> <SNIP>
>
> Now, a way out here is to remove lumpy reclaim (please).  And make the
> problem not come back by promising to never call putback_lru_pages(lots
> of pages) (how do we do this?).
> 
> So I think the best way ahead here is to distribute this patch in the
> same release in which we remove lumpy reclaim (pokes Mel).
> 

I'll take a look at this in the New Years. Removing lumpy reclaim is
straight-forward. Making sure that it does not ruin THP allocation
success rates will be the time consuming part.

-- 
Mel Gorman
SUSE Labs

--
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 internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 3/3] mm: take pagevecs off reclaim stack

[Hugh Dickins]

On Thu, 29 Dec 2011, Andrew Morton wrote:
> 
> This is not all some handwavy theoretical thing either.  If we've gone
> and introduced serious latency issues, people *will* hit them and treat
> it as a regression.

Sure, though the worst I've seen so far (probably haven't been trying
hard enough yet, I need to go for THPs) is 39 pages freed in one call.

Regression?  Well, any bad latency would already have been there on
the gathering side.

> 
> Now, a way out here is to remove lumpy reclaim (please).  And make the
> problem not come back by promising to never call putback_lru_pages(lots
> of pages) (how do we do this?).

We can very easily put a counter in it, doing a spin_unlock_irq every
time we hit the max.  Nothing prevents that, it's just an excrescence
I'd have preferred to omit and have not today implemented.

> 
> So I think the best way ahead here is to distribute this patch in the
> same release in which we remove lumpy reclaim (pokes Mel).

I'm sure there are better reasons for removing lumpy than that I posted
a patch which happened to remove some limitation.  No need to poke Mel
on my behalf!

Hugh

--
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 internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 3/3] mm: take pagevecs off reclaim stack

[Andrew Morton]

On Sat, 31 Dec 2011 23:18:15 -0800 (PST)
Hugh Dickins <hughd@google.com> wrote:

> On Thu, 29 Dec 2011, Andrew Morton wrote:
> > 
> > This is not all some handwavy theoretical thing either.  If we've gone
> > and introduced serious latency issues, people *will* hit them and treat
> > it as a regression.
> 
> Sure, though the worst I've seen so far (probably haven't been trying
> hard enough yet, I need to go for THPs) is 39 pages freed in one call.

39 is OK.  How hugepage-intensive was the workload?

> Regression?  Well, any bad latency would already have been there on
> the gathering side.
> 
> > 
> > Now, a way out here is to remove lumpy reclaim (please).  And make the
> > problem not come back by promising to never call putback_lru_pages(lots
> > of pages) (how do we do this?).
> 
> We can very easily put a counter in it, doing a spin_unlock_irq every
> time we hit the max.  Nothing prevents that, it's just an excrescence
> I'd have preferred to omit and have not today implemented.

Yes.  It's ultra-cautious, but perhaps we should do this at least until
lumpy goes away.

> > 
> > So I think the best way ahead here is to distribute this patch in the
> > same release in which we remove lumpy reclaim (pokes Mel).
> 
> I'm sure there are better reasons for removing lumpy than that I posted
> a patch which happened to remove some limitation.  No need to poke Mel
> on my behalf!

No harm done - Mel's been getting rather unpoked lately.

Not that poking works very well anyway <checks to see if mm/thrash.c
is still there>

--
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 internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 3/3] mm: take pagevecs off reclaim stack

[KOSAKI Motohiro]

>> I'm sure there are better reasons for removing lumpy than that I posted
>> a patch which happened to remove some limitation.  No need to poke Mel
>> on my behalf!
>
> No harm done - Mel's been getting rather unpoked lately.
>
> Not that poking works very well anyway<checks to see if mm/thrash.c
> is still there>

Maybe I touched mm/thrash.c at last. Do you have any problem in it?


--
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 internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 3/3] mm: take pagevecs off reclaim stack

[Andrew Morton]

On Tue, 03 Jan 2012 18:17:43 -0500
KOSAKI Motohiro <kosaki.motohiro@gmail.com> wrote:

> >> I'm sure there are better reasons for removing lumpy than that I posted
> >> a patch which happened to remove some limitation.  No need to poke Mel
> >> on my behalf!
> >
> > No harm done - Mel's been getting rather unpoked lately.
> >
> > Not that poking works very well anyway<checks to see if mm/thrash.c
> > is still there>
> 
> Maybe I touched mm/thrash.c at last. Do you have any problem in it?
> 

https://lkml.org/lkml/2011/8/27/13

The thrash detection logic was accidentally disabled over a year ago,
and nobody has noticed.  To me, this information says "kill it".

--
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 internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 3/3] mm: take pagevecs off reclaim stack

[KOSAKI Motohiro]

(1/3/12 6:29 PM), Andrew Morton wrote:
> On Tue, 03 Jan 2012 18:17:43 -0500
> KOSAKI Motohiro<kosaki.motohiro@gmail.com>  wrote:
>
>>>> I'm sure there are better reasons for removing lumpy than that I posted
>>>> a patch which happened to remove some limitation.  No need to poke Mel
>>>> on my behalf!
>>>
>>> No harm done - Mel's been getting rather unpoked lately.
>>>
>>> Not that poking works very well anyway<checks to see if mm/thrash.c
>>> is still there>
>>
>> Maybe I touched mm/thrash.c at last. Do you have any problem in it?
>>
>
> https://lkml.org/lkml/2011/8/27/13
>
> The thrash detection logic was accidentally disabled over a year ago,
> and nobody has noticed.  To me, this information says "kill it".

Thanks, I'm looking at it later.


--
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 internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 3/3] mm: take pagevecs off reclaim stack

[Hugh Dickins]

On Tue, 3 Jan 2012, Andrew Morton wrote:
> On Sat, 31 Dec 2011 23:18:15 -0800 (PST)
> Hugh Dickins <hughd@google.com> wrote:
> 
> > On Thu, 29 Dec 2011, Andrew Morton wrote:
> > > 
> > > This is not all some handwavy theoretical thing either.  If we've gone
> > > and introduced serious latency issues, people *will* hit them and treat
> > > it as a regression.
> > 
> > Sure, though the worst I've seen so far (probably haven't been trying
> > hard enough yet, I need to go for THPs) is 39 pages freed in one call.
> 
> 39 is OK.  How hugepage-intensive was the workload?

Not very hugepagey at all.  I've since tried harder, and the most I've
seen is 523 - I expect you to be more disagreeable about that number!

And we should be able to see twice that on i386 without PAE, though
I don't suppose there's a vital market for THP in that direction.

> 
> > Regression?  Well, any bad latency would already have been there on
> > the gathering side.

I did check whether similar numbers were coming out of isolate_lru_pages
(it could have been that only a hugepage was gathered, but then split
into many by the threat of swapping); yes, similar numbers at that end.

So using page_list in putback_lru/inactive_pages would not be increasing
the worst latency, just doubling its frequency.  (Assuming that isolating
and putting back have the same cost: my guess is roughly the same, but
I've not measured.)

> > > 
> > > Now, a way out here is to remove lumpy reclaim (please).  And make the
> > > problem not come back by promising to never call putback_lru_pages(lots
> > > of pages) (how do we do this?).
> > 
> > We can very easily put a counter in it, doing a spin_unlock_irq every
> > time we hit the max.  Nothing prevents that, it's just an excrescence
> > I'd have preferred to omit and have not today implemented.
> 
> Yes.  It's ultra-cautious, but perhaps we should do this at least until
> lumpy goes away.

I don't think you'll accept my observations above as excuse to do
nothing, but please clarify which you think is more cautious.  Should
I or should I not break up the isolating end in the same way as the
putting back?

I imagine breaking in every SWAP_CLUSTER_MAX 32, so the common order
0 isn't slowed at all; hmm, maybe add on (1 << PAGE_ALLOC_COSTLY_ORDER)
8 so Kosaki-san's point is respected at least for the uncostly orders.

Hugh

--
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 internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 3/3] mm: take pagevecs off reclaim stack

[Andrew Morton]

On Tue, 3 Jan 2012 19:22:42 -0800 (PST)
Hugh Dickins <hughd@google.com> wrote:

> > > > 
> > > > Now, a way out here is to remove lumpy reclaim (please).  And make the
> > > > problem not come back by promising to never call putback_lru_pages(lots
> > > > of pages) (how do we do this?).
> > > 
> > > We can very easily put a counter in it, doing a spin_unlock_irq every
> > > time we hit the max.  Nothing prevents that, it's just an excrescence
> > > I'd have preferred to omit and have not today implemented.
> > 
> > Yes.  It's ultra-cautious, but perhaps we should do this at least until
> > lumpy goes away.
> 
> I don't think you'll accept my observations above as excuse to do
> nothing, but please clarify which you think is more cautious.  Should
> I or should I not break up the isolating end in the same way as the
> putting back?

If we already have the latency problem at the isolate_lru_pages() stage
then I suppose we can assume that nobody is noticing it, so we'll
probably be OK.

For a while.  Someone will complain at some stage and we'll probably
end up busting this work into chunks.

--
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 internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>