lkml.org 
[lkml]   [2010]   [Apr]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 01/14] mm,migration: Take a reference to the anon_vma before migrating
    On Tue, 20 Apr 2010 22:01:03 +0100
    Mel Gorman <mel@csn.ul.ie> wrote:

    > rmap_walk_anon() does not use page_lock_anon_vma() for looking up and
    > locking an anon_vma and it does not appear to have sufficient locking to
    > ensure the anon_vma does not disappear from under it.
    >
    > This patch copies an approach used by KSM to take a reference on the
    > anon_vma while pages are being migrated. This should prevent rmap_walk()
    > running into nasty surprises later because anon_vma has been freed.
    >
    > Signed-off-by: Mel Gorman <mel@csn.ul.ie>
    > Acked-by: Rik van Riel <riel@redhat.com>

    Reviewed-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>

    Hmm, a nitpick..


    > static inline struct anon_vma *page_anon_vma(struct page *page)
    > {
    > diff --git a/mm/migrate.c b/mm/migrate.c
    > index 5938db5..b768a1d 100644
    > --- a/mm/migrate.c
    > +++ b/mm/migrate.c
    > @@ -543,6 +543,7 @@ static int unmap_and_move(new_page_t get_new_page, unsigned long private,
    > int rcu_locked = 0;
    > int charge = 0;
    > struct mem_cgroup *mem = NULL;
    > + struct anon_vma *anon_vma = NULL;
    >
    > if (!newpage)
    > return -ENOMEM;
    > @@ -599,6 +600,8 @@ static int unmap_and_move(new_page_t get_new_page, unsigned long private,
    > if (PageAnon(page)) {
    > rcu_read_lock();
    > rcu_locked = 1;
    > + anon_vma = page_anon_vma(page);
    > + atomic_inc(&anon_vma->migrate_refcount);
    > }
    >
    > /*
    > @@ -638,6 +641,15 @@ skip_unmap:
    > if (rc)
    > remove_migration_ptes(page, page);
    > rcu_unlock:
    > +
    > + /* Drop an anon_vma reference if we took one */
    > + if (anon_vma && atomic_dec_and_lock(&anon_vma->migrate_refcount, &anon_vma->lock)) {
    > + int empty = list_empty(&anon_vma->head);
    > + spin_unlock(&anon_vma->lock);
    > + if (empty)
    > + anon_vma_free(anon_vma);
    > + }
    > +

    Hmm. Can't we move this part to mm/rmap.c ?

    as
    anon_vma_drop_external_reference()
    or some.

    Thanks,
    -Kame



    \
     
     \ /
      Last update: 2010-04-21 04:55    [from the cache]
    ©2003-2014 Jasper Spaans. Advertise on this site