Re: [PATCH] userfaultfd: reset err to be 0 when move_pages_ptes succeeded

On Mon, Aug 17, 2026 at 6:13 PM Foxie Flakey <[email protected]> wrote:
>
> Hi, Andrew.
>
> On Mon, 17 Aug 2026, Andrew Morton wrote:
>
> > On Sat, 15 Aug 2026 17:42:12 +0700 (WIB) Foxie Flakey <[email protected]> wrote:
> >
> > >
> > > An fix for edge case can occur if move_pages_ptes return -EAGAIN, later
> > > when checked and it is EAGAIN, outer loop would retry again on same page
> > > and succeeded but the err isn't reset so the outer loop would think need
> > > to retry again so it goes back again and move pages again. On third attempt
> > > move_pages_ptes will fail because it already moved and returns an error
> > > that is not EAGAIN when outer loop checks again it sees non EAGAIN so it
> > > dont retry and break out of loop. When loop is terminated it did not update
> > > the "moved" variable from successful 2nd iteration.
> > >
> > > That behaviour manifested into this at userspace
> > >
> > > Source:      [ .. unmapped  .. ][ .. mapped    ..]
> > > Destination: [ .. mapped    .. ][ .. unmapped  ..]
> > >                           ^     ^
> > >                           \     Kernel moved this far in actuality
> > >                            What is reported to userspace on struct
> > >                            uffdio_move's move field
> > >
> > > When the previous behaviour is
> > > Source:      [ .. unmapped  .. ][ .. mapped    ..]
> > > Destination: [ .. mapped    .. ][ .. unmapped  ..]
> > >                                 ^
> > >                                 Reported to user space via uffdio_move's
> > >                                 move field
> >
> > Thanks.
> >
> > The text is a bit hard to follow.  I asked Gemini to redo it and
> > perhaps you prefer that?  https://share.gemini.google/cOWn3pQadvVw
>
> Yes, thanks. I have already make cleaner text from feedback on other branch
> of mails with Suren.
>
> > > Fixes: 50944692052b ("userfaultfd: opportunistic TLB-flush batching for present pages in MOVE")
> > > Signed-off-by: Foxie Flakey <[email protected]>
> >
> > Yes, the pseudonym is problematic - it is contrary to our written
> > rules.  But I'm a sucker for fixes, sigh.  Perhaps if someone else were
> > to send me your patch with their signoff also, the rules would be less
> > offended.
>
> I see, I'll post the updated patch and work on signoff issue later (there
> has been updates, I haven't posted v2 which updates the patch and
> description following a feedback. Why I didn't? mainly I'm not sure with
> answer for signoff)

Yeah, please follow Mike's advise.

>
> > > --- a/mm/userfaultfd.c
> > > +++ b/mm/userfaultfd.c
> > > @@ -2069,10 +2069,12 @@ static ssize_t move_pages(struct userfaultfd_ctx *ctx, unsigned long dst_start,
> > >                     ret = move_pages_ptes(mm, dst_pmd, src_pmd,
> > >                                           dst_vma, src_vma, dst_addr,
> > >                                           src_addr, src_end - src_addr, mode);
> > > -                   if (ret < 0)
> > > +                   if (ret < 0) {
> > >                             err = ret;
> > > -                   else
> > > +                   } else {
> > > +                           err = 0;
> > >                             step_size = ret;
> > > +                   }
> > >             }
> > >
> > >             cond_resched();
> >
> > Maintainers, when reviewing this please let me know whether you think
> > it should be backported.
> >
> > Sashiko did what it usually does when we make it look at uffd:
> >       https://sashiko.dev/#/patchset/[email protected]
> >
>