Re: [PATCH 6/6] userfaultfd: collapse VM_UFFD_{MISSING,WP,MINOR,RWP} into single VM_UFFD

[Lance Yang]

On 2026/8/24 16:17, Mike Rapoport wrote:
> Hi Lance,
>
> On Mon, Aug 24, 2026 at 03:11:26PM +0800, Lance Yang wrote:
> > On Sun, Aug 23, 2026 at 03:17:43PM +0300, Mike Rapoport (Microsoft) wrote:
> > [...]
> > > diff --git a/mm/userfaultfd.c b/mm/userfaultfd.c
> > > index 83587d34b189..193f6e65d875 100644
> > > --- a/mm/userfaultfd.c
> > > +++ b/mm/userfaultfd.c
> > > @@ -50,10 +50,10 @@ struct mfill_state {
> > > 	pmd_t *pmd;
> > > };
> > >
> > > -static bool anon_can_userfault(struct vm_area_struct *vma, vm_flags_t vm_flags)
> > > +static bool anon_can_userfault(struct vm_area_struct *vma, unsigned int mode)
> > > {
> > > 	/* anonymous memory does not support MINOR mode */
> > > -	if (vm_flags & VM_UFFD_MINOR)
> > > +	if (mode & UFFD_MODE_MINOR)
> > > 		return false;
> > > 	return true;
> > > }
> > > @@ -462,7 +462,7 @@ static int mfill_copy_folio_locked(struct folio *folio, unsigned long src_addr)
> > > }
> > >
> > > #define MFILL_RETRY_STATE_VMA_FLAGS \
> > > -	append_vma_flags(__VMA_UFFD_FLAGS, VMA_SHARED_BIT)
> > > +	append_vma_flags(VMA_UFFD, VMA_SHARED_BIT)
> >
> > Looks like this drops registration mode from the retry snapshot. Assume a
> > shared shmem VMA is registered for MISSING and COPY reaches
> > mfill_copy_folio_retry(). While locks are dropped, the same userfaultfd|
> > can re-register the range for MINOR. VMA_UFFD, VM_SHARED, ops, file and
> > pgoff all stay unchanged, so the old COPY can continue instead of
> > returning -EAGAIN ... no?
>
> Good catch, thanks!

Cheers!

>   
> > Maybe something like this?
>
> I prefer to add mode to the retry_state:
>
> diff --git a/mm/userfaultfd.c b/mm/userfaultfd.c
> index 193f6e65d875..a0377f7ecc69 100644
> --- a/mm/userfaultfd.c
> +++ b/mm/userfaultfd.c
> @@ -471,6 +471,7 @@ static int mfill_copy_folio_locked(struct folio *folio, unsigned long src_addr)
>    */
>   struct mfill_retry_state {
>   	const struct vm_uffd_ops *ops;
> +	unsigned long mode;
>   	struct file *file;
>   	vma_flags_t flags;
>   	pgoff_t pgoff;
> @@ -482,6 +483,7 @@ static void mfill_retry_state_save(struct mfill_retry_state *s,
>   	s->flags = vma_flags_and_mask(&vma->flags, MFILL_RETRY_STATE_VMA_FLAGS);
>   	s->ops = vma_uffd_ops(vma);
>   	s->pgoff = vma_start_pgoff(vma);
> +	s->mode = uffd_mode(vma);
>   
>   	if (vma->vm_file)
>   		s->file = get_file(vma->vm_file);
> @@ -493,8 +495,9 @@ static bool mfill_retry_state_changed(struct mfill_retry_state *state,
>   	vma_flags_t flags = vma_flags_and_mask(&vma->flags,
>   					       MFILL_RETRY_STATE_VMA_FLAGS);
>   
> -	/* Have any UFFD flags (missing, WP, minor) changed? */
> -	if (!vma_flags_same_pair(&state->flags, &flags))
> +	/* UFFD registration mode or VMA sharing changed */
> +	if (!vma_flags_same_pair(&state->flags, &flags) ||
> +	    s->mode != uffd_mode(vma))

I assume you meant

s/s->mode/state->mode/

With that, LGTM :)

>   		return true;
>   
>   	/* VMA type or effective uffd_ops changed while the lock was dropped */
>   
> > Cheers, Lance

Cheers, Lance