Re: [BUG]userfaultfd_move fails to move a folio when swap-in occurs concurrently with swap-out

[Barry Song <21cnbao@gmail.com>]

On Fri, May 23, 2025 at 11:44 AM Lokesh Gidra <lokeshgidra@google.com> wrote:
>
> Thanks Barry for stress testing MOVE ioctl. It's really helpful :)
>
> On Thu, May 22, 2025 at 4:23 PM Barry Song <21cnbao@gmail.com> wrote:
> >
> > Hi All,
> >
> > I'm encountering another bug that can be easily reproduced using the small
> > program below[1], which performs swap-out and swap-in in parallel.
> >
> > The issue occurs when a folio is being swapped out while it is accessed
> > concurrently. In this case, do_swap_page() handles the access. However,
> > because the folio is under writeback, do_swap_page() completely removes
> > its exclusive attribute.
> >
> > do_swap_page:
> >                } else if (exclusive && folio_test_writeback(folio) &&
> >                           data_race(si->flags & SWP_STABLE_WRITES)) {
> >                         ...
> >                         exclusive = false;
> >
> > As a result, userfaultfd_move() will return -EBUSY, even though the
> > folio is not shared and is in fact exclusively owned.
> >
> >                         folio = vm_normal_folio(src_vma, src_addr,
> > orig_src_pte);
> >                         if (!folio || !PageAnonExclusive(&folio->page)) {
> >                                 spin_unlock(src_ptl);
> > +                               pr_err("%s %d folio:%lx exclusive:%d
> > swapcache:%d\n",
> > +                                       __func__, __LINE__, folio,
> > PageAnonExclusive(&folio->page),
> > +                                       folio_test_swapcache(folio));
> >                                 err = -EBUSY;
> >                                 goto out;
> >                         }
> >
> > I understand that shared folios should not be moved. However, in this
> > case, the folio is not shared, yet its exclusive flag is not set.
> >
> > Therefore, I believe PageAnonExclusive is not a reliable indicator of
> > whether a folio is truly exclusive to a process.
> >
> > The kernel log output is shown below:
> > [   23.009516] move_pages_pte 1285 folio:fffffdffc01bba40 exclusive:0
> > swapcache:1
> >
> > I'm still struggling to find a real fix; it seems quite challenging.
> > Please let me know if you have any ideas. In any case It seems
> > userspace should fall back to userfaultfd_copy.
> >
> I'm not sure this is really a bug. A page under write-back is in a way
> 'busy' isn't it? I am not an expert of anon-exclusive, but it seems to
> me that an exclusively mapped anonymous page would have it true. So,
> isn't it expected that a page under write-back will not have it set as
> the page isn't mapped?

We have two return codes: -EAGAIN and -EBUSY. In many cases, we return
-EAGAIN, which is transparent to userspace because the syscall is retried.

Therefore, I expect -EAGAIN or a similar code here to avoid userspace noise,
since we handle other cases where folios are undergoing transitions to
become stable again by -EAGAIN.

>
> I have observed this in my testing as well, and there are a couple of
> ways to deal with it in userspace. As you suggested, falling back to
> userfaultfd_copy on receiving -EBUSY is one option. In my case, making
> a fake store on the src page and then retrying has been working fine.

Good to know you have some fallbacks implemented in userspace.
That makes the issue less serious now.

> >
> >
> > [1] The small program:
> >
> > //Just in a couple of seconds, we are running into
> > //"UFFDIO_MOVE: Device or resource busy"
> >
> > #define _GNU_SOURCE
> > #include <stdio.h>
> > #include <stdlib.h>
> > #include <string.h>
> > #include <sys/mman.h>
> > #include <sys/ioctl.h>
> > #include <sys/syscall.h>
> > #include <linux/userfaultfd.h>
> > #include <fcntl.h>
> > #include <pthread.h>
> > #include <unistd.h>
> > #include <poll.h>
> > #include <errno.h>
> >
> > #define PAGE_SIZE 4096
> > #define REGION_SIZE (512 * 1024)
> >
> > #ifndef UFFDIO_MOVE
> > struct uffdio_move {
> >     __u64 dst;
> >     __u64 src;
> >     __u64 len;
> >     #define UFFDIO_MOVE_MODE_DONTWAKE        ((__u64)1<<0)
> >     #define UFFDIO_MOVE_MODE_ALLOW_SRC_HOLES ((__u64)1<<1)
> >     __u64 mode;
> >     __s64 move;
> > };
> >
> > #define _UFFDIO_MOVE  (0x05)
> > #define UFFDIO_MOVE   _IOWR(UFFDIO, _UFFDIO_MOVE, struct uffdio_move)
> > #endif
> >
> >
> > void *src, *dst;
> > int uffd;
> >
> > void *madvise_thread(void *arg) {
> >     for (size_t i = 0; i < REGION_SIZE; i += PAGE_SIZE) {
> >     madvise(src + i, PAGE_SIZE, MADV_PAGEOUT);
> >     usleep(100);
> >     }
> >     return NULL;
> > }
> >
> > void *swapin_thread(void *arg) {
> >     volatile char dummy;
> >     for (size_t i = 0; i < REGION_SIZE; i += PAGE_SIZE) {
> >         dummy = ((char *)src)[i];
> >         usleep(100);
> >     }
> >     return NULL;
> > }
> >
> >
> > void *fault_handler_thread(void *arg) {
> >
> >     struct uffd_msg msg;
> >     struct uffdio_move move;
> >     struct pollfd pollfd = { .fd = uffd, .events = POLLIN };
> >     pthread_setcancelstate(PTHREAD_CANCEL_ENABLE, NULL);
> >     pthread_setcanceltype(PTHREAD_CANCEL_DEFERRED, NULL);
> >
> >     while (1) {
> >         if (poll(&pollfd, 1, -1) == -1) {
> >             perror("poll");
> >             exit(EXIT_FAILURE);
> >         }
> >
> >         if (read(uffd, &msg, sizeof(msg)) <= 0) {
> >             perror("read");
> >             exit(EXIT_FAILURE);
> >         }
> >
> >
> >         if (msg.event != UFFD_EVENT_PAGEFAULT) {
> >             fprintf(stderr, "Unexpected event\n");
> >             exit(EXIT_FAILURE);
> >         }
> >
> >         move.src = (unsigned long)src + (msg.arg.pagefault.address -
> > (unsigned long)dst);
> >         move.dst = msg.arg.pagefault.address & ~(PAGE_SIZE - 1);
> >         move.len = PAGE_SIZE;
> >         move.mode = 0;
> >
> >         if (ioctl(uffd, UFFDIO_MOVE, &move) == -1) {
> >             perror("UFFDIO_MOVE");
> >             exit(EXIT_FAILURE);
> >         }
> >     }
> >     return NULL;
> > }
> >
> > int main() {
> > again:
> >     pthread_t thr, madv_thr, swapin_thr;
> >     struct uffdio_api uffdio_api = { .api = UFFD_API, .features = 0 };
> >     struct uffdio_register uffdio_register;
> >
> >     src = mmap(NULL, REGION_SIZE, PROT_READ | PROT_WRITE, MAP_PRIVATE
> > | MAP_ANONYMOUS, -1, 0);
> >
> >     if (src == MAP_FAILED) {
> >         perror("mmap src");
> >         exit(EXIT_FAILURE);
> >     }
> >
> >     memset(src, 1, REGION_SIZE);
> >
> >     dst = mmap(NULL, REGION_SIZE, PROT_READ | PROT_WRITE, MAP_PRIVATE
> > | MAP_ANONYMOUS, -1, 0);
> >
> >     if (dst == MAP_FAILED) {
> >         perror("mmap dst");
> >         exit(EXIT_FAILURE);
> >     }
> >
> >
> >     uffd = syscall(SYS_userfaultfd, O_CLOEXEC | O_NONBLOCK);
> >     if (uffd == -1) {
> >         perror("userfaultfd");
> >         exit(EXIT_FAILURE);
> >     }
> >
> >
> >     if (ioctl(uffd, UFFDIO_API, &uffdio_api) == -1) {
> >         perror("UFFDIO_API");
> >         exit(EXIT_FAILURE);
> >     }
> >
> >     uffdio_register.range.start = (unsigned long)dst;
> >     uffdio_register.range.len = REGION_SIZE;
> >     uffdio_register.mode = UFFDIO_REGISTER_MODE_MISSING;
> >
> >     if (ioctl(uffd, UFFDIO_REGISTER, &uffdio_register) == -1) {
> >         perror("UFFDIO_REGISTER");
> >         exit(EXIT_FAILURE);
> >
> >     }
> >
> >     if (pthread_create(&madv_thr, NULL, madvise_thread, NULL) != 0) {
> >         perror("pthread_create madvise_thread");
> >         exit(EXIT_FAILURE);
> >     }
> >
> >     if (pthread_create(&swapin_thr, NULL, swapin_thread, NULL) != 0) {
> >         perror("pthread_create swapin_thread");
> >         exit(EXIT_FAILURE);
> >     }
> >
> >     if (pthread_create(&thr, NULL, fault_handler_thread, NULL) != 0) {
> >         perror("pthread_create fault_handler_thread");
> >         exit(EXIT_FAILURE);
> >     }
> >
> >     for (size_t i = 0; i < REGION_SIZE; i += PAGE_SIZE) {
> >         char val = ((char *)dst)[i];
> >         printf("Accessing dst at offset %zu, value: %d\n", i, val);
> >     }
> >
> >     pthread_join(madv_thr, NULL);
> >     pthread_join(swapin_thr, NULL);
> >     pthread_cancel(thr);
> >     pthread_join(thr, NULL);
> >     munmap(src, REGION_SIZE);
> >     munmap(dst, REGION_SIZE);
> >     close(uffd);
> >     goto again;
> >
> >     return 0;
> > }
> >

Thanks
Barry