Re: [PATCH v2 09/15] userfaultfd: introduce vm_uffd_ops->alloc_folio()

[Mike Rapoport <rppt@kernel.org>]

On Thu, Mar 26, 2026 at 05:07:08PM -0700, James Houghton wrote:
> On Fri, Mar 6, 2026 at 9:19 AM Mike Rapoport <rppt@kernel.org> wrote:
> >
> > From: "Mike Rapoport (Microsoft)" <rppt@kernel.org>
> >
> > and use it to refactor mfill_atomic_pte_zeroed_folio() and
> > mfill_atomic_pte_copy().
> >
> > mfill_atomic_pte_zeroed_folio() and mfill_atomic_pte_copy() perform
> > almost identical actions:
> > * allocate a folio
> > * update folio contents (either copy from userspace of fill with zeros)
> > * update page tables with the new folio
> >
> > Split a __mfill_atomic_pte() helper that handles both cases and uses
> > newly introduced vm_uffd_ops->alloc_folio() to allocate the folio.
> >
> > Pass the ops structure from the callers to __mfill_atomic_pte() to later
> > allow using anon_uffd_ops for MAP_PRIVATE mappings of file-backed VMAs.
> >
> > Note, that the new ops method is called alloc_folio() rather than
> > folio_alloc() to avoid clash with alloc_tag macro folio_alloc().
> >
> > Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
> 
> Feel free to add:
> 
> Reviewed-by: James Houghton <jthoughton@google.com>

Thanks!
 
> > ---
> >  include/linux/userfaultfd_k.h |  6 +++
> >  mm/userfaultfd.c              | 92 ++++++++++++++++++-----------------
> >  2 files changed, 54 insertions(+), 44 deletions(-)
> >
> > diff --git a/include/linux/userfaultfd_k.h b/include/linux/userfaultfd_k.h
> > index 66dfc3c164e6..4d8b879eed91 100644
> > --- a/include/linux/userfaultfd_k.h
> > +++ b/include/linux/userfaultfd_k.h
> > @@ -91,6 +91,12 @@ struct vm_uffd_ops {
> >          * The returned folio is locked and with reference held.
> >          */
> >         struct folio *(*get_folio_noalloc)(struct inode *inode, pgoff_t pgoff);
> > +       /*
> > +        * Called during resolution of UFFDIO_COPY request.
> > +        * Should return allocate a and return folio or NULL if allocation fails.
> 
> "Should allocate and return a folio or NULL if allocation fails."
> 
> I see this mistake is fixed in the next patch. :)

Endless rebases :)
Will try to sort it out this time :)
 
> > @@ -483,9 +498,15 @@ static int mfill_atomic_pte_copy(struct mfill_state *state)
> >                  * If there was an error, we must mfill_put_vma() anyway and it
> >                  * will take care of unlocking if needed.
> >                  */
> > -               ret = mfill_copy_folio_retry(state, folio);
> > -               if (ret)
> > -                       goto out_release;
> > +               if (unlikely(ret)) {
> > +                       ret = mfill_copy_folio_retry(state, folio);
> > +                       if (ret)
> > +                               goto err_folio_put;
> > +               }
> > +       } else if (uffd_flags_mode_is(flags, MFILL_ATOMIC_ZEROPAGE)) {
> > +               clear_user_highpage(&folio->page, state->dst_addr);
> > +       } else {
> > +               VM_WARN_ONCE(1, "unknown UFFDIO operation");
> 
> "Unknown UFFDIO operation. flags=%x", flags
> 
> seems a little better to me.

Yeah, why not.
 

-- 
Sincerely yours,
Mike.