Re: [PATCH v2 02/15] userfaultfd: introduce struct mfill_state

[Mike Rapoport]

On Fri, Mar 20, 2026 at 01:43:02PM +0100, David Hildenbrand (Arm) wrote:
> On 3/6/26 18:18, Mike Rapoport wrote:
> > From: "Mike Rapoport (Microsoft)" <rppt@xxxxxxxxxx>
> > 
> > mfill_atomic() passes a lot of parameters down to its callees.
> > 
> > Aggregate them all into mfill_state structure and pass this structure to
> > functions that implement various UFFDIO_ commands.
> > 
> > Tracking the state in a structure will allow moving the code that retries
> > copying of data for UFFDIO_COPY into mfill_atomic_pte_copy() and make the
> > loop in mfill_atomic() identical for all UFFDIO operations on PTE-mapped
> > memory.
> > 
> > The mfill_state definition is deliberately local to mm/userfaultfd.c, hence
> > shmem_mfill_atomic_pte() is not updated.
> > 
> > Signed-off-by: Mike Rapoport (Microsoft) <rppt@xxxxxxxxxx>
> > ---
> 
> [...]
> 
> >  			if (fatal_signal_pending(current))
> > @@ -866,10 +882,10 @@ static __always_inline ssize_t mfill_atomic(struct userfaultfd_ctx *ctx,
> >  
> >  out_unlock:
> >  	up_read(&ctx->map_changing_lock);
> > -	uffd_mfill_unlock(dst_vma);
> > +	uffd_mfill_unlock(state.vma);
> >  out:
> > -	if (folio)
> > -		folio_put(folio);
> > +	if (state.folio)
> > +		folio_put(state.folio);
> >  	VM_WARN_ON_ONCE(copied < 0);
> >  	VM_WARN_ON_ONCE(err > 0);
> >  	VM_WARN_ON_ONCE(!copied && !err);
> >	return copied ? copied : err;
> 
> I guess, "copied" could be easily avoided by
> comparing state.src_addr with start_addr.

Yeah, but a dedicated variable is more robust against potential changes
in state handling. And it's surely more descriptive and shorter than 

state.src_addr - src_addr;
 
> Acked-by: David Hildenbrand (Arm) <david@xxxxxxxxxx>

Thanks!
 
> -- 
> Cheers,
> 
> David

-- 
Sincerely yours,
Mike.