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)" <[email protected]> > > > > 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) <[email protected]> > > --- > > [...] > > > 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) <[email protected]> Thanks! > -- > Cheers, > > David -- Sincerely yours, Mike.
