Skip to content

Commit 18e7760

Browse files
Hugh Dickinstorvalds
authored andcommitted
khugepaged: retract_page_tables() remember to test exit
Only once have I seen this scenario (and forgot even to notice what forced the eventual crash): a sequence of "BUG: Bad page map" alerts from vm_normal_page(), from zap_pte_range() servicing exit_mmap(); pmd:00000000, pte values corresponding to data in physical page 0. The pte mappings being zapped in this case were supposed to be from a huge page of ext4 text (but could as well have been shmem): my belief is that it was racing with collapse_file()'s retract_page_tables(), found *pmd pointing to a page table, locked it, but *pmd had become 0 by the time start_pte was decided. In most cases, that possibility is excluded by holding mmap lock; but exit_mmap() proceeds without mmap lock. Most of what's run by khugepaged checks khugepaged_test_exit() after acquiring mmap lock: khugepaged_collapse_pte_mapped_thps() and hugepage_vma_revalidate() do so, for example. But retract_page_tables() did not: fix that. The fix is for retract_page_tables() to check khugepaged_test_exit(), after acquiring mmap lock, before doing anything to the page table. Getting the mmap lock serializes with __mmput(), which briefly takes and drops it in __khugepaged_exit(); then the khugepaged_test_exit() check on mm_users makes sure we don't touch the page table once exit_mmap() might reach it, since exit_mmap() will be proceeding without mmap lock, not expecting anyone to be racing with it. Fixes: f3f0e1d ("khugepaged: add support of collapse for tmpfs/shmem pages") Signed-off-by: Hugh Dickins <hughd@google.com> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Acked-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com> Cc: Andrea Arcangeli <aarcange@redhat.com> Cc: Mike Kravetz <mike.kravetz@oracle.com> Cc: Song Liu <songliubraving@fb.com> Cc: <stable@vger.kernel.org> [4.8+] Link: http://lkml.kernel.org/r/alpine.LSU.2.11.2008021215400.27773@eggly.anvils Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
1 parent 119a5fc commit 18e7760

File tree

1 file changed

+14
-10
lines changed

1 file changed

+14
-10
lines changed

mm/khugepaged.c

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1532,6 +1532,7 @@ static int khugepaged_collapse_pte_mapped_thps(struct mm_slot *mm_slot)
15321532
static void retract_page_tables(struct address_space *mapping, pgoff_t pgoff)
15331533
{
15341534
struct vm_area_struct *vma;
1535+
struct mm_struct *mm;
15351536
unsigned long addr;
15361537
pmd_t *pmd, _pmd;
15371538

@@ -1560,7 +1561,8 @@ static void retract_page_tables(struct address_space *mapping, pgoff_t pgoff)
15601561
continue;
15611562
if (vma->vm_end < addr + HPAGE_PMD_SIZE)
15621563
continue;
1563-
pmd = mm_find_pmd(vma->vm_mm, addr);
1564+
mm = vma->vm_mm;
1565+
pmd = mm_find_pmd(mm, addr);
15641566
if (!pmd)
15651567
continue;
15661568
/*
@@ -1570,17 +1572,19 @@ static void retract_page_tables(struct address_space *mapping, pgoff_t pgoff)
15701572
* mmap_lock while holding page lock. Fault path does it in
15711573
* reverse order. Trylock is a way to avoid deadlock.
15721574
*/
1573-
if (mmap_write_trylock(vma->vm_mm)) {
1574-
spinlock_t *ptl = pmd_lock(vma->vm_mm, pmd);
1575-
/* assume page table is clear */
1576-
_pmd = pmdp_collapse_flush(vma, addr, pmd);
1577-
spin_unlock(ptl);
1578-
mmap_write_unlock(vma->vm_mm);
1579-
mm_dec_nr_ptes(vma->vm_mm);
1580-
pte_free(vma->vm_mm, pmd_pgtable(_pmd));
1575+
if (mmap_write_trylock(mm)) {
1576+
if (!khugepaged_test_exit(mm)) {
1577+
spinlock_t *ptl = pmd_lock(mm, pmd);
1578+
/* assume page table is clear */
1579+
_pmd = pmdp_collapse_flush(vma, addr, pmd);
1580+
spin_unlock(ptl);
1581+
mm_dec_nr_ptes(mm);
1582+
pte_free(mm, pmd_pgtable(_pmd));
1583+
}
1584+
mmap_write_unlock(mm);
15811585
} else {
15821586
/* Try again later */
1583-
khugepaged_add_pte_mapped_thp(vma->vm_mm, addr);
1587+
khugepaged_add_pte_mapped_thp(mm, addr);
15841588
}
15851589
}
15861590
i_mmap_unlock_write(mapping);

0 commit comments

Comments
 (0)