Re: [PATCH v4 5/9] mm: introduce copy-on-fork VMAs and make VM_MAYBE_GUARD one

[Chris Mason <clm@meta.com>]

On Tue, 18 Nov 2025 10:17:47 +0000 Lorenzo Stoakes <lorenzo.stoakes@oracle.com> wrote:

> Gather all the VMA flags whose presence implies that page tables must be
> copied on fork into a single bitmap - VM_COPY_ON_FORK - and use this
> rather than specifying individual flags in vma_needs_copy().
> 
> We also add VM_MAYBE_GUARD to this list, as it being set on a VMA implies
> that there may be metadata contained in the page tables (that is - guard
> markers) which would will not and cannot be propagated upon fork.
> 
> This was already being done manually previously in vma_needs_copy(), but
> this makes it very explicit, alongside VM_PFNMAP, VM_MIXEDMAP and
> VM_UFFD_WP all of which imply the same.
> 
> Note that VM_STICKY flags ought generally to be marked VM_COPY_ON_FORK too
> - because equally a flag being VM_STICKY indicates that the VMA contains
> metadat that is not propagated by being faulted in - i.e.  that the VMA
> metadata does not fully describe the VMA alone, and thus we must propagate
> whatever metadata there is on a fork.
> 
> However, for maximum flexibility, we do not make this necessarily the case
> here.
> 

Hi Lorenzo, one more from the review automation:

> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index fea113d1d723c..af2904aeb1631 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -555,6 +555,32 @@ extern unsigned int kobjsize(const void *objp);
>  */
> #define VM_IGNORE_MERGE (VM_SOFTDIRTY | VM_STICKY)
>
> +/*
> + * Flags which should result in page tables being copied on fork. These are
> + * flags which indicate that the VMA maps page tables which cannot be
> + * reconsistuted upon page fault, so necessitate page table copying upon
> + *
> + * VM_PFNMAP / VM_MIXEDMAP - These contain kernel-mapped data which cannot be
> + *                           reasonably reconstructed on page fault.
> + *
> + *              VM_UFFD_WP - Encodes metadata about an installed uffd
> + *                           write protect handler, which cannot be
> + *                           reconstructed on page fault.
> + *
> + *                           We always copy pgtables when dst_vma has uffd-wp
                                                           ^^^^^^^
The comment says "dst_vma" but the new code in vma_needs_copy() checks
src_vma->vm_flags. Is this intentional?

The old code checked userfaultfd_wp(dst_vma), which tests whether the
child VMA has VM_UFFD_WP set. After dup_userfaultfd() clears VM_UFFD_WP
from the child when !UFFD_FEATURE_EVENT_FORK, the old code would skip
page table copying since the child no longer needs uffd-wp protection.

The new code checks src_vma->vm_flags & VM_COPY_ON_FORK, which includes
VM_UFFD_WP. Since the parent VMA still has VM_UFFD_WP set, this will now
force page table copying even when the child has had VM_UFFD_WP cleared.

For file-backed VMAs (e.g., shmem) with uffd-wp but !UFFD_FEATURE_EVENT_FORK,
this appears to cause unnecessary page table copying where the old code
would have allowed the child to fault pages in fresh.

The call chain is:

    dup_mmap()
      -> vm_area_dup(mpnt)                  // child gets same flags
      -> dup_userfaultfd(tmp, &uf)          // may clear VM_UFFD_WP from child
      -> copy_page_range(tmp, mpnt)
           -> vma_needs_copy(dst_vma=tmp, src_vma=mpnt)

> + *                           enabled even if it's file-backed
> + *                           (e.g. shmem). Because when uffd-wp is enabled,
> + *                           pgtable contains uffd-wp protection information,
> + *                           that's something we can't retrieve from page cache,
> + *                           and skip copying will lose those info.
> + *
> + *          VM_MAYBE_GUARD - Could contain page guard region markers which
> + *                           by design are a property of the page tables
> + *                           only and thus cannot be reconstructed on page
> + *                           fault.
> + */
> +#define VM_COPY_ON_FORK (VM_PFNMAP | VM_MIXEDMAP | VM_UFFD_WP | VM_MAYBE_GUARD)

[ ... ]

> diff --git a/mm/memory.c b/mm/memory.c
> index d1728d0538d64..27bc457b32c2e 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -1463,25 +1463,15 @@ copy_p4d_range(struct vm_area_struct *dst_vma, struct vm_area_struct *src_vma,
> static bool
> vma_needs_copy(struct vm_area_struct *dst_vma, struct vm_area_struct *src_vma)
> {
> +	if (src_vma->vm_flags & VM_COPY_ON_FORK)
        ^^^^^^^^
Was the change from checking dst_vma to src_vma for VM_UFFD_WP intentional?

The old code was:

    if (userfaultfd_wp(dst_vma))
        return true;

which expanded to:

    if (dst_vma->vm_flags & VM_UFFD_WP)
        return true;