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

[Suren Baghdasaryan]

On Sun, Aug 16, 2026 at 2:15 AM Mike Rapoport <rppt@xxxxxxxxxx> wrote:
>
> (adding Suren)

Thanks Mike!

>
> On Sat, Aug 15, 2026 at 05:42:12PM +0700, Foxie Flakey 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

This description left me scratching my head. If I understand the
problem correctly, the issue is that the err is not cleared after we
decided that we need to retry. If so, how about a simpler explanation:

During move_pages() operation, when move_pages_ptes() returns EAGAIN,
the error code is not cleared even after we processed it. This leads
to a successful retry but then the same pages are retried again due to
the stale error code. This time move fails because pages are already
moved, loop is terminated and move_pages() reports a failure.
Clear the error code once we processes EAGAIN.

> >
> > Fixes: 50944692052b ("userfaultfd: opportunistic TLB-flush batching for present pages in MOVE")
> > Signed-off-by: Foxie Flakey <foxieflakey@xxxxxxxxx>
>
> Is Foxie Flakey your real name?
> Signed-off-by should be using a known identity (sorry, no anonymous contributions.)
>
> > ---
> >  mm/userfaultfd.c | 6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/mm/userfaultfd.c b/mm/userfaultfd.c
> > index c3adedaaf7d5..595e7e232f90 100644
> > --- 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;
> > +                     }

This fix is wrong. It resets the err before we process it and
determine that a retry is needed.
A proper fix is to reset it later here:

if (err) {
-        if (err == -EAGAIN)
+       if (err == -EAGAIN) {
+              err = 0;
               continue;
+       }
         break;
}

> >               }
> >
> >               cond_resched();
> >
> > base-commit: 62cc90241548d5570ee68e01aaba6506964e9811
> > --
> > 2.55.0
> >
>
> --
> Sincerely yours,
> Mike.