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

Foxie Flakey <[email protected]>
Newsgroups org.kernel.vger.linux-kernel,org.kvack.linux-mm
Message-ID <[email protected]>
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)

> > --- 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]
>
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.