Anonymous page might have fs-private metadata, the page is truncated. As
the page hasn't mapping, page migration refuse to migrate the page. It
appears the page is only freed in page reclaim and if zone watermark is
low, the page is never freed, as a result migration always fail. I
thought we could free the metadata so such page can be freed in
migration and make migration more reliable?

Thanks,
Shaohua

diff --git a/mm/migrate.c b/mm/migrate.c
index 6a207e8..6bc38f7 100644
--- a/mm/migrate.c
+++ b/mm/migrate.c
@@ -633,6 +633,17 @@ static int unmap_and_move(new_page_t get_new_page, unsigned long private,
 			goto unlock;
 		wait_on_page_writeback(page);
 	}
+
+	/*
+	 * See truncate_complete_page(). Anonymous page might have
+	 * fs-private metadata, the page is truncated. Such page can't be
+	 * migrated. Try to free metadata, so the page can be freed.
+	 */
+	if (!page->mapping && !PageAnon(page) && PagePrivate(page)) {
+		try_to_release_page(page, GFP_KERNEL);
+		goto unlock;
+	}
+
 	/*
 	 * By try_to_unmap(), page->mapcount goes down to 0 here. In this case,
 	 * we cannot notice that anon_vma is freed while we migrates a page.


--

[ message continues ]

Anonymous pages should not have fs-private metadata.

Orphaned pages I guess you mean? They should not be accessable via
the pagecache or the page tables, so how do they keep tangling up
--

[ message continues ]

yes, maybe, but the comments in truncate_complete_page called the page
the page is still in lru list. Memory hot remove will try to migrate the
page.

Thanks,

--

[ message continues ]

So this is an abandoned page with a refcount that only exists because of 
fs private data? Truncate race?

--

[ message continues ]

truncate_complete_page could fail to remove fs private data, this is the
comments say.

Thanks,
Shaohua

--

[ message continues ]

Ah, I see. I think we should use orphaned (or anything except
anonymous) to describe these pages.
--

[ message continues ]

Is this maybe related to memory unplug or some such project?

--

[ message continues ]

Well maybe you should change the comment to refer to an orphaned page. 
That is what Nick used. Also change the comment in truncate_complete_page. 
Anonymous page is confusing here because you check that it is *not* an 


Could you move that into the corner case handling that follows?

So it would be something like

if (!page->mapping) {
	if (!PageAnon(page) && PagePrivate(page))
		try_to_relase_page(page, GFP_KERNEL);
	goto rcu_unlock;
}

?

--

[ message continues ]

Ok, changed.

Orphaned page might have fs-private metadata, the page is truncated. As
the page hasn't mapping, page migration refuse to migrate the page. It
appears the page is only freed in page reclaim and if zone watermark is
low, the page is never freed, as a result migration always fail. I
thought we could free the metadata so such page can be freed in
migration and make migration more reliable.

Signed-off-by: Shaohua Li <shaohua.li@intel.com>

diff --git a/mm/migrate.c b/mm/migrate.c
index 6a207e8..e82acc9 100644
--- a/mm/migrate.c
+++ b/mm/migrate.c
@@ -652,8 +652,16 @@ static int unmap_and_move(new_page_t get_new_page, unsigned long private,
 	 * Calling try_to_unmap() against a page->mapping==NULL page is
 	 * BUG. So handle it here.
 	 */
-	if (!page->mapping)
+	if (!page->mapping) {
+		/*
+		 * See truncate_complete_page(). Orphaned page might have
+		 * fs-private metadata, the page is truncated. Such page can't
+		 * be migrated. Try to free metadata, so the page can be freed.
+		 */
+		if (!PageAnon(page) && PagePrivate(page))
+			try_to_release_page(page, GFP_KERNEL);
 		goto rcu_unlock;
+	}
 	/* Establish migration ptes or remove ptes */
 	try_to_unmap(page, 1);
 
