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] >