Re: [PATCH v6] mm: retry page faults once under the per-VMA lock
flat view
From: "Lorenzo Stoakes (ARM)" <ljs@kernel.org>
Date: 2026-10-02 13:12:57
Also in:
linux-arm-kernel, linux-mm, linux-riscv, linux-s390, lkml, loongarch
On Thu, Oct 01, 2026 at 10:12:16PM +0100, Matthew Wilcox wrote:
quoted hunk ↗ jump to hunk
So what I didn't realise is that fork() waits for page faults to finish. I don't think that's necessary, so we can just stop doing that (whitespace damaged):diff --git a/mm/mmap.c b/mm/mmap.c index 4bf26b0f1e6e..e79555247d3a 100644 --- a/mm/mmap.c +++ b/mm/mmap.c@@ -1739,9 +1739,6 @@ __latent_entropy int dup_mmap(struct mm_struct *mm, struct mm_struct *oldmm) for_each_vma(vmi, mpnt) { struct file *file; - retval = vma_start_write_killable(mpnt); - if (retval < 0) - goto loop_out; if (vma_test(mpnt, VMA_DONTCOPY_BIT)) { retval = vma_iter_clear_gfp(&vmi, mpnt->vm_start, mpnt->vm_end, GFP_KERNEL);I think this is safe. I've booted a kernel with this change, and
I'm afraid it's not safe :)
We already actually did this (by mistake I think?) before. See commit
fb49c455323f ("fork: lock VMAs of the parent process when forking") - we shipped
that in 6.4.0 -> 6.4.2 and it corrupted memory all over the place.
Reports + a consistent reproducer:
https://bugzilla.kernel.org/show_bug.cgi?id=217624
https://lore.kernel.org/all/dbdef34c-3a07-5951-e1ae-e9c6e3cdf51b@kernel.org/ (local)
https://lore.kernel.org/all/b198d649-f4bf-b971-31d0-e8433ec2a34c@applied-asynchrony.com/ (local)
Also we have asserts for this lock in copy_page_range() and
copy_hugetlb_page_range() which will now fail (and possibly more further down
the stack?)
My argument for why it's safe is that a thread which takes a page fault during fork() might have taken the page fault either before or after fork(). The faults will definitely happen in the parent process. They may or may not have happened in the child process, which can't possibly care whether or not they've happened.
The problem isn't the child it's the parent - copy_page_range() _modifies_ the _parent_'s page tables in-place, on assumption that nothing else can access them (for CoW). And actually nowadays things are _even_ worse, you could also have concurrent page faults, MADV_DONTNEED, MADV_FREE, MADV_GUARD_INSTALL/REMOVE, UFFDIO_COPY + UFFDIO_MOVE (yikes) all of which might introduce new issues + would have to be audited too. So - it turns out a lot of stuff implicitly assumes VMA write lock now: 1. CoW - deferred TLB flush race (The issue above) https://lore.kernel.org/all/20230705171213.2843068-2-surenb@google.com/ (local) This happens because __copy_present_ptes() does wrprotect_ptes() on the parent without a TLB flush (that's deferred until the end of the fork). But unfortunately without a VMA write lock something can then replace the page table entry in the meantime and you can end up with data loss as a result. Same issue exists with hugetlb wp too. And god only knows with uffd wp :) 2. THP - can destroy parent THP mappings copy_pmd_range() does: if (pmd_is_huge(*src_pmd)) { copy_huge_pmd(); ... } if (pmd_none_or_clear_bad(src_pmd)) continue; Without the write lock, a concurrent fault can install a huge PMD via do_huge_pmd_anonymous_page() or filemap_map_pmd()/do_set_pmd(). The issue is that pmd_bad() fires for leaf PMD (ugh) on at least x86 and arm64, meaning that'll clear the page table entry. I think Hugh left a comment alluding to this before: * copy_pmd_range()'s prior pmd_none_or_clear_bad(src_pmd), and the * error handling here, assume that exclusive mmap_lock on dst and src * protects anon from unexpected THP transitions; ... It's like the mess around pmd_trans_unstable() :( 3. copy_pte_range() skips the required pmd_same() recheck assuming write lock It uses pte_offset_map_rw_nolock() with a dummy pmdval and: * We already hold the exclusive mmap_lock, the copy_pte_range() and * retract_page_tables() are using vma->anon_vma to be exclusive, so * the PTE page is stable, and there is no need to get pmdval and do * pmd_same() check. (Probably needs updating for VMA write lock...) 4. The asserts Mentioned above! I suspect there's more as well. Some of it could be changed, but it'd all be at the cost of behaving very strangely (no VMA lock but mmap write lock) while manipulating things. And I am not really happy with changing page table entries in the parent VMA (for CoW) without a VMA write lock held. I also wonder whether it would achieve all that much really - the issue is with holding the VMA lock across I/O, anything else that tries to grab a write lock is going to end up being the new thing that's blocked (mprotect, munmap, mremap, mlock etc.) I think doing something like this would need a very significant redesign and I'm not convinced it's one that really achieves what we want given all the other cases that'd get blocked by the VMA lock being held still. -- Cheers, Lorenzo