diff --git a/mm/truncate.c b/mm/truncate.c
index cadc156..62fd8cd 100644
--- a/mm/truncate.c
+++ b/mm/truncate.c
@@ -84,7 +84,7 @@ EXPORT_SYMBOL(cancel_dirty_page);
 
 /*
  * If truncate cannot remove the fs-private metadata from the page, the page
- * becomes anonymous.  It will be left on the LRU and may even be mapped into
+ * becomes orphaned.  It will be left on the LRU and may even be mapped into
  * user pagetables if we're racing with filemap_fault().
  *
  * We need to bale out if page->mapping is no longer equal to the original


--

[ message continues ]

Acked-by: Christoph Lameter <clameter@sgi.com>

Nick? Ok with you too?

--

[ message continues ]

Yeah, for memory hot remove that makes sense. A comment
might be in order, at least a reference to the orphaned
page code in vmscan.c.

Otherwise, it is OK by me.

Acked-by: Nick Piggin <npiggin@suse.de>
--

[ message continues ]

Right. The surrounding comments in mm/migrate.c also need to be made 
consistent. The comment before is now slightly off. Could you do another 
patch Shaohua with better comments?
--

[ message continues ]

Orphaned page might have fs-private metadata, the page is truncated. As
the page hasn't mapping, page migration refuse to migrate the page. It
appears the page is only freed in page reclaim and if zone watermark is
low, the page is never freed, as a result migration always fail. I
thought we could free the metadata so such page can be freed in
migration and make migration more reliable.

Signed-off-by: Shaohua Li <shaohua.li@intel.com>
Acked-by: Nick Piggin <npiggin@suse.de>
Acked-by: Christoph Lameter <clameter@sgi.com>

diff --git a/mm/migrate.c b/mm/migrate.c
index 6a207e8..2c1a48a 100644
--- a/mm/migrate.c
+++ b/mm/migrate.c
@@ -646,14 +646,22 @@ static int unmap_and_move(new_page_t get_new_page, unsigned long private,
 		rcu_locked = 1;
 	}
 	/*
-	 * This is a corner case handling.
-	 * When a new swap-cache is read into, it is linked to LRU
+	 * There are corner cases handling.
+	 * 1. When a new swap-cache is read into, it is linked to LRU
 	 * and treated as swapcache but has no rmap yet.
 	 * Calling try_to_unmap() against a page->mapping==NULL page is
 	 * BUG. So handle it here.
+	 * 2. Orphaned page (see truncate_complete_page) might have
+	 * fs-private metadata, the page is truncated. The page can be picked
+	 * up due to memory offlineing. Everywhere else except page reclaim,
+	 * the page is invisible to the vm, so the page can't be migrated. Try
+	 * to free metadata, so the page can be freed.
 	 */
-	if (!page->mapping)
+	if (!page->mapping) {
+		if (!PageAnon(page) && PagePrivate(page))
+			try_to_release_page(page, GFP_KERNEL);
 		goto rcu_unlock;
+	}
 	/* Establish migration ptes or remove ptes */
 	try_to_unmap(page, 1);
 
diff --git a/mm/truncate.c b/mm/truncate.c
index cadc156..62fd8cd 100644
--- a/mm/truncate.c
+++ b/mm/truncate.c
@@ -84,7 +84,7 @@ EXPORT_SYMBOL(cancel_dirty_page);
 
 /*
  * If truncate cannot remove the fs-private metadata from the page, the page
- * becomes anonymous.  It will be left on the LRU and may even be ...
[ message continues ]

We call something(GFP_KERNEL) under rcu_read_lock()?  I've lost track of
the myriad flavours of rcu which we purport to support, but I don't think
they'll all like us blocking under rcu_read_lock().

We _won't_ block, because try_to_release_page() will see the NULL ->mapping
and will call the non-blocking try_to_free_buffers().  But still, it looks
bad, and will cause problems if someone decides to add a might_sleep_if()
to try_to_release_page().

So...  I'd suggest that it would be better to add an apologetic comment and


hm.



Please review and test....

--- a/mm/migrate.c~page-migraton-handle-orphaned-pages-fix
+++ a/mm/migrate.c
@@ -650,20 +650,28 @@ static int unmap_and_move(new_page_t get
 	}
 
 	/*
-	 * There are corner cases handling.
-	 * 1. When a new swap-cache is read into, it is linked to LRU
-	 * and treated as swapcache but has no rmap yet.
-	 * Calling try_to_unmap() against a page->mapping==NULL page is
-	 * BUG. So handle it here.
-	 * 2. Orphaned page (see truncate_complete_page) might have
-	 * fs-private metadata, the page is truncated. The page can be picked
-	 * up due to memory offlineing. Everywhere else except page reclaim,
-	 * the page is invisible to the vm, so the page can't be migrated. Try
-	 * to free metadata, so the page can be freed.
+	 * 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.
+	 * 2. An orphaned page (see truncate_complete_page) might have
+	 * fs-private metadata. The page can be picked up due to memory
+	 * offlining.  Everywhere else except page reclaim, the page is
+	 * invisible to the vm, so the page can not be migrated.  So try to
+	 * free the metadata, so the page can be freed.
 	 */
 	if (!page->mapping) {
-		if (!PageAnon(page) && PagePrivate(page))
-			try_to_release_page(page, GFP_KERNEL);
+		if ...
[ message continues ]

You're right, but can't we just rcu_read_unlock() before try_to_release_page?
--

[ message continues ]

or we could move the code above before doing rcu_read_lock()?

--

[ message continues ]

Right. Looks good. PageWriteback cannot be set if we do not have a 
mappig...

--

[ message continues ]

I still think your solution is good but even with the original patch we do 
not call try_to_release_page() with GFP_KERNEL under rcu.

   if (PageAnon(page)) {
                rcu_read_lock();
                rcu_locked = 1;
        }


rcu is only active if we have an anonymous page and in that case we do not
call try_to_release_page(). Just to make sure that Nick and I do not get 
dinged needlessly....

--

[ message continues ]