From: Mel Gorman <mel@csn.ul.ie>
To: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
Andrea Arcangeli <aarcange@redhat.com>,
Christoph Lameter <cl@linux-foundation.org>,
Adam Litke <agl@us.ibm.com>, Avi Kivity <avi@redhat.com>,
David Rientjes <rientjes@google.com>,
Minchan Kim <minchan.kim@gmail.com>,
KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>,
Rik van Riel <riel@redhat.com>,
linux-kernel@vger.kernel.org, linux-mm@kvack.org
Subject: Re: [PATCH 14/14] mm,migration: Allow the migration of PageSwapCache pages
Date: Wed, 31 Mar 2010 12:27:30 +0100 [thread overview]
Message-ID: <20100331112730.GB27389@csn.ul.ie> (raw)
In-Reply-To: <20100331142623.62ac9175.kamezawa.hiroyu@jp.fujitsu.com>
On Wed, Mar 31, 2010 at 02:26:23PM +0900, KAMEZAWA Hiroyuki wrote:
> On Tue, 30 Mar 2010 10:14:49 +0100
> Mel Gorman <mel@csn.ul.ie> wrote:
>
> > PageAnon pages that are unmapped may or may not have an anon_vma so
> > are not currently migrated. However, a swap cache page can be migrated
> > and fits this description. This patch identifies page swap caches and
> > allows them to be migrated.
> >
>
> Some comments.
>
> > Signed-off-by: Mel Gorman <mel@csn.ul.ie>
> > ---
> > mm/migrate.c | 15 ++++++++++-----
> > mm/rmap.c | 6 ++++--
> > 2 files changed, 14 insertions(+), 7 deletions(-)
> >
> > diff --git a/mm/migrate.c b/mm/migrate.c
> > index 35aad2a..f9bf37e 100644
> > --- a/mm/migrate.c
> > +++ b/mm/migrate.c
> > @@ -203,6 +203,9 @@ static int migrate_page_move_mapping(struct address_space *mapping,
> > void **pslot;
> >
> > if (!mapping) {
> > + if (PageSwapCache(page))
> > + SetPageSwapCache(newpage);
> > +
>
> Migration of SwapCache requires radix-tree replacement, IOW,
> mapping == NULL && PageSwapCache is BUG.
>
> So, this never happens.
>
Correct. In the initial attempt to allow PageSwapCache pages to move, I
encountered a number of bugs. This was a "it can't be happening but something
is odd" change that I meant to drop in the final version but forgot. Thanks
>
> > /* Anonymous page without mapping */
> > if (page_count(page) != 1)
> > return -EAGAIN;
> > @@ -607,11 +610,13 @@ static int unmap_and_move(new_page_t get_new_page, unsigned long private,
> > * the page was isolated and when we reached here while
> > * the RCU lock was not held
> > */
> > - if (!page_mapped(page))
> > - goto rcu_unlock;
> > -
> > - anon_vma = page_anon_vma(page);
> > - atomic_inc(&anon_vma->external_refcount);
> > + if (!page_mapped(page)) {
> > + if (!PageSwapCache(page))
> > + goto rcu_unlock;
> > + } else {
> > + anon_vma = page_anon_vma(page);
> > + atomic_inc(&anon_vma->external_refcount);
> > + }
> > }
> >
> > /*
> > diff --git a/mm/rmap.c b/mm/rmap.c
> > index af35b75..d5ea1f2 100644
> > --- a/mm/rmap.c
> > +++ b/mm/rmap.c
> > @@ -1394,9 +1394,11 @@ int rmap_walk(struct page *page, int (*rmap_one)(struct page *,
> >
> > if (unlikely(PageKsm(page)))
> > return rmap_walk_ksm(page, rmap_one, arg);
> > - else if (PageAnon(page))
> > + else if (PageAnon(page)) {
> > + if (PageSwapCache(page))
> > + return SWAP_AGAIN;
> > return rmap_walk_anon(page, rmap_one, arg);
>
> SwapCache has a condition as (PageSwapCache(page) && page_mapped(page) == true.
>
> Please see do_swap_page(), PageSwapCache bit is cleared only when
>
> do_swap_page()...
> swap_free(entry);
> if (vm_swap_full() || (vma->vm_flags & VM_LOCKED) || PageMlocked(page))
> try_to_free_swap(page);
>
> Then, PageSwapCache is cleared only when swap is freeable even if mapped.
>
> rmap_walk_anon() should be called and the check is not necessary.
>
I follow your reasoning. What caught me is the following comment;
* Corner case handling:
* 1. When a new swap-cache page is read into, it is added to the LRU
* and treated as swapcache but it has no rmap yet.
* Calling try_to_unmap() against a page->mapping==NULL page will
* trigger a BUG. So handle it here.
and the fact that without the check the following bug is triggered;
[ 476.541321] BUG: unable to handle kernel NULL pointer dereference at (null)
[ 476.590328] IP: [<ffffffff81072162>] __bfs+0x1a1/0x1d7
[ 476.590328] PGD 3781c067 PUD 371b2067 PMD 0
[ 476.590328] Oops: 0000 [#1] PREEMPT SMP
[ 476.590328] last sysfs file: /sys/block/sr0/capability
[ 476.590328] CPU 0
[ 476.590328] Modules linked in: highalloc trace_allocmap buddyinfo vmregress_core oprofile dm_crypt loop i2c_piix4 evdev i2c_core processor serio_raw tpm_tis tpm tpm_bios shpchp pci_hotplug button ext3 jbd mbcache dm_mirror dm_region_hash dm_log dm_snapshot dm_mod sg sr_mod cdrom sd_mod ata_generic ahci libahci ide_pci_generic libata ide_core scsi_mod ohci_hcd r8169 mii ehci_hcd floppy thermal fan thermal_sys
[ 477.296405]
[ 477.296405] Pid: 4343, comm: bench-stresshig Not tainted 2.6.34-rc2-mm1-compaction-v7r3 #1 GA-MA790GP-UD4H/GA-MA790GP-UD4H
[ 477.296405] RIP: 0010:[<ffffffff81072162>] [<ffffffff81072162>] __bfs+0x1a1/0x1d7
[ 477.296405] RSP: 0000:ffff88007817d4c8 EFLAGS: 00010046
[ 477.296405] RAX: ffffffff81c80170 RBX: ffff88007817d528 RCX: 0000000000000000
[ 477.296405] RDX: ffff88007817d528 RSI: 0000000000000000 RDI: ffff88007817d528
[ 477.296405] RBP: ffff88007817d518 R08: 0000000000000000 R09: 0000000000000000
[ 477.296405] R10: ffffffff816a1d08 R11: 0000000000000046 R12: 0000000000000000
[ 477.922839] R13: ffffffff81513887 R14: ffffffff81c80170 R15: 0000000000000000
[ 477.922839] FS: 00007f8d853d26e0(0000) GS:ffff880002200000(0000) knlGS:0000000000000000
[ 478.123091] CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b
[ 478.123091] CR2: 0000000000000000 CR3: 0000000037a0e000 CR4: 00000000000006f0
[ 478.123091] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[ 478.123091] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
[ 478.123091] Process bench-stresshig (pid: 4343, threadinfo ffff88007817c000, task ffff88007ebea000)
[ 478.566030] Stack:
[ 478.566030] ffff88007817d570 ffffffff81070f92 0000000000000000 0000000000000002
[ 478.566030] <0> ffff88007817d528 ffff88007817d528 ffff88007ebea668 ffffffff81513887
[ 478.566030] <0> ffff88007ebea000 ffff88007ebea000 ffff88007817d598 ffffffff81074201
[ 478.566030] Call Trace:
[ 478.566030] [<ffffffff81070f92>] ? usage_match+0x0/0x17
[ 478.566030] [<ffffffff81074201>] check_usage_backwards+0x93/0xca
[ 478.566030] [<ffffffff8107416e>] ? check_usage_backwards+0x0/0xca
[ 478.566030] [<ffffffff81073544>] mark_lock+0x31d/0x52f
[ 478.566030] [<ffffffff8107515a>] __lock_acquire+0x801/0x1776
[ 478.566030] [<ffffffff810761c5>] lock_acquire+0xf6/0x122
[ 478.566030] [<ffffffff810ef121>] ? rmap_walk+0x5c/0x16d
[ 478.566030] [<ffffffff812fcfeb>] _raw_spin_lock+0x3b/0x47
[ 478.566030] [<ffffffff810ef121>] ? rmap_walk+0x5c/0x16d
[ 478.566030] [<ffffffff810ef121>] rmap_walk+0x5c/0x16d
[ 478.566030] [<ffffffff81106396>] ? remove_migration_pte+0x0/0x234
[ 478.566030] [<ffffffff81300dc1>] ? sub_preempt_count+0x9/0x83
[ 478.566030] [<ffffffff81106914>] ? migrate_page_copy+0xa0/0x1ed
[ 478.566030] [<ffffffff81106ea4>] migrate_pages+0x3fc/0x5d3
[ 478.566030] [<ffffffff81106c56>] ? migrate_pages+0x1ae/0x5d3
[ 478.566030] [<ffffffff81073a24>] ? trace_hardirqs_on_caller+0x110/0x134
[ 478.566030] [<ffffffff81107e11>] ? compaction_alloc+0x0/0x283
[ 478.566030] [<ffffffff811079b0>] ? compact_zone+0x14e/0x4bd
Granted, what I should have spotted was that a more relevant check was for
the specific corner case like in the revised patch below. Please note that
if I put a WARN_ON in the check in rmap.c, it can and does trigger. If this
situation really is never meant to occur, there is a race that needs to be
closed before PageSwapCache can be migrated.
==== CUT HERE ====
mm,migration: Allow the migration of PageSwapCache pages
PageAnon pages that are unmapped may or may not have an anon_vma so
are not currently migrated. However, a swap cache page can be migrated
and fits this description. This patch identifies page swap caches and
allows them to be migrated.
Signed-off-by: Mel Gorman <mel@csn.ul.ie>
---
mm/migrate.c | 12 +++++++-----
mm/rmap.c | 7 +++++--
2 files changed, 12 insertions(+), 7 deletions(-)
diff --git a/mm/migrate.c b/mm/migrate.c
index 35aad2a..2284d79 100644
--- a/mm/migrate.c
+++ b/mm/migrate.c
@@ -607,11 +607,13 @@ static int unmap_and_move(new_page_t get_new_page, unsigned long private,
* the page was isolated and when we reached here while
* the RCU lock was not held
*/
- if (!page_mapped(page))
- goto rcu_unlock;
-
- anon_vma = page_anon_vma(page);
- atomic_inc(&anon_vma->external_refcount);
+ if (!page_mapped(page)) {
+ if (!PageSwapCache(page))
+ goto rcu_unlock;
+ } else {
+ anon_vma = page_anon_vma(page);
+ atomic_inc(&anon_vma->external_refcount);
+ }
}
/*
diff --git a/mm/rmap.c b/mm/rmap.c
index af35b75..7d63c68 100644
--- a/mm/rmap.c
+++ b/mm/rmap.c
@@ -1394,9 +1394,12 @@ int rmap_walk(struct page *page, int (*rmap_one)(struct page *,
if (unlikely(PageKsm(page)))
return rmap_walk_ksm(page, rmap_one, arg);
- else if (PageAnon(page))
+ else if (PageAnon(page)) {
+ /* See comment on corner case handling in mm/migrate.c */
+ if (PageSwapCache(page) && !page_mapped(page))
+ return SWAP_AGAIN;
return rmap_walk_anon(page, rmap_one, arg);
- else
+ } else
return rmap_walk_file(page, rmap_one, arg);
}
#endif /* CONFIG_MIGRATION */
--
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>
next prev parent reply other threads:[~2010-03-31 11:27 UTC|newest]
Thread overview: 36+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-03-30 9:14 [PATCH 0/14] Memory Compaction v6 Mel Gorman
2010-03-30 9:14 ` [PATCH 01/14] mm,migration: Take a reference to the anon_vma before migrating Mel Gorman
2010-03-30 9:14 ` [PATCH 02/14] mm,migration: Do not try to migrate unmapped anonymous pages Mel Gorman
2010-03-30 9:14 ` [PATCH 03/14] mm: Share the anon_vma ref counts between KSM and page migration Mel Gorman
2010-03-30 9:14 ` [PATCH 04/14] Allow CONFIG_MIGRATION to be set without CONFIG_NUMA or memory hot-remove Mel Gorman
2010-03-30 9:14 ` [PATCH 05/14] Export unusable free space index via /proc/unusable_index Mel Gorman
2010-03-30 9:14 ` [PATCH 06/14] Export fragmentation index via /proc/extfrag_index Mel Gorman
2010-03-30 9:14 ` [PATCH 07/14] Move definition for LRU isolation modes to a header Mel Gorman
2010-03-30 9:14 ` [PATCH 08/14] Memory compaction core Mel Gorman
2010-03-30 9:14 ` [PATCH 09/14] Add /proc trigger for memory compaction Mel Gorman
2010-03-30 9:14 ` [PATCH 10/14] Add /sys trigger for per-node " Mel Gorman
2010-03-30 9:14 ` [PATCH 11/14] Direct compact when a high-order allocation fails Mel Gorman
2010-03-30 9:14 ` [PATCH 12/14] Add a tunable that decides when memory should be compacted and when it should be reclaimed Mel Gorman
2010-03-30 9:14 ` [PATCH 13/14] Do not compact within a preferred zone after a compaction failure Mel Gorman
2010-03-30 9:14 ` [PATCH 14/14] mm,migration: Allow the migration of PageSwapCache pages Mel Gorman
2010-03-31 5:26 ` KAMEZAWA Hiroyuki
2010-03-31 11:27 ` Mel Gorman [this message]
2010-03-31 23:57 ` KAMEZAWA Hiroyuki
2010-04-01 2:39 ` Minchan Kim
2010-04-01 2:43 ` Minchan Kim
2010-04-01 3:01 ` KAMEZAWA Hiroyuki
2010-04-01 4:44 ` Minchan Kim
2010-04-01 5:42 ` KAMEZAWA Hiroyuki
2010-04-01 10:51 ` Minchan Kim
2010-04-01 17:36 ` Mel Gorman
2010-04-02 0:20 ` Minchan Kim
2010-04-02 8:51 ` Mel Gorman
2010-04-02 0:21 ` KAMEZAWA Hiroyuki
2010-04-02 8:52 ` Mel Gorman
2010-04-01 9:30 ` Mel Gorman
2010-04-01 10:42 ` Minchan Kim
2010-04-02 16:02 [PATCH 0/14] Memory Compaction v7 Mel Gorman
2010-04-02 16:02 ` [PATCH 14/14] mm,migration: Allow the migration of PageSwapCache pages Mel Gorman
2010-04-06 6:54 ` KAMEZAWA Hiroyuki
2010-04-06 15:37 ` Minchan Kim
2010-04-07 0:06 ` Andrew Morton
2010-04-07 16:49 ` Mel Gorman
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20100331112730.GB27389@csn.ul.ie \
--to=mel@csn.ul.ie \
--cc=aarcange@redhat.com \
--cc=agl@us.ibm.com \
--cc=akpm@linux-foundation.org \
--cc=avi@redhat.com \
--cc=cl@linux-foundation.org \
--cc=kamezawa.hiroyu@jp.fujitsu.com \
--cc=kosaki.motohiro@jp.fujitsu.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=minchan.kim@gmail.com \
--cc=riel@redhat.com \
--cc=rientjes@google.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox