Re: [PATCH] userfaultfd: reset err to be 0 when move_pages_ptes succeeded
From: Foxie Flakey
Date: Mon Aug 17 2026 - 21:13:12 EST
Hi, Andrew.
On Mon, 17 Aug 2026, Andrew Morton wrote:
> On Sat, 15 Aug 2026 17:42:12 +0700 (WIB) Foxie Flakey <foxieflakey@xxxxxxxxx> 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 <foxieflakey@xxxxxxxxx>
>
> 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)
> > --- 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/9c936a9f-ed27-e510-872f-5b3b8c680975@xxxxxxxxx
>