mm anon rmap: remove anon_vma_moveto_tail

mremap() had a clever optimization where move_ptes() did not take the
anon_vma lock to avoid a race with anon rmap users such as page migration.
 Instead, the avc's were ordered in such a way that the origin vma was
always visited by rmap before the destination.  This ordering and the use
of page table locks rmap usage safe.  However, we want to replace the use
of linked lists in anon rmap with an interval tree, and this will make it
harder to impose such ordering as the interval tree will always be sorted
by the avc->vma->vm_pgoff value.  For now, let's replace the
anon_vma_moveto_tail() ordering function with proper anon_vma locking in
move_ptes().  Once we have the anon interval tree in place, we will
re-introduce an optimization to avoid taking these locks in the most
common cases.

Signed-off-by: Michel Lespinasse <walken@google.com>
Cc: Andrea Arcangeli <aarcange@redhat.com>
Cc: Rik van Riel <riel@redhat.com>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Daniel Santos <daniel.santos@pobox.com>
Cc: Hugh Dickins <hughd@google.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

diff --git a/include/linux/rmap.h b/include/linux/rmap.h
index 3fce545df394c61b3c8a7f4babfdbbee212131a8..7f32cec57e6744da01f2ba47e1c96e129c56f674 100644 (file)
--- a/include/linux/rmap.h
+++ b/include/linux/rmap.h
@@ -120,7 +120,6 @@ void anon_vma_init(void);   /* create anon_vma_cachep */
 int  anon_vma_prepare(struct vm_area_struct *);
 void unlink_anon_vmas(struct vm_area_struct *);
 int anon_vma_clone(struct vm_area_struct *, struct vm_area_struct *);
-void anon_vma_moveto_tail(struct vm_area_struct *);
 int anon_vma_fork(struct vm_area_struct *, struct vm_area_struct *);
 
 static inline void anon_vma_merge(struct vm_area_struct *vma,

diff --git a/mm/mmap.c b/mm/mmap.c
index 5ac533f88e99ccf1ef2b13a146b8c076e7949402..66984aab79152f8fba0a55ad5925e8f08dce5cf0 100644 (file)
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -2378,8 +2378,7 @@ struct vm_area_struct *copy_vma(struct vm_area_struct **vmap,
                         */
                        VM_BUG_ON(faulted_in_anon_vma);
                        *vmap = new_vma;
-               } else
-                       anon_vma_moveto_tail(new_vma);
+               }
        } else {
                new_vma = kmem_cache_alloc(vm_area_cachep, GFP_KERNEL);
                if (new_vma) {

diff --git a/mm/mremap.c b/mm/mremap.c
index cc06d0e48d050dc0a2ec47cf62fd8d3b22b4a2be..5588bb6e92956c049932baac68826fbaa6ce4a24 100644 (file)
--- a/mm/mremap.c
+++ b/mm/mremap.c
@@ -74,6 +74,7 @@ static void move_ptes(struct vm_area_struct *vma, pmd_t *old_pmd,
                unsigned long new_addr)
 {
        struct address_space *mapping = NULL;
+       struct anon_vma *anon_vma = vma->anon_vma;
        struct mm_struct *mm = vma->vm_mm;
        pte_t *old_pte, *new_pte, pte;
        spinlock_t *old_ptl, *new_ptl;
@@ -88,6 +89,8 @@ static void move_ptes(struct vm_area_struct *vma, pmd_t *old_pmd,
                mapping = vma->vm_file->f_mapping;
                mutex_lock(&mapping->i_mmap_mutex);
        }
+       if (anon_vma)
+               anon_vma_lock(anon_vma);
 
        /*
         * We don't have to worry about the ordering of src and dst
@@ -114,6 +117,8 @@ static void move_ptes(struct vm_area_struct *vma, pmd_t *old_pmd,
                spin_unlock(new_ptl);
        pte_unmap(new_pte - 1);
        pte_unmap_unlock(old_pte - 1, old_ptl);
+       if (anon_vma)
+               anon_vma_unlock(anon_vma);
        if (mapping)
                mutex_unlock(&mapping->i_mmap_mutex);
 }
@@ -220,15 +225,6 @@ static unsigned long move_vma(struct vm_area_struct *vma,
 
        moved_len = move_page_tables(vma, old_addr, new_vma, new_addr, old_len);
        if (moved_len < old_len) {
-               /*
-                * Before moving the page tables from the new vma to
-                * the old vma, we need to be sure the old vma is
-                * queued after new vma in the same_anon_vma list to
-                * prevent SMP races with rmap_walk (that could lead
-                * rmap_walk to miss some page table).
-                */
-               anon_vma_moveto_tail(vma);
-
                /*
                 * On error, move entries back from new area to old,
                 * which will succeed since page tables still there,

diff --git a/mm/rmap.c b/mm/rmap.c
index 7b5b51d25fc506e215f04ceddb558740bf072193..8cbd62fde0f18528a1eb7e15486bbc8995c70670 100644 (file)
--- a/mm/rmap.c
+++ b/mm/rmap.c
@@ -268,51 +268,6 @@ int anon_vma_clone(struct vm_area_struct *dst, struct vm_area_struct *src)
        return -ENOMEM;
 }
 
-/*
- * Some rmap walk that needs to find all ptes/hugepmds without false
- * negatives (like migrate and split_huge_page) running concurrent
- * with operations that copy or move pagetables (like mremap() and
- * fork()) to be safe. They depend on the anon_vma "same_anon_vma"
- * list to be in a certain order: the dst_vma must be placed after the
- * src_vma in the list. This is always guaranteed by fork() but
- * mremap() needs to call this function to enforce it in case the
- * dst_vma isn't newly allocated and chained with the anon_vma_clone()
- * function but just an extension of a pre-existing vma through
- * vma_merge.
- *
- * NOTE: the same_anon_vma list can still be changed by other
- * processes while mremap runs because mremap doesn't hold the
- * anon_vma mutex to prevent modifications to the list while it
- * runs. All we need to enforce is that the relative order of this
- * process vmas isn't changing (we don't care about other vmas
- * order). Each vma corresponds to an anon_vma_chain structure so
- * there's no risk that other processes calling anon_vma_moveto_tail()
- * and changing the same_anon_vma list under mremap() will screw with
- * the relative order of this process vmas in the list, because we
- * they can't alter the order of any vma that belongs to this
- * process. And there can't be another anon_vma_moveto_tail() running
- * concurrently with mremap() coming from this process because we hold
- * the mmap_sem for the whole mremap(). fork() ordering dependency
- * also shouldn't be affected because fork() only cares that the
- * parent vmas are placed in the list before the child vmas and
- * anon_vma_moveto_tail() won't reorder vmas from either the fork()
- * parent or child.
- */
-void anon_vma_moveto_tail(struct vm_area_struct *dst)
-{
-       struct anon_vma_chain *pavc;
-       struct anon_vma *root = NULL;
-
-       list_for_each_entry_reverse(pavc, &dst->anon_vma_chain, same_vma) {
-               struct anon_vma *anon_vma = pavc->anon_vma;
-               VM_BUG_ON(pavc->vma != dst);
-               root = lock_anon_vma_root(root, anon_vma);
-               list_del(&pavc->same_anon_vma);
-               list_add_tail(&pavc->same_anon_vma, &anon_vma->head);
-       }
-       unlock_anon_vma_root(root);
-}
-
 /*
  * Attach vma to its own anon_vma, as well as to the anon_vmas that
  * the corresponding VMA in the parent process is attached to.