[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <20080126220351.a04819f9.akpm@linux-foundation.org>
Date: Sat, 26 Jan 2008 22:03:51 -0800
From: Andrew Morton <akpm@...ux-foundation.org>
To: Shaohua Li <shaohua.li@...el.com>
Cc: nickpiggin@...oo.com.au, clameter@....com,
linux-kernel@...r.kernel.org
Subject: Re: [RFC] some page can't be migrated
> On Fri, 25 Jan 2008 14:03:25 +0800 Shaohua Li <shaohua.li@...el.com> wrote:
>
> - if (!page->mapping)
> + if (!page->mapping) {
> + if (!PageAnon(page) && PagePrivate(page))
> + try_to_release_page(page, GFP_KERNEL);
> goto rcu_unlock;
> + }
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
call direct into try_to_free_buffers().
> - * 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
hm, that didn't improve the rather flakey grammar in there. Oh well.
> Acked-by: Nick Piggin <npiggin@...e.de>
> Acked-by: Christoph Lameter <clameter@....com>
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 (!PageAnon(page) && PagePrivate(page)) {
+ /*
+ * Go direct to try_to_free_buffers() here because
+ * a) that's what try_to_release_page() would do anyway
+ * b) we may be under rcu_read_lock() here, so we can't
+ * use GFP_KERNEL which is what try_to_release_page()
+ * needs to be effective.
+ */
+ try_to_release_buffers(page);
+ }
goto rcu_unlock;
}
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists