Received-SPF: pass (google.com: domain of linux-kernel+bounces-119267-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Precedence: bulk
MIME-Version: 1.0
References: <20240326143210.291116-1-david@redhat.com> <20240326143210.291116-3-david@redhat.com>
In-Reply-To: <20240326143210.291116-3-david@redhat.com>
From: Miklos Szeredi <mszeredi@redhat.com>
Date: Tue, 26 Mar 2024 16:10:11 +0100
Message-ID: <CAOssrKdKaYua48K5F2xD68cFCoiGZSxcYdqgVXrdYTocey2pTg@mail.gmail.com>
Subject: Re: [PATCH v2 2/3] selftests/memfd_secret: add vmsplice() test
To: David Hildenbrand <david@redhat.com>
Cc: linux-kernel@vger.kernel.org, linux-mm@kvack.org, 
	Andrew Morton <akpm@linux-foundation.org>, Mike Rapoport <rppt@kernel.org>, 
	Lorenzo Stoakes <lstoakes@gmail.com>, xingwei lee <xrivendell7@gmail.com>, 
	yue sun <samsun1006219@gmail.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Tue, Mar 26, 2024 at 3:32=E2=80=AFPM David Hildenbrand <david@redhat.com=
> wrote:
>
> Let's add a simple reproducer for a scenario where GUP-fast could succeed
> on secretmem folios, making vmsplice() succeed instead of failing. The
> reproducer is based on a reproducer [1] by Miklos Szeredi.
>
> We want to perform two tests: vmsplice() when a fresh page was just
> faulted in, and vmsplice() on an existing page after munmap() that
> would drain certain LRU caches/batches in the kernel.
>
> In an ideal world, we could use fallocate(FALLOC_FL_PUNCH_HOLE) /
> MADV_REMOVE to remove any existing page. As that is currently not
> possible, run the test before any other tests that would allocate memory
> in the secretmem fd.
>
> Perform the ftruncate() only once, and check the return value.
>
> [1] https://lkml.kernel.org/r/CAJfpegt3UCsMmxd0taOY11Uaw5U=3DeS1fE5dn0wZX=
3HF0oy8-oQ@mail.gmail.com
>
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>  tools/testing/selftests/mm/memfd_secret.c | 51 ++++++++++++++++++++++-
>  1 file changed, 49 insertions(+), 2 deletions(-)
>
> diff --git a/tools/testing/selftests/mm/memfd_secret.c b/tools/testing/se=
lftests/mm/memfd_secret.c
> index 9b298f6a04b3..9a0597310a76 100644
> --- a/tools/testing/selftests/mm/memfd_secret.c
> +++ b/tools/testing/selftests/mm/memfd_secret.c
> @@ -20,6 +20,7 @@
>  #include <unistd.h>
>  #include <errno.h>
>  #include <stdio.h>
> +#include <fcntl.h>
>
>  #include "../kselftest.h"
>
> @@ -83,6 +84,45 @@ static void test_mlock_limit(int fd)
>         pass("mlock limit is respected\n");
>  }
>
> +static void test_vmsplice(int fd, const char *desc)
> +{
> +       ssize_t transferred;
> +       struct iovec iov;
> +       int pipefd[2];
> +       char *mem;
> +
> +       if (pipe(pipefd)) {
> +               fail("pipe failed: %s\n", strerror(errno));
> +               return;
> +       }
> +
> +       mem =3D mmap(NULL, page_size, prot, mode, fd, 0);
> +       if (mem =3D=3D MAP_FAILED) {
> +               fail("Unable to mmap secret memory\n");
> +               goto close_pipe;
> +       }
> +
> +       /*
> +        * vmsplice() may use GUP-fast, which must also fail. Prefault th=
e
> +        * page table, so GUP-fast could find it.
> +        */
> +       memset(mem, PATTERN, page_size);

Shouldn't the non-prefault case be tested as well?

Thanks,
Miklos