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


Groups > linux.kernel > #1452235 > unrolled thread

[PATCH] mm: move swap-in anonymous page into active list

Started byMinchan Kim <minchan@kernel.org>
First post2016-07-29 05:30 +0200
Last post2016-07-29 20:10 +0200
Articles 4 — 4 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH] mm: move swap-in anonymous page into active list Minchan Kim <minchan@kernel.org> - 2016-07-29 05:30 +0200
    Re: [PATCH] mm: move swap-in anonymous page into active list Johannes Weiner <hannes@cmpxchg.org> - 2016-07-29 15:40 +0200
    Re: [PATCH] mm: move swap-in anonymous page into active list Rik van Riel <riel@redhat.com> - 2016-07-29 19:00 +0200
      Re: [PATCH] mm: move swap-in anonymous page into active list Nadav Amit <nadav.amit@gmail.com> - 2016-07-29 20:10 +0200

#1452235 — [PATCH] mm: move swap-in anonymous page into active list

FromMinchan Kim <minchan@kernel.org>
Date2016-07-29 05:30 +0200
Subject[PATCH] mm: move swap-in anonymous page into active list
Message-ID<s06EO-7sn-3@gated-at.bofh.it>
Every swap-in anonymous page starts from inactive lru list's head.
It should be activated unconditionally when VM decide to reclaim
because page table entry for the page always usually has marked
accessed bit. Thus, their window size for getting a new referece
is 2 * NR_inactive + NR_active while others is NR_active + NR_active.

It's not fair that it has more chance to be referenced compared
to other newly allocated page which starts from active lru list's
head.

Signed-off-by: Minchan Kim <minchan@kernel.org>
---
 mm/memory.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/mm/memory.c b/mm/memory.c
index 4425b6059339..3a730b920242 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -2642,6 +2642,7 @@ int do_swap_page(struct fault_env *fe, pte_t orig_pte)
 	if (page == swapcache) {
 		do_page_add_anon_rmap(page, vma, fe->address, exclusive);
 		mem_cgroup_commit_charge(page, memcg, true, false);
+		activate_page(page);
 	} else { /* ksm created a completely new copy */
 		page_add_new_anon_rmap(page, vma, fe->address, false);
 		mem_cgroup_commit_charge(page, memcg, false, false);
-- 
1.9.1

[toc] | [next] | [standalone]


#1452409

FromJohannes Weiner <hannes@cmpxchg.org>
Date2016-07-29 15:40 +0200
Message-ID<s0gb7-5fG-5@gated-at.bofh.it>
In reply to#1452235
On Fri, Jul 29, 2016 at 12:25:40PM +0900, Minchan Kim wrote:
> Every swap-in anonymous page starts from inactive lru list's head.
> It should be activated unconditionally when VM decide to reclaim
> because page table entry for the page always usually has marked
> accessed bit. Thus, their window size for getting a new referece
> is 2 * NR_inactive + NR_active while others is NR_active + NR_active.
> 
> It's not fair that it has more chance to be referenced compared
> to other newly allocated page which starts from active lru list's
> head.
> 
> Signed-off-by: Minchan Kim <minchan@kernel.org>

That behavior stood out to me as well recently, but I couldn't
convince myself that activation is the right thing.

The page can still have a valid copy on the swap device, so prefering
to reclaim that page over a fresh one could make sense. But as you
point out, having it start inactive instead of active actually ends up
giving it *more* LRU time, and that seems to be without justification.

So this change makes sense to me. Maybe somebody else remembers a good
reason for why the behavior is the way it is, but likely it has always
been an oversight.

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

> ---
>  mm/memory.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/mm/memory.c b/mm/memory.c
> index 4425b6059339..3a730b920242 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -2642,6 +2642,7 @@ int do_swap_page(struct fault_env *fe, pte_t orig_pte)
>  	if (page == swapcache) {
>  		do_page_add_anon_rmap(page, vma, fe->address, exclusive);
>  		mem_cgroup_commit_charge(page, memcg, true, false);
> +		activate_page(page);
>  	} else { /* ksm created a completely new copy */
>  		page_add_new_anon_rmap(page, vma, fe->address, false);
>  		mem_cgroup_commit_charge(page, memcg, false, false);
> -- 
> 1.9.1

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


#1452480

FromRik van Riel <riel@redhat.com>
Date2016-07-29 19:00 +0200
Message-ID<s0jiF-7fv-1@gated-at.bofh.it>
In reply to#1452235

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

On Fri, 2016-07-29 at 12:25 +0900, Minchan Kim wrote:
> Every swap-in anonymous page starts from inactive lru list's head.
> It should be activated unconditionally when VM decide to reclaim
> because page table entry for the page always usually has marked
> accessed bit. Thus, their window size for getting a new referece
> is 2 * NR_inactive + NR_active while others is NR_active + NR_active.
> 
> It's not fair that it has more chance to be referenced compared
> to other newly allocated page which starts from active lru list's
> head.
> 
> Signed-off-by: Minchan Kim <minchan@kernel.org>

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

The reason newly read in swap cache pages start on the
inactive list is that we do some amount of read-around,
and do not know which pages will get used.

However, immediately activating the ones that DO get
used, like your patch does, is the right thing to do.

-- 
All Rights Reversed.

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


#1452502

FromNadav Amit <nadav.amit@gmail.com>
Date2016-07-29 20:10 +0200
Message-ID<s0kop-8aN-3@gated-at.bofh.it>
In reply to#1452480
Rik van Riel <riel@redhat.com> wrote:

> On Fri, 2016-07-29 at 12:25 +0900, Minchan Kim wrote:
>> Every swap-in anonymous page starts from inactive lru list's head.
>> It should be activated unconditionally when VM decide to reclaim
>> because page table entry for the page always usually has marked
>> accessed bit. Thus, their window size for getting a new referece
>> is 2 * NR_inactive + NR_active while others is NR_active + NR_active.
>> 
>> It's not fair that it has more chance to be referenced compared
>> to other newly allocated page which starts from active lru list's
>> head.
>> 
>> Signed-off-by: Minchan Kim <minchan@kernel.org>
> 
> Acked-by: Rik van Riel <riel@redhat.com>
> 
> The reason newly read in swap cache pages start on the
> inactive list is that we do some amount of read-around,
> and do not know which pages will get used.
> 
> However, immediately activating the ones that DO get
> used, like your patch does, is the right thing to do.

Can it cause the swap clusters to lose spatial locality?

For instance, if a process writes sequentially to memory multiple times,
and if pages are swapped out, in and back out. In such case, doesn’t it
increase the probability that the swap cluster will hold irrelevant data and
make swap prefetch less efficient?

Regards,
Nadav

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web