mm/khugepaged: take the right locks for page table retraction

[platform/kernel/linux-starfive.git] / mm / khugepaged.c

diff --git a/mm/khugepaged.c b/mm/khugepaged.c
index a8d5ef2..0a11e13 100644 (file)
--- a/mm/khugepaged.c
+++ b/mm/khugepaged.c
@@ -1379,16 +1379,37 @@ static int set_huge_pmd(struct vm_area_struct *vma, unsigned long addr,
        return SCAN_SUCCEED;
 }
 
+/*
+ * A note about locking:
+ * Trying to take the page table spinlocks would be useless here because those
+ * are only used to synchronize:
+ *
+ *  - modifying terminal entries (ones that point to a data page, not to another
+ *    page table)
+ *  - installing *new* non-terminal entries
+ *
+ * Instead, we need roughly the same kind of protection as free_pgtables() or
+ * mm_take_all_locks() (but only for a single VMA):
+ * The mmap lock together with this VMA's rmap locks covers all paths towards
+ * the page table entries we're messing with here, except for hardware page
+ * table walks and lockless_pages_from_mm().
+ */
 static void collapse_and_free_pmd(struct mm_struct *mm, struct vm_area_struct *vma,
                                  unsigned long addr, pmd_t *pmdp)
 {
-       spinlock_t *ptl;
        pmd_t pmd;
 
        mmap_assert_write_locked(mm);
-       ptl = pmd_lock(vma->vm_mm, pmdp);
+       if (vma->vm_file)
+               lockdep_assert_held_write(&vma->vm_file->f_mapping->i_mmap_rwsem);
+       /*
+        * All anon_vmas attached to the VMA have the same root and are
+        * therefore locked by the same lock.
+        */
+       if (vma->anon_vma)
+               lockdep_assert_held_write(&vma->anon_vma->root->rwsem);
+
        pmd = pmdp_collapse_flush(vma, addr, pmdp);
-       spin_unlock(ptl);
        mm_dec_nr_ptes(mm);
        page_table_check_pte_clear_range(mm, addr, pmd);
        pte_free(mm, pmd_pgtable(pmd));
@@ -1439,6 +1460,14 @@ int collapse_pte_mapped_thp(struct mm_struct *mm, unsigned long addr,
        if (!hugepage_vma_check(vma, vma->vm_flags, false, false, false))
                return SCAN_VMA_CHECK;
 
+       /*
+        * Symmetry with retract_page_tables(): Exclude MAP_PRIVATE mappings
+        * that got written to. Without this, we'd have to also lock the
+        * anon_vma if one exists.
+        */
+       if (vma->anon_vma)
+               return SCAN_VMA_CHECK;
+
        /* Keep pmd pgtable for uffd-wp; see comment in retract_page_tables() */
        if (userfaultfd_wp(vma))
                return SCAN_PTE_UFFD_WP;
@@ -1472,6 +1501,20 @@ int collapse_pte_mapped_thp(struct mm_struct *mm, unsigned long addr,
                goto drop_hpage;
        }
 
+       /*
+        * We need to lock the mapping so that from here on, only GUP-fast and
+        * hardware page walks can access the parts of the page tables that
+        * we're operating on.
+        * See collapse_and_free_pmd().
+        */
+       i_mmap_lock_write(vma->vm_file->f_mapping);
+
+       /*
+        * This spinlock should be unnecessary: Nobody else should be accessing
+        * the page tables under spinlock protection here, only
+        * lockless_pages_from_mm() and the hardware page walker can access page
+        * tables while all the high-level locks are held in write mode.
+        */
        start_pte = pte_offset_map_lock(mm, pmd, haddr, &ptl);
        result = SCAN_FAIL;
 
@@ -1526,6 +1569,8 @@ int collapse_pte_mapped_thp(struct mm_struct *mm, unsigned long addr,
        /* step 4: remove pte entries */
        collapse_and_free_pmd(mm, vma, haddr, pmd);
 
+       i_mmap_unlock_write(vma->vm_file->f_mapping);
+
 maybe_install_pmd:
        /* step 5: install pmd entry */
        result = install_pmd
@@ -1539,6 +1584,7 @@ drop_hpage:
 
 abort:
        pte_unmap_unlock(start_pte, ptl);
+       i_mmap_unlock_write(vma->vm_file->f_mapping);
        goto drop_hpage;
 }
 
@@ -1595,7 +1641,8 @@ static int retract_page_tables(struct address_space *mapping, pgoff_t pgoff,
                 * An alternative would be drop the check, but check that page
                 * table is clear before calling pmdp_collapse_flush() under
                 * ptl. It has higher chance to recover THP for the VMA, but
-                * has higher cost too.
+                * has higher cost too. It would also probably require locking
+                * the anon_vma.
                 */
                if (vma->anon_vma) {
                        result = SCAN_PAGE_ANON;