Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1415405 > unrolled thread

[PATCH 05/10] mm: remove LRU balancing effect of temporary page isolation

Started byJohannes Weiner <hannes@cmpxchg.org>
First post2016-06-06 22:00 +0200
Last post2016-06-08 18:10 +0200
Articles 10 — 4 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  [PATCH 05/10] mm: remove LRU balancing effect of temporary page isolation Johannes Weiner <hannes@cmpxchg.org> - 2016-06-06 22:00 +0200
    Re: [PATCH 05/10] mm: remove LRU balancing effect of temporary page  isolation Rik van Riel <riel@redhat.com> - 2016-06-07 00:00 +0200
      Re: [PATCH 05/10] mm: remove LRU balancing effect of temporary page  isolation Johannes Weiner <hannes@cmpxchg.org> - 2016-06-07 00:20 +0200
        Re: [PATCH 05/10] mm: remove LRU balancing effect of temporary page  isolation Rik van Riel <riel@redhat.com> - 2016-06-07 03:20 +0200
          Re: [PATCH 05/10] mm: remove LRU balancing effect of temporary page  isolation Johannes Weiner <hannes@cmpxchg.org> - 2016-06-07 16:00 +0200
        Re: [PATCH 05/10] mm: remove LRU balancing effect of temporary page  isolation Michal Hocko <mhocko@kernel.org> - 2016-06-07 11:30 +0200
          Re: [PATCH 05/10] mm: remove LRU balancing effect of temporary page  isolation Johannes Weiner <hannes@cmpxchg.org> - 2016-06-07 16:10 +0200
    Re: [PATCH 05/10] mm: remove LRU balancing effect of temporary page  isolation Michal Hocko <mhocko@kernel.org> - 2016-06-07 12:00 +0200
    Re: [PATCH 05/10] mm: remove LRU balancing effect of temporary page  isolation Minchan Kim <minchan@kernel.org> - 2016-06-08 09:40 +0200
      Re: [PATCH 05/10] mm: remove LRU balancing effect of temporary page  isolation Johannes Weiner <hannes@cmpxchg.org> - 2016-06-08 18:10 +0200

#1415405 — [PATCH 05/10] mm: remove LRU balancing effect of temporary page isolation

FromJohannes Weiner <hannes@cmpxchg.org>
Date2016-06-06 22:00 +0200
Subject[PATCH 05/10] mm: remove LRU balancing effect of temporary page isolation
Message-ID<rH8QO-Oc-33@gated-at.bofh.it>
Isolating an existing LRU page and subsequently putting it back on the
list currently influences the balance between the anon and file LRUs.
For example, heavy page migration or compaction could influence the
balance between the LRUs and make one type more attractive when that
type of page is affected more than the other. That doesn't make sense.

Add a dedicated LRU cache for putback, so that we can tell new LRU
pages from existing ones at the time of linking them to the lists.

Signed-off-by: Johannes Weiner <hannes@cmpxchg.org>
---
 include/linux/pagevec.h |  2 +-
 include/linux/swap.h    |  1 +
 mm/mlock.c              |  2 +-
 mm/swap.c               | 34 ++++++++++++++++++++++++++++------
 mm/vmscan.c             |  2 +-
 5 files changed, 32 insertions(+), 9 deletions(-)

diff --git a/include/linux/pagevec.h b/include/linux/pagevec.h
index b45d391b4540..3f8a2a01131c 100644
--- a/include/linux/pagevec.h
+++ b/include/linux/pagevec.h
@@ -21,7 +21,7 @@ struct pagevec {
 };
 
 void __pagevec_release(struct pagevec *pvec);
-void __pagevec_lru_add(struct pagevec *pvec);
+void __pagevec_lru_add(struct pagevec *pvec, bool new);
 unsigned pagevec_lookup_entries(struct pagevec *pvec,
 				struct address_space *mapping,
 				pgoff_t start, unsigned nr_entries,
diff --git a/include/linux/swap.h b/include/linux/swap.h
index 38fe1e91ba55..178f084365c2 100644
--- a/include/linux/swap.h
+++ b/include/linux/swap.h
@@ -296,6 +296,7 @@ extern unsigned long nr_free_pagecache_pages(void);
 
 /* linux/mm/swap.c */
 extern void lru_cache_add(struct page *);
+extern void lru_cache_putback(struct page *page);
 extern void lru_add_page_tail(struct page *page, struct page *page_tail,
 			 struct lruvec *lruvec, struct list_head *head);
 extern void activate_page(struct page *);
diff --git a/mm/mlock.c b/mm/mlock.c
index 96f001041928..449c291a286d 100644
--- a/mm/mlock.c
+++ b/mm/mlock.c
@@ -264,7 +264,7 @@ static void __putback_lru_fast(struct pagevec *pvec, int pgrescued)
 	 *__pagevec_lru_add() calls release_pages() so we don't call
 	 * put_page() explicitly
 	 */
-	__pagevec_lru_add(pvec);
+	__pagevec_lru_add(pvec, false);
 	count_vm_events(UNEVICTABLE_PGRESCUED, pgrescued);
 }
 
diff --git a/mm/swap.c b/mm/swap.c
index c6936507abb5..576c721f210b 100644
--- a/mm/swap.c
+++ b/mm/swap.c
@@ -44,6 +44,7 @@
 int page_cluster;
 
 static DEFINE_PER_CPU(struct pagevec, lru_add_pvec);
+static DEFINE_PER_CPU(struct pagevec, lru_putback_pvec);
 static DEFINE_PER_CPU(struct pagevec, lru_rotate_pvecs);
 static DEFINE_PER_CPU(struct pagevec, lru_deactivate_file_pvecs);
 static DEFINE_PER_CPU(struct pagevec, lru_deactivate_pvecs);
@@ -405,12 +406,23 @@ void lru_cache_add(struct page *page)
 
 	get_page(page);
 	if (!pagevec_space(pvec))
-		__pagevec_lru_add(pvec);
+		__pagevec_lru_add(pvec, true);
 	pagevec_add(pvec, page);
 	put_cpu_var(lru_add_pvec);
 }
 EXPORT_SYMBOL(lru_cache_add);
 
+void lru_cache_putback(struct page *page)
+{
+	struct pagevec *pvec = &get_cpu_var(lru_putback_pvec);
+
+	get_page(page);
+	if (!pagevec_space(pvec))
+		__pagevec_lru_add(pvec, false);
+	pagevec_add(pvec, page);
+	put_cpu_var(lru_putback_pvec);
+}
+
 /**
  * add_page_to_unevictable_list - add a page to the unevictable list
  * @page:  the page to be added to the unevictable list
@@ -561,10 +573,15 @@ static void lru_deactivate_fn(struct page *page, struct lruvec *lruvec,
  */
 void lru_add_drain_cpu(int cpu)
 {
-	struct pagevec *pvec = &per_cpu(lru_add_pvec, cpu);
+	struct pagevec *pvec;
+
+	pvec = &per_cpu(lru_add_pvec, cpu);
+	if (pagevec_count(pvec))
+		__pagevec_lru_add(pvec, true);
 
+	pvec = &per_cpu(lru_putback_pvec, cpu);
 	if (pagevec_count(pvec))
-		__pagevec_lru_add(pvec);
+		__pagevec_lru_add(pvec, false);
 
 	pvec = &per_cpu(lru_rotate_pvecs, cpu);
 	if (pagevec_count(pvec)) {
@@ -819,12 +836,17 @@ static void __pagevec_lru_add_fn(struct page *page, struct lruvec *lruvec,
 	int file = page_is_file_cache(page);
 	int active = PageActive(page);
 	enum lru_list lru = page_lru(page);
+	bool new = (bool)arg;
 
 	VM_BUG_ON_PAGE(PageLRU(page), page);
 
 	SetPageLRU(page);
 	add_page_to_lru_list(page, lruvec, lru);
-	update_page_reclaim_stat(lruvec, file, active, hpage_nr_pages(page));
+
+	if (new)
+		update_page_reclaim_stat(lruvec, file, active,
+					 hpage_nr_pages(page));
+
 	trace_mm_lru_insertion(page, lru);
 }
 
@@ -832,9 +854,9 @@ static void __pagevec_lru_add_fn(struct page *page, struct lruvec *lruvec,
  * Add the passed pages to the LRU, then drop the caller's refcount
  * on them.  Reinitialises the caller's pagevec.
  */
-void __pagevec_lru_add(struct pagevec *pvec)
+void __pagevec_lru_add(struct pagevec *pvec, bool new)
 {
-	pagevec_lru_move_fn(pvec, __pagevec_lru_add_fn, NULL);
+	pagevec_lru_move_fn(pvec, __pagevec_lru_add_fn, (void *)new);
 }
 
 /**
diff --git a/mm/vmscan.c b/mm/vmscan.c
index f79010bbcdd4..8503713bb60e 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -737,7 +737,7 @@ redo:
 		 * We know how to handle that.
 		 */
 		is_unevictable = false;
-		lru_cache_add(page);
+		lru_cache_putback(page);
 	} else {
 		/*
 		 * Put unevictable pages directly on zone's unevictable
-- 
2.8.3

[toc] | [next] | [standalone]


#1415509 — Re: [PATCH 05/10] mm: remove LRU balancing effect of temporary page isolation

FromRik van Riel <riel@redhat.com>
Date2016-06-07 00:00 +0200
SubjectRe: [PATCH 05/10] mm: remove LRU balancing effect of temporary page isolation
Message-ID<rHaIW-25i-27@gated-at.bofh.it>
In reply to#1415405

[Multipart message — attachments visible in raw view] — view raw

On Mon, 2016-06-06 at 15:48 -0400, Johannes Weiner wrote:
> 
> +void lru_cache_putback(struct page *page)
> +{
> +	struct pagevec *pvec = &get_cpu_var(lru_putback_pvec);
> +
> +	get_page(page);
> +	if (!pagevec_space(pvec))
> +		__pagevec_lru_add(pvec, false);
> +	pagevec_add(pvec, page);
> +	put_cpu_var(lru_putback_pvec);
> +}
> 

Wait a moment.

So now we have a putback_lru_page, which does adjust
the statistics, and an lru_cache_putback which does
not?

This function could use a name that is not as similar
to its counterpart :)

-- 
All Rights Reversed.

[toc] | [prev] | [next] | [standalone]


#1415517 — Re: [PATCH 05/10] mm: remove LRU balancing effect of temporary page isolation

FromJohannes Weiner <hannes@cmpxchg.org>
Date2016-06-07 00:20 +0200
SubjectRe: [PATCH 05/10] mm: remove LRU balancing effect of temporary page isolation
Message-ID<rHb2h-2sU-13@gated-at.bofh.it>
In reply to#1415509
On Mon, Jun 06, 2016 at 05:56:09PM -0400, Rik van Riel wrote:
> On Mon, 2016-06-06 at 15:48 -0400, Johannes Weiner wrote:
> > 
> > +void lru_cache_putback(struct page *page)
> > +{
> > +	struct pagevec *pvec = &get_cpu_var(lru_putback_pvec);
> > +
> > +	get_page(page);
> > +	if (!pagevec_space(pvec))
> > +		__pagevec_lru_add(pvec, false);
> > +	pagevec_add(pvec, page);
> > +	put_cpu_var(lru_putback_pvec);
> > +}
> > 
> 
> Wait a moment.
> 
> So now we have a putback_lru_page, which does adjust
> the statistics, and an lru_cache_putback which does
> not?
> 
> This function could use a name that is not as similar
> to its counterpart :)

lru_cache_add() and lru_cache_putback() are the two sibling functions,
where the first influences the LRU balance and the second one doesn't.

The last hunk in the patch (obscured by showing the label instead of
the function name as context) updates putback_lru_page() from using
lru_cache_add() to using lru_cache_putback().

Does that make sense?

[toc] | [prev] | [next] | [standalone]


#1415612 — Re: [PATCH 05/10] mm: remove LRU balancing effect of temporary page isolation

FromRik van Riel <riel@redhat.com>
Date2016-06-07 03:20 +0200
SubjectRe: [PATCH 05/10] mm: remove LRU balancing effect of temporary page isolation
Message-ID<rHdQt-4hl-5@gated-at.bofh.it>
In reply to#1415517

[Multipart message — attachments visible in raw view] — view raw

On Mon, 2016-06-06 at 18:15 -0400, Johannes Weiner wrote:
> On Mon, Jun 06, 2016 at 05:56:09PM -0400, Rik van Riel wrote:
> > 
> > On Mon, 2016-06-06 at 15:48 -0400, Johannes Weiner wrote:
> > > 
> > >  
> > > +void lru_cache_putback(struct page *page)
> > > +{
> > > +	struct pagevec *pvec = &get_cpu_var(lru_putback_pvec);
> > > +
> > > +	get_page(page);
> > > +	if (!pagevec_space(pvec))
> > > +		__pagevec_lru_add(pvec, false);
> > > +	pagevec_add(pvec, page);
> > > +	put_cpu_var(lru_putback_pvec);
> > > +}
> > > 
> > Wait a moment.
> > 
> > So now we have a putback_lru_page, which does adjust
> > the statistics, and an lru_cache_putback which does
> > not?
> > 
> > This function could use a name that is not as similar
> > to its counterpart :)
> lru_cache_add() and lru_cache_putback() are the two sibling
> functions,
> where the first influences the LRU balance and the second one
> doesn't.
> 
> The last hunk in the patch (obscured by showing the label instead of
> the function name as context) updates putback_lru_page() from using
> lru_cache_add() to using lru_cache_putback().
> 
> Does that make sense?

That means the page reclaim does not update the
"rotated" statistics.  That seems undesirable,
no?  Am I overlooking something?


-- 
All Rights Reversed.

[toc] | [prev] | [next] | [standalone]


#1416220 — Re: [PATCH 05/10] mm: remove LRU balancing effect of temporary page isolation

FromJohannes Weiner <hannes@cmpxchg.org>
Date2016-06-07 16:00 +0200
SubjectRe: [PATCH 05/10] mm: remove LRU balancing effect of temporary page isolation
Message-ID<rHpHY-3oH-35@gated-at.bofh.it>
In reply to#1415612
On Mon, Jun 06, 2016 at 09:11:18PM -0400, Rik van Riel wrote:
> On Mon, 2016-06-06 at 18:15 -0400, Johannes Weiner wrote:
> > On Mon, Jun 06, 2016 at 05:56:09PM -0400, Rik van Riel wrote:
> > > 
> > > On Mon, 2016-06-06 at 15:48 -0400, Johannes Weiner wrote:
> > > > 
> > > >  
> > > > +void lru_cache_putback(struct page *page)
> > > > +{
> > > > +	struct pagevec *pvec = &get_cpu_var(lru_putback_pvec);
> > > > +
> > > > +	get_page(page);
> > > > +	if (!pagevec_space(pvec))
> > > > +		__pagevec_lru_add(pvec, false);
> > > > +	pagevec_add(pvec, page);
> > > > +	put_cpu_var(lru_putback_pvec);
> > > > +}
> > > > 
> > > Wait a moment.
> > > 
> > > So now we have a putback_lru_page, which does adjust
> > > the statistics, and an lru_cache_putback which does
> > > not?
> > > 
> > > This function could use a name that is not as similar
> > > to its counterpart :)
> > lru_cache_add() and lru_cache_putback() are the two sibling
> > functions,
> > where the first influences the LRU balance and the second one
> > doesn't.
> > 
> > The last hunk in the patch (obscured by showing the label instead of
> > the function name as context) updates putback_lru_page() from using
> > lru_cache_add() to using lru_cache_putback().
> > 
> > Does that make sense?
> 
> That means the page reclaim does not update the
> "rotated" statistics.  That seems undesirable,
> no?  Am I overlooking something?

Oh, reclaim doesn't use putback_lru_page(), except for the stray
unevictable corner case. It does open-coded putback in batch, and
those functions continue to update the reclaim statistics. See the
recent_scanned/recent_rotated manipulations in putback_inactive_pages(),
shrink_inactive_list(), and shrink_active_list().

putback_lru_page() is mainly used by page migration, cgroup migration,
mlock etc. - all operations which muck with the LRU for purposes other
than reclaim or aging, and so shouldn't affect the anon/file balance.

This patch only changes those LRU users, not page reclaim.

[toc] | [prev] | [next] | [standalone]


#1415919 — Re: [PATCH 05/10] mm: remove LRU balancing effect of temporary page isolation

FromMichal Hocko <mhocko@kernel.org>
Date2016-06-07 11:30 +0200
SubjectRe: [PATCH 05/10] mm: remove LRU balancing effect of temporary page isolation
Message-ID<rHluG-Te-21@gated-at.bofh.it>
In reply to#1415517
On Mon 06-06-16 18:15:50, Johannes Weiner wrote:
[...]
> The last hunk in the patch (obscured by showing the label instead of
> the function name as context)

JFYI my ~/.gitconfig has the following to workaround this:
[diff "default"]
        xfuncname = "^[[:alpha:]$_].*[^:]$"

-- 
Michal Hocko
SUSE Labs

[toc] | [prev] | [next] | [standalone]


#1416230 — Re: [PATCH 05/10] mm: remove LRU balancing effect of temporary page isolation

FromJohannes Weiner <hannes@cmpxchg.org>
Date2016-06-07 16:10 +0200
SubjectRe: [PATCH 05/10] mm: remove LRU balancing effect of temporary page isolation
Message-ID<rHpRD-3Ha-15@gated-at.bofh.it>
In reply to#1415919
On Tue, Jun 07, 2016 at 11:26:29AM +0200, Michal Hocko wrote:
> On Mon 06-06-16 18:15:50, Johannes Weiner wrote:
> [...]
> > The last hunk in the patch (obscured by showing the label instead of
> > the function name as context)
> 
> JFYI my ~/.gitconfig has the following to workaround this:
> [diff "default"]
>         xfuncname = "^[[:alpha:]$_].*[^:]$"

Thanks, that's useful. I added it to my ~/.gitconfig, so this should
be a little less confusing in v2.

[toc] | [prev] | [next] | [standalone]


#1415948 — Re: [PATCH 05/10] mm: remove LRU balancing effect of temporary page isolation

FromMichal Hocko <mhocko@kernel.org>
Date2016-06-07 12:00 +0200
SubjectRe: [PATCH 05/10] mm: remove LRU balancing effect of temporary page isolation
Message-ID<rHlXH-13D-17@gated-at.bofh.it>
In reply to#1415405
On Mon 06-06-16 15:48:31, Johannes Weiner wrote:
> Isolating an existing LRU page and subsequently putting it back on the
> list currently influences the balance between the anon and file LRUs.
> For example, heavy page migration or compaction could influence the
> balance between the LRUs and make one type more attractive when that
> type of page is affected more than the other. That doesn't make sense.
> 
> Add a dedicated LRU cache for putback, so that we can tell new LRU
> pages from existing ones at the time of linking them to the lists.

It is far from trivial to review this one (there are quite some callers)
but it makes sense to me from the semantic point of view.
 
> Signed-off-by: Johannes Weiner <hannes@cmpxchg.org>

Acked-by: Michal Hocko <mhocko@suse.com>

> ---
>  include/linux/pagevec.h |  2 +-
>  include/linux/swap.h    |  1 +
>  mm/mlock.c              |  2 +-
>  mm/swap.c               | 34 ++++++++++++++++++++++++++++------
>  mm/vmscan.c             |  2 +-
>  5 files changed, 32 insertions(+), 9 deletions(-)
> 
> diff --git a/include/linux/pagevec.h b/include/linux/pagevec.h
> index b45d391b4540..3f8a2a01131c 100644
> --- a/include/linux/pagevec.h
> +++ b/include/linux/pagevec.h
> @@ -21,7 +21,7 @@ struct pagevec {
>  };
>  
>  void __pagevec_release(struct pagevec *pvec);
> -void __pagevec_lru_add(struct pagevec *pvec);
> +void __pagevec_lru_add(struct pagevec *pvec, bool new);
>  unsigned pagevec_lookup_entries(struct pagevec *pvec,
>  				struct address_space *mapping,
>  				pgoff_t start, unsigned nr_entries,
> diff --git a/include/linux/swap.h b/include/linux/swap.h
> index 38fe1e91ba55..178f084365c2 100644
> --- a/include/linux/swap.h
> +++ b/include/linux/swap.h
> @@ -296,6 +296,7 @@ extern unsigned long nr_free_pagecache_pages(void);
>  
>  /* linux/mm/swap.c */
>  extern void lru_cache_add(struct page *);
> +extern void lru_cache_putback(struct page *page);
>  extern void lru_add_page_tail(struct page *page, struct page *page_tail,
>  			 struct lruvec *lruvec, struct list_head *head);
>  extern void activate_page(struct page *);
> diff --git a/mm/mlock.c b/mm/mlock.c
> index 96f001041928..449c291a286d 100644
> --- a/mm/mlock.c
> +++ b/mm/mlock.c
> @@ -264,7 +264,7 @@ static void __putback_lru_fast(struct pagevec *pvec, int pgrescued)
>  	 *__pagevec_lru_add() calls release_pages() so we don't call
>  	 * put_page() explicitly
>  	 */
> -	__pagevec_lru_add(pvec);
> +	__pagevec_lru_add(pvec, false);
>  	count_vm_events(UNEVICTABLE_PGRESCUED, pgrescued);
>  }
>  
> diff --git a/mm/swap.c b/mm/swap.c
> index c6936507abb5..576c721f210b 100644
> --- a/mm/swap.c
> +++ b/mm/swap.c
> @@ -44,6 +44,7 @@
>  int page_cluster;
>  
>  static DEFINE_PER_CPU(struct pagevec, lru_add_pvec);
> +static DEFINE_PER_CPU(struct pagevec, lru_putback_pvec);
>  static DEFINE_PER_CPU(struct pagevec, lru_rotate_pvecs);
>  static DEFINE_PER_CPU(struct pagevec, lru_deactivate_file_pvecs);
>  static DEFINE_PER_CPU(struct pagevec, lru_deactivate_pvecs);
> @@ -405,12 +406,23 @@ void lru_cache_add(struct page *page)
>  
>  	get_page(page);
>  	if (!pagevec_space(pvec))
> -		__pagevec_lru_add(pvec);
> +		__pagevec_lru_add(pvec, true);
>  	pagevec_add(pvec, page);
>  	put_cpu_var(lru_add_pvec);
>  }
>  EXPORT_SYMBOL(lru_cache_add);
>  
> +void lru_cache_putback(struct page *page)
> +{
> +	struct pagevec *pvec = &get_cpu_var(lru_putback_pvec);
> +
> +	get_page(page);
> +	if (!pagevec_space(pvec))
> +		__pagevec_lru_add(pvec, false);
> +	pagevec_add(pvec, page);
> +	put_cpu_var(lru_putback_pvec);
> +}
> +
>  /**
>   * add_page_to_unevictable_list - add a page to the unevictable list
>   * @page:  the page to be added to the unevictable list
> @@ -561,10 +573,15 @@ static void lru_deactivate_fn(struct page *page, struct lruvec *lruvec,
>   */
>  void lru_add_drain_cpu(int cpu)
>  {
> -	struct pagevec *pvec = &per_cpu(lru_add_pvec, cpu);
> +	struct pagevec *pvec;
> +
> +	pvec = &per_cpu(lru_add_pvec, cpu);
> +	if (pagevec_count(pvec))
> +		__pagevec_lru_add(pvec, true);
>  
> +	pvec = &per_cpu(lru_putback_pvec, cpu);
>  	if (pagevec_count(pvec))
> -		__pagevec_lru_add(pvec);
> +		__pagevec_lru_add(pvec, false);
>  
>  	pvec = &per_cpu(lru_rotate_pvecs, cpu);
>  	if (pagevec_count(pvec)) {
> @@ -819,12 +836,17 @@ static void __pagevec_lru_add_fn(struct page *page, struct lruvec *lruvec,
>  	int file = page_is_file_cache(page);
>  	int active = PageActive(page);
>  	enum lru_list lru = page_lru(page);
> +	bool new = (bool)arg;
>  
>  	VM_BUG_ON_PAGE(PageLRU(page), page);
>  
>  	SetPageLRU(page);
>  	add_page_to_lru_list(page, lruvec, lru);
> -	update_page_reclaim_stat(lruvec, file, active, hpage_nr_pages(page));
> +
> +	if (new)
> +		update_page_reclaim_stat(lruvec, file, active,
> +					 hpage_nr_pages(page));
> +
>  	trace_mm_lru_insertion(page, lru);
>  }
>  
> @@ -832,9 +854,9 @@ static void __pagevec_lru_add_fn(struct page *page, struct lruvec *lruvec,
>   * Add the passed pages to the LRU, then drop the caller's refcount
>   * on them.  Reinitialises the caller's pagevec.
>   */
> -void __pagevec_lru_add(struct pagevec *pvec)
> +void __pagevec_lru_add(struct pagevec *pvec, bool new)
>  {
> -	pagevec_lru_move_fn(pvec, __pagevec_lru_add_fn, NULL);
> +	pagevec_lru_move_fn(pvec, __pagevec_lru_add_fn, (void *)new);
>  }
>  
>  /**
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index f79010bbcdd4..8503713bb60e 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -737,7 +737,7 @@ redo:
>  		 * We know how to handle that.
>  		 */
>  		is_unevictable = false;
> -		lru_cache_add(page);
> +		lru_cache_putback(page);
>  	} else {
>  		/*
>  		 * Put unevictable pages directly on zone's unevictable
> -- 
> 2.8.3

-- 
Michal Hocko
SUSE Labs

[toc] | [prev] | [next] | [standalone]


#1416984 — Re: [PATCH 05/10] mm: remove LRU balancing effect of temporary page isolation

FromMinchan Kim <minchan@kernel.org>
Date2016-06-08 09:40 +0200
SubjectRe: [PATCH 05/10] mm: remove LRU balancing effect of temporary page isolation
Message-ID<rHGfL-5Ap-13@gated-at.bofh.it>
In reply to#1415405
On Mon, Jun 06, 2016 at 03:48:31PM -0400, Johannes Weiner wrote:
> Isolating an existing LRU page and subsequently putting it back on the
> list currently influences the balance between the anon and file LRUs.
> For example, heavy page migration or compaction could influence the
> balance between the LRUs and make one type more attractive when that
> type of page is affected more than the other. That doesn't make sense.
> 
> Add a dedicated LRU cache for putback, so that we can tell new LRU
> pages from existing ones at the time of linking them to the lists.
> 
> Signed-off-by: Johannes Weiner <hannes@cmpxchg.org>
> ---
>  include/linux/pagevec.h |  2 +-
>  include/linux/swap.h    |  1 +
>  mm/mlock.c              |  2 +-
>  mm/swap.c               | 34 ++++++++++++++++++++++++++++------
>  mm/vmscan.c             |  2 +-
>  5 files changed, 32 insertions(+), 9 deletions(-)
> 
> diff --git a/include/linux/pagevec.h b/include/linux/pagevec.h
> index b45d391b4540..3f8a2a01131c 100644
> --- a/include/linux/pagevec.h
> +++ b/include/linux/pagevec.h
> @@ -21,7 +21,7 @@ struct pagevec {
>  };
>  
>  void __pagevec_release(struct pagevec *pvec);
> -void __pagevec_lru_add(struct pagevec *pvec);
> +void __pagevec_lru_add(struct pagevec *pvec, bool new);
>  unsigned pagevec_lookup_entries(struct pagevec *pvec,
>  				struct address_space *mapping,
>  				pgoff_t start, unsigned nr_entries,
> diff --git a/include/linux/swap.h b/include/linux/swap.h
> index 38fe1e91ba55..178f084365c2 100644
> --- a/include/linux/swap.h
> +++ b/include/linux/swap.h
> @@ -296,6 +296,7 @@ extern unsigned long nr_free_pagecache_pages(void);
>  
>  /* linux/mm/swap.c */
>  extern void lru_cache_add(struct page *);
> +extern void lru_cache_putback(struct page *page);
>  extern void lru_add_page_tail(struct page *page, struct page *page_tail,
>  			 struct lruvec *lruvec, struct list_head *head);
>  extern void activate_page(struct page *);
> diff --git a/mm/mlock.c b/mm/mlock.c
> index 96f001041928..449c291a286d 100644
> --- a/mm/mlock.c
> +++ b/mm/mlock.c
> @@ -264,7 +264,7 @@ static void __putback_lru_fast(struct pagevec *pvec, int pgrescued)
>  	 *__pagevec_lru_add() calls release_pages() so we don't call
>  	 * put_page() explicitly
>  	 */
> -	__pagevec_lru_add(pvec);
> +	__pagevec_lru_add(pvec, false);
>  	count_vm_events(UNEVICTABLE_PGRESCUED, pgrescued);
>  }
>  
> diff --git a/mm/swap.c b/mm/swap.c
> index c6936507abb5..576c721f210b 100644
> --- a/mm/swap.c
> +++ b/mm/swap.c
> @@ -44,6 +44,7 @@
>  int page_cluster;
>  
>  static DEFINE_PER_CPU(struct pagevec, lru_add_pvec);
> +static DEFINE_PER_CPU(struct pagevec, lru_putback_pvec);
>  static DEFINE_PER_CPU(struct pagevec, lru_rotate_pvecs);
>  static DEFINE_PER_CPU(struct pagevec, lru_deactivate_file_pvecs);
>  static DEFINE_PER_CPU(struct pagevec, lru_deactivate_pvecs);
> @@ -405,12 +406,23 @@ void lru_cache_add(struct page *page)
>  
>  	get_page(page);
>  	if (!pagevec_space(pvec))
> -		__pagevec_lru_add(pvec);
> +		__pagevec_lru_add(pvec, true);
>  	pagevec_add(pvec, page);
>  	put_cpu_var(lru_add_pvec);
>  }
>  EXPORT_SYMBOL(lru_cache_add);
>  
> +void lru_cache_putback(struct page *page)
> +{
> +	struct pagevec *pvec = &get_cpu_var(lru_putback_pvec);
> +
> +	get_page(page);
> +	if (!pagevec_space(pvec))
> +		__pagevec_lru_add(pvec, false);
> +	pagevec_add(pvec, page);
> +	put_cpu_var(lru_putback_pvec);
> +}
> +
>  /**
>   * add_page_to_unevictable_list - add a page to the unevictable list
>   * @page:  the page to be added to the unevictable list
> @@ -561,10 +573,15 @@ static void lru_deactivate_fn(struct page *page, struct lruvec *lruvec,
>   */
>  void lru_add_drain_cpu(int cpu)
>  {
> -	struct pagevec *pvec = &per_cpu(lru_add_pvec, cpu);
> +	struct pagevec *pvec;
> +
> +	pvec = &per_cpu(lru_add_pvec, cpu);
> +	if (pagevec_count(pvec))
> +		__pagevec_lru_add(pvec, true);
>  
> +	pvec = &per_cpu(lru_putback_pvec, cpu);
>  	if (pagevec_count(pvec))
> -		__pagevec_lru_add(pvec);
> +		__pagevec_lru_add(pvec, false);
>  
>  	pvec = &per_cpu(lru_rotate_pvecs, cpu);
>  	if (pagevec_count(pvec)) {
> @@ -819,12 +836,17 @@ static void __pagevec_lru_add_fn(struct page *page, struct lruvec *lruvec,
>  	int file = page_is_file_cache(page);
>  	int active = PageActive(page);
>  	enum lru_list lru = page_lru(page);
> +	bool new = (bool)arg;
>  
>  	VM_BUG_ON_PAGE(PageLRU(page), page);
>  
>  	SetPageLRU(page);
>  	add_page_to_lru_list(page, lruvec, lru);
> -	update_page_reclaim_stat(lruvec, file, active, hpage_nr_pages(page));
> +
> +	if (new)
> +		update_page_reclaim_stat(lruvec, file, active,
> +					 hpage_nr_pages(page));
> +
>  	trace_mm_lru_insertion(page, lru);
>  }
>  
> @@ -832,9 +854,9 @@ static void __pagevec_lru_add_fn(struct page *page, struct lruvec *lruvec,
>   * Add the passed pages to the LRU, then drop the caller's refcount
>   * on them.  Reinitialises the caller's pagevec.
>   */
> -void __pagevec_lru_add(struct pagevec *pvec)
> +void __pagevec_lru_add(struct pagevec *pvec, bool new)
>  {
> -	pagevec_lru_move_fn(pvec, __pagevec_lru_add_fn, NULL);
> +	pagevec_lru_move_fn(pvec, __pagevec_lru_add_fn, (void *)new);
>  }

Just trivial:

'new' argument would be not clear in this context what does it mean
so worth to comment it, IMO but no strong opinion.

Other than that,

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

[toc] | [prev] | [next] | [standalone]


#1417624 — Re: [PATCH 05/10] mm: remove LRU balancing effect of temporary page isolation

FromJohannes Weiner <hannes@cmpxchg.org>
Date2016-06-08 18:10 +0200
SubjectRe: [PATCH 05/10] mm: remove LRU balancing effect of temporary page isolation
Message-ID<rHOdj-2nA-3@gated-at.bofh.it>
In reply to#1416984
On Wed, Jun 08, 2016 at 04:39:44PM +0900, Minchan Kim wrote:
> On Mon, Jun 06, 2016 at 03:48:31PM -0400, Johannes Weiner wrote:
> > @@ -832,9 +854,9 @@ static void __pagevec_lru_add_fn(struct page *page, struct lruvec *lruvec,
> >   * Add the passed pages to the LRU, then drop the caller's refcount
> >   * on them.  Reinitialises the caller's pagevec.
> >   */
> > -void __pagevec_lru_add(struct pagevec *pvec)
> > +void __pagevec_lru_add(struct pagevec *pvec, bool new)
> >  {
> > -	pagevec_lru_move_fn(pvec, __pagevec_lru_add_fn, NULL);
> > +	pagevec_lru_move_fn(pvec, __pagevec_lru_add_fn, (void *)new);
> >  }
> 
> Just trivial:
> 
> 'new' argument would be not clear in this context what does it mean
> so worth to comment it, IMO but no strong opinion.

True, it's a little mysterious. I'll document it.

> Other than that,
> 
> Acked-by: Minchan Kim <minchan@kernel.org>

Thanks!

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web