lkml.org 
[lkml]   [2010]   [Apr]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 14/14] mm,migration: Allow the migration of PageSwapCache pages
    On Thu, 1 Apr 2010 18:36:41 +0100
    Mel Gorman <mel@csn.ul.ie> wrote:

    > > > ==
    > > >        skip_remap = 0;
    > > >        if (PageAnon(page)) {
    > > >                rcu_read_lock();
    > > >                if (!page_mapped(page)) {
    > > >                        if (!PageSwapCache(page))
    > > >                                goto rcu_unlock;
    > > >                        /*
    > > >                         * We can't convice this anon_vma is valid or not because
    > > >                         * !page_mapped(page). Then, we do migration(radix-tree replacement)
    > > >                         * but don't remap it which touches anon_vma in page->mapping.
    > > >                         */
    > > >                        skip_remap = 1;
    > > >                        goto skip_unmap;
    > > >                } else {
    > > >                        anon_vma = page_anon_vma(page);
    > > >                        atomic_inc(&anon_vma->external_refcount);
    > > >                }
    > > >        }
    > > >        .....copy page, radix-tree replacement,....
    > > >
    > >
    > > It's not enough.
    > > we uses remove_migration_ptes in move_to_new_page, too.
    > > We have to prevent it.
    > > We can check PageSwapCache(page) in move_to_new_page and then
    > > skip remove_migration_ptes.
    > >
    > > ex)
    > > static int move_to_new_page(....)
    > > {
    > > int swapcache = PageSwapCache(page);
    > > ...
    > > if (!swapcache)
    > > if(!rc)
    > > remove_migration_ptes
    > > else
    > > newpage->mapping = NULL;
    > > }
    > >
    >
    > This I agree with.
    >
    me, too.


    > I am not sure this race exists because the page is locked but a key
    > observation has been made - A page that is unmapped can be migrated if
    > it's PageSwapCache but it may not have a valid anon_vma. Hence, in the
    > !page_mapped case, the key is to not use anon_vma. How about the
    > following patch?
    >

    Seems good to me. But (see below)


    > ==== 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 but ensures that no attempt to made to remap the pages
    > would would potentially try to access an already freed anon_vma.
    >
    > Signed-off-by: Mel Gorman <mel@csn.ul.ie>
    >
    > diff --git a/mm/migrate.c b/mm/migrate.c
    > index 35aad2a..5d0218b 100644
    > --- a/mm/migrate.c
    > +++ b/mm/migrate.c
    > @@ -484,7 +484,8 @@ static int fallback_migrate_page(struct address_space *mapping,
    > * < 0 - error code
    > * == 0 - success
    > */
    > -static int move_to_new_page(struct page *newpage, struct page *page)
    > +static int move_to_new_page(struct page *newpage, struct page *page,
    > + int safe_to_remap)
    > {
    > struct address_space *mapping;
    > int rc;
    > @@ -519,10 +520,12 @@ static int move_to_new_page(struct page *newpage, struct page *page)
    > else
    > rc = fallback_migrate_page(mapping, newpage, page);
    >
    > - if (!rc)
    > - remove_migration_ptes(page, newpage);
    > - else
    > - newpage->mapping = NULL;
    > + if (safe_to_remap) {
    > + if (!rc)
    > + remove_migration_ptes(page, newpage);
    > + else
    > + newpage->mapping = NULL;
    > + }
    >
    if (rc)
    newpage->mapping = NULL;
    else if (safe_to_remap)
    remove_migrateion_ptes(page, newpage);

    Is better. Old code cleared newpage->mapping if rc!=0.

    Thanks,
    -Kame




    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2010-04-02 02:29    [W:0.027 / U:30.664 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site