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

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