Re: mm: migrate_pages using

From: Michal Hocko
Date: Mon Mar 12 2007 - 14:42:41 EST


On Mon, Mar 12, 2007 at 04:32:55PM +0900, KAMEZAWA Hiroyuki wrote:
>
> > When I try to put_page(original) (because I don't want it for this
> > moment) I get to bad_page path and need to reboot...
> >
> > What am I missing? Am I using migrate_pages correctly?
> >
>
> ==
> migrate_pages() | refcnt = 2,
> -> unmap_and_move() | refcnt = 2,
> -> try_to_unmap() | refcnt = 1
> -> move_to_lru() | refcnt = 1
> -> lru_cache_add() | refcnt = 1
> -> page_cache_get() | refcnt = 2
> -> pagevec_add() | refcnt = 2
> -> page_cache_put() | refcnt 1
> ==

Thanks for clarification.

> see pagevec codes and release_pages() in swap.c
> per-cpu pagevec caches freed pages for a while.
> It will be pushed back to the free list when pagevec goes full.

I am not sure about that. When I looked inside lru_cache_add, it will
call __pagevec_lru_add if per-cpu pagevec goes empty and this function
will add all pages to the zone specific inactive list.

I want to prevent that because I need to use original page for other
purposes and so it can't be in inactive list. So I have created little
change to unmap_and_move function so that original page is added back tu
LRU only if required. migrate_pages uses variant with put_lru parameter
and I am using __migrate_pfn_page which uses put_lru=0. See the attached
patch.
What do you think about that. Is this way correct?

(Please add me to CC, because I am not list member)

Best regards
--
Michal Hocko
--- /usr/src/linux-vanilla/mm/migrate.c 2006-12-02 15:08:23.000000000 +0100
+++ /usr/src/linux/mm/migrate.c 2007-03-12 18:44:16.000000000 +0100

@@ -587,7 +595,7 @@ static int move_to_new_page(struct page
* to the newly allocated page in newpage.
*/
static int unmap_and_move(new_page_t get_new_page, unsigned long private,
- struct page *page, int force)
+ struct page *page, int force, int put_lru)
{
int rc = 0;
int *result = NULL;
@@ -631,10 +639,11 @@ unlock:
* A page that has been migrated has all references
* removed and will be freed. A page that has not been
* migrated will have kepts its references and be
- * restored.
+ * restored. Don't push page to LRU unless wanted.
*/
list_del(&page->lru);
- move_to_lru(page);
+ if(put_lru)
+ move_to_lru(page);
}

move_newpage:
@@ -687,7 +696,7 @@ int migrate_pages(struct list_head *from
cond_resched();

rc = unmap_and_move(get_new_page, private,
- page, pass > 2);
+ page, pass > 2, 1);

switch(rc) {
case -ENOMEM:
@@ -717,6 +726,111 @@ out:
return nr_failed + retry;
}

+/* target page is stored in private parameter when used from
+ * migrate_pfn_page
+ */
+static struct page * return_target(struct page * page, unsigned long private,
+ int ** result)
+{
+ return (struct page *)private;
+}
+
+int __migrate_pfn_page(struct page * page, new_page_t get_new_page, unsigned long private)
+{
+ int rc = 0;
+ int pass;
+ int retry = 1;
+
+ for(pass = 0; pass < 10 && retry; pass++)
+ {
+ retry = 0;
+ cond_resched();
+ rc = unmap_and_move(get_new_page, private, page, pass > 2, 0);
+ switch(rc)
+ {
+ case -ENOMEM:
+ goto out;
+ case -EAGAIN:
+ retry = 1;
+ break;
+ case 0:
+ break;
+ default:
+ return 1;
+ }
+ }
+ if(rc)
+ move_to_lru(page);
+out:
+ return rc;
+}
+
+int migrate_pfn_page(struct page * original, struct page * target)
+{
+ LIST_HEAD(pagelist);
+ int ret;
+ int swapwrite = current->flags & PF_SWAPWRITE;
+
+ if (!swapwrite)
+ current->flags |= PF_SWAPWRITE;
+
+ /* this will fail if migration is not supported */
+ if((ret = migrate_prep()))
+ goto done;
+ get_page(original);
+ ret = isolate_lru_page(original, &pagelist);
+ put_page(original);
+ if(ret)
+ goto done;
+ ret = __migrate_pfn_page(original, return_target, (unsigned long)target);
+ if(!ret)
+ page_cache_release(original);
+done:
+ if (!swapwrite)
+ current->flags &= ~PF_SWAPWRITE;
+ return ret;
+}