linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Lokesh Gidra <lokeshgidra@google.com>
To: Barry Song <21cnbao@gmail.com>
Cc: Peter Xu <peterx@redhat.com>,
	David Hildenbrand <david@redhat.com>,
	 Suren Baghdasaryan <surenb@google.com>,
	Andrea Arcangeli <aarcange@redhat.com>,
	 Andrew Morton <akpm@linux-foundation.org>,
	Linux-MM <linux-mm@kvack.org>,  Kairui Song <ryncsn@gmail.com>,
	LKML <linux-kernel@vger.kernel.org>
Subject: Re: [BUG]userfaultfd_move fails to move a folio when swap-in occurs concurrently with swap-out
Date: Thu, 22 May 2025 16:43:50 -0700	[thread overview]
Message-ID: <CA+EESO5BafvOpcbEX1g3ELPMCUdVQ9TFBGKH-sD3OiJvofMkAg@mail.gmail.com> (raw)
In-Reply-To: <CAGsJ_4zMNaNjRXbtDD6xYfDhckcDCnTvv+4-yB1xpuYcur=fyw@mail.gmail.com>

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?

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.
>
>
> [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


  reply	other threads:[~2025-05-22 23:44 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-05-22 23:23 Barry Song
2025-05-22 23:43 ` Lokesh Gidra [this message]
2025-05-22 23:53   ` Barry Song
2025-05-23  0:03     ` Lokesh Gidra
2025-05-26 12:39 ` David Hildenbrand
2025-05-27  4:17   ` Barry Song
2025-05-27  8:37     ` Barry Song
2025-05-27  9:00       ` David Hildenbrand
2025-05-27  9:31         ` Barry Song
2025-05-27 11:06           ` David Hildenbrand

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CA+EESO5BafvOpcbEX1g3ELPMCUdVQ9TFBGKH-sD3OiJvofMkAg@mail.gmail.com \
    --to=lokeshgidra@google.com \
    --cc=21cnbao@gmail.com \
    --cc=aarcange@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=david@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=peterx@redhat.com \
    --cc=ryncsn@gmail.com \
    --cc=surenb@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox