diff mbox series

[v6,5/5] selftests/mm: add UFFDIO_MOVE ioctl test

Message ID 20231206103702.3873743-6-surenb@google.com (mailing list archive)
State Accepted
Commit a2bf6a9ca80532b75f8f8b6a1cd75ef7e5150576
Headers show
Series userfaultfd move option | expand

Commit Message

Suren Baghdasaryan Dec. 6, 2023, 10:36 a.m. UTC
Add tests for new UFFDIO_MOVE ioctl which uses uffd to move source
into destination buffer while checking the contents of both after
the move. After the operation the content of the destination buffer
should match the original source buffer's content while the source
buffer should be zeroed. Separate tests are designed for PMD aligned and
unaligned cases because they utilize different code paths in the kernel.

Signed-off-by: Suren Baghdasaryan <surenb@google.com>
---
 tools/testing/selftests/mm/uffd-common.c     |  24 +++
 tools/testing/selftests/mm/uffd-common.h     |   1 +
 tools/testing/selftests/mm/uffd-unit-tests.c | 189 +++++++++++++++++++
 3 files changed, 214 insertions(+)

Comments

Mark Brown Dec. 10, 2023, 2:23 p.m. UTC | #1
On Wed, Dec 06, 2023 at 02:36:59AM -0800, Suren Baghdasaryan wrote:
> Add tests for new UFFDIO_MOVE ioctl which uses uffd to move source
> into destination buffer while checking the contents of both after
> the move. After the operation the content of the destination buffer
> should match the original source buffer's content while the source
> buffer should be zeroed. Separate tests are designed for PMD aligned and
> unaligned cases because they utilize different code paths in the kernel.
> 
> Signed-off-by: Suren Baghdasaryan <surenb@google.com>
> ---
>  tools/testing/selftests/mm/uffd-common.c     |  24 +++
>  tools/testing/selftests/mm/uffd-common.h     |   1 +
>  tools/testing/selftests/mm/uffd-unit-tests.c | 189 +++++++++++++++++++
>  3 files changed, 214 insertions(+)

This breaks the build in at least some configurations with separate
output directories like those used by KernelCI:

make KBUILD_BUILD_USER=KernelCI FORMAT=.xz ARCH=arm64 HOSTCC=gcc CROSS_COMPILE=aarch64-linux-gnu- CROSS_COMPILE_COMPAT=arm-linux-gnueabihf- CC="ccache aarch64-linux-gnu-gcc" O=/tmp/kci/linux/build -C/tmp/kci/linux -j10 kselftest-gen_tar

(full logs for both arm64 and x86_64 at):

   https://storage.kernelci.org/next/master/next-20231208/arm64/defconfig/gcc-10/logs/kselftest.log
   https://storage.kernelci.org/next/master/next-20231208/x86_64/x86_64_defconfig/clang-17/logs/kselftest.log

or tuxmake:

make --silent --keep-going --jobs=16 O=/home/broonie/.cache/tuxmake/builds/25/build INSTALL_PATH=/home/broonie/.cache/tuxmake/builds/25/build/kselftest_install ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- CROSS_COMPILE_COMPAT=arm-linux-gnueabihf- kselftest-install

The specific failure:

aarch64-linux-gnu-gcc -Wall -I /tmp/kci/linux/tools/testing/selftests/../../..  -isystem /tmp/kci/linux/build/usr/include     uffd-stress.c vm_util.c uffd-common.c -lrt -lpthread -lm -o /tmp/kci/linux/build/kselftest/mm/uffd-stress
uffd-common.c: In function ‘move_page’:
uffd-common.c:636:21: error: storage size of ‘uffdio_move’ isn’t known
  636 |  struct uffdio_move uffdio_move;
      |                     ^~~~~~~~~~~
uffd-common.c:643:21: error: ‘UFFDIO_MOVE_MODE_ALLOW_SRC_HOLES’ undeclared (first use in this function)
  643 |  uffdio_move.mode = UFFDIO_MOVE_MODE_ALLOW_SRC_HOLES;
      |                     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
uffd-common.c:643:21: note: each undeclared identifier is reported only once for each function it appears in
uffd-common.c:645:17: error: ‘UFFDIO_MOVE’ undeclared (first use in this function); did you mean ‘UFFDIO_COPY’?
  645 |  if (ioctl(ufd, UFFDIO_MOVE, &uffdio_move)) {
      |                 ^~~~~~~~~~~
      |                 UFFDIO_COPY
uffd-common.c:636:21: warning: unused variable ‘uffdio_move’ [-Wunused-variable]
  636 |  struct uffdio_move uffdio_move;
      |                     ^~~~~~~~~~~
Suren Baghdasaryan Dec. 11, 2023, 1:01 a.m. UTC | #2
On Sun, Dec 10, 2023 at 6:26 AM Mark Brown <broonie@kernel.org> wrote:
>
> On Wed, Dec 06, 2023 at 02:36:59AM -0800, Suren Baghdasaryan wrote:
> > Add tests for new UFFDIO_MOVE ioctl which uses uffd to move source
> > into destination buffer while checking the contents of both after
> > the move. After the operation the content of the destination buffer
> > should match the original source buffer's content while the source
> > buffer should be zeroed. Separate tests are designed for PMD aligned and
> > unaligned cases because they utilize different code paths in the kernel.
> >
> > Signed-off-by: Suren Baghdasaryan <surenb@google.com>
> > ---
> >  tools/testing/selftests/mm/uffd-common.c     |  24 +++
> >  tools/testing/selftests/mm/uffd-common.h     |   1 +
> >  tools/testing/selftests/mm/uffd-unit-tests.c | 189 +++++++++++++++++++
> >  3 files changed, 214 insertions(+)
>
> This breaks the build in at least some configurations with separate
> output directories like those used by KernelCI:
>
> make KBUILD_BUILD_USER=KernelCI FORMAT=.xz ARCH=arm64 HOSTCC=gcc CROSS_COMPILE=aarch64-linux-gnu- CROSS_COMPILE_COMPAT=arm-linux-gnueabihf- CC="ccache aarch64-linux-gnu-gcc" O=/tmp/kci/linux/build -C/tmp/kci/linux -j10 kselftest-gen_tar
>
> (full logs for both arm64 and x86_64 at):
>
>    https://storage.kernelci.org/next/master/next-20231208/arm64/defconfig/gcc-10/logs/kselftest.log
>    https://storage.kernelci.org/next/master/next-20231208/x86_64/x86_64_defconfig/clang-17/logs/kselftest.log
>
> or tuxmake:
>
> make --silent --keep-going --jobs=16 O=/home/broonie/.cache/tuxmake/builds/25/build INSTALL_PATH=/home/broonie/.cache/tuxmake/builds/25/build/kselftest_install ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- CROSS_COMPILE_COMPAT=arm-linux-gnueabihf- kselftest-install
>
> The specific failure:
>
> aarch64-linux-gnu-gcc -Wall -I /tmp/kci/linux/tools/testing/selftests/../../..  -isystem /tmp/kci/linux/build/usr/include     uffd-stress.c vm_util.c uffd-common.c -lrt -lpthread -lm -o /tmp/kci/linux/build/kselftest/mm/uffd-stress
> uffd-common.c: In function ‘move_page’:
> uffd-common.c:636:21: error: storage size of ‘uffdio_move’ isn’t known
>   636 |  struct uffdio_move uffdio_move;
>       |                     ^~~~~~~~~~~
> uffd-common.c:643:21: error: ‘UFFDIO_MOVE_MODE_ALLOW_SRC_HOLES’ undeclared (first use in this function)
>   643 |  uffdio_move.mode = UFFDIO_MOVE_MODE_ALLOW_SRC_HOLES;
>       |                     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> uffd-common.c:643:21: note: each undeclared identifier is reported only once for each function it appears in
> uffd-common.c:645:17: error: ‘UFFDIO_MOVE’ undeclared (first use in this function); did you mean ‘UFFDIO_COPY’?
>   645 |  if (ioctl(ufd, UFFDIO_MOVE, &uffdio_move)) {
>       |                 ^~~~~~~~~~~
>       |                 UFFDIO_COPY
> uffd-common.c:636:21: warning: unused variable ‘uffdio_move’ [-Wunused-variable]
>   636 |  struct uffdio_move uffdio_move;
>       |                     ^~~~~~~~~~~

Thanks for reporting! I'll try that later today.
Just to clarify, are you using mm-unstable and if so, has it been
rebased since Friday? There was an update to this patchset in
mm-unstable which Andrew merged on Friday and the failure does look
like something that would happen with the previous version.
Suren Baghdasaryan Dec. 11, 2023, 3:04 a.m. UTC | #3
On Sun, Dec 10, 2023 at 5:01 PM Suren Baghdasaryan <surenb@google.com> wrote:
>
> On Sun, Dec 10, 2023 at 6:26 AM Mark Brown <broonie@kernel.org> wrote:
> >
> > On Wed, Dec 06, 2023 at 02:36:59AM -0800, Suren Baghdasaryan wrote:
> > > Add tests for new UFFDIO_MOVE ioctl which uses uffd to move source
> > > into destination buffer while checking the contents of both after
> > > the move. After the operation the content of the destination buffer
> > > should match the original source buffer's content while the source
> > > buffer should be zeroed. Separate tests are designed for PMD aligned and
> > > unaligned cases because they utilize different code paths in the kernel.
> > >
> > > Signed-off-by: Suren Baghdasaryan <surenb@google.com>
> > > ---
> > >  tools/testing/selftests/mm/uffd-common.c     |  24 +++
> > >  tools/testing/selftests/mm/uffd-common.h     |   1 +
> > >  tools/testing/selftests/mm/uffd-unit-tests.c | 189 +++++++++++++++++++
> > >  3 files changed, 214 insertions(+)
> >
> > This breaks the build in at least some configurations with separate
> > output directories like those used by KernelCI:
> >
> > make KBUILD_BUILD_USER=KernelCI FORMAT=.xz ARCH=arm64 HOSTCC=gcc CROSS_COMPILE=aarch64-linux-gnu- CROSS_COMPILE_COMPAT=arm-linux-gnueabihf- CC="ccache aarch64-linux-gnu-gcc" O=/tmp/kci/linux/build -C/tmp/kci/linux -j10 kselftest-gen_tar
> >
> > (full logs for both arm64 and x86_64 at):
> >
> >    https://storage.kernelci.org/next/master/next-20231208/arm64/defconfig/gcc-10/logs/kselftest.log
> >    https://storage.kernelci.org/next/master/next-20231208/x86_64/x86_64_defconfig/clang-17/logs/kselftest.log
> >
> > or tuxmake:
> >
> > make --silent --keep-going --jobs=16 O=/home/broonie/.cache/tuxmake/builds/25/build INSTALL_PATH=/home/broonie/.cache/tuxmake/builds/25/build/kselftest_install ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- CROSS_COMPILE_COMPAT=arm-linux-gnueabihf- kselftest-install
> >
> > The specific failure:
> >
> > aarch64-linux-gnu-gcc -Wall -I /tmp/kci/linux/tools/testing/selftests/../../..  -isystem /tmp/kci/linux/build/usr/include     uffd-stress.c vm_util.c uffd-common.c -lrt -lpthread -lm -o /tmp/kci/linux/build/kselftest/mm/uffd-stress
> > uffd-common.c: In function ‘move_page’:
> > uffd-common.c:636:21: error: storage size of ‘uffdio_move’ isn’t known
> >   636 |  struct uffdio_move uffdio_move;
> >       |                     ^~~~~~~~~~~
> > uffd-common.c:643:21: error: ‘UFFDIO_MOVE_MODE_ALLOW_SRC_HOLES’ undeclared (first use in this function)
> >   643 |  uffdio_move.mode = UFFDIO_MOVE_MODE_ALLOW_SRC_HOLES;
> >       |                     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > uffd-common.c:643:21: note: each undeclared identifier is reported only once for each function it appears in
> > uffd-common.c:645:17: error: ‘UFFDIO_MOVE’ undeclared (first use in this function); did you mean ‘UFFDIO_COPY’?
> >   645 |  if (ioctl(ufd, UFFDIO_MOVE, &uffdio_move)) {
> >       |                 ^~~~~~~~~~~
> >       |                 UFFDIO_COPY
> > uffd-common.c:636:21: warning: unused variable ‘uffdio_move’ [-Wunused-variable]
> >   636 |  struct uffdio_move uffdio_move;
> >       |                     ^~~~~~~~~~~
>
> Thanks for reporting! I'll try that later today.
> Just to clarify, are you using mm-unstable and if so, has it been
> rebased since Friday? There was an update to this patchset in
> mm-unstable which Andrew merged on Friday and the failure does look
> like something that would happen with the previous version.

I tried reproducing the issue but so far unsuccessfully. Could you
please confirm that on the latest mm-unstable branch it's still
reproducible and if so, please provide detailed instructions on how
you reproduce it.
Thanks,
Suren.
Mark Brown Dec. 11, 2023, 11:15 a.m. UTC | #4
On Sun, Dec 10, 2023 at 07:04:19PM -0800, Suren Baghdasaryan wrote:
> On Sun, Dec 10, 2023 at 5:01 PM Suren Baghdasaryan <surenb@google.com> wrote:

> > Thanks for reporting! I'll try that later today.
> > Just to clarify, are you using mm-unstable and if so, has it been
> > rebased since Friday? There was an update to this patchset in
> > mm-unstable which Andrew merged on Friday and the failure does look
> > like something that would happen with the previous version.

> I tried reproducing the issue but so far unsuccessfully. Could you
> please confirm that on the latest mm-unstable branch it's still
> reproducible and if so, please provide detailed instructions on how
> you reproduce it.

This is linux-next.  I pasted the commands used to build and sent links
to a full build log in the original report.
David Hildenbrand Dec. 11, 2023, 12:03 p.m. UTC | #5
On 11.12.23 12:15, Mark Brown wrote:
> On Sun, Dec 10, 2023 at 07:04:19PM -0800, Suren Baghdasaryan wrote:
>> On Sun, Dec 10, 2023 at 5:01 PM Suren Baghdasaryan <surenb@google.com> wrote:
> 
>>> Thanks for reporting! I'll try that later today.
>>> Just to clarify, are you using mm-unstable and if so, has it been
>>> rebased since Friday? There was an update to this patchset in
>>> mm-unstable which Andrew merged on Friday and the failure does look
>>> like something that would happen with the previous version.
> 
>> I tried reproducing the issue but so far unsuccessfully. Could you
>> please confirm that on the latest mm-unstable branch it's still
>> reproducible and if so, please provide detailed instructions on how
>> you reproduce it.
> 
> This is linux-next.  I pasted the commands used to build and sent links
> to a full build log in the original report.

Probably also related to "make headers-install":

https://lkml.kernel.org/r/20231209020144.244759-1-jhubbard@nvidia.com

The general problem is that some mm selftests are currently not written 
in way that allows them to compile with old linux headers. That's why 
the build fails if "make headers-install" was not executed, but it does 
not fail if "make headers-install" was once upon a time executed, but 
the headers are outdated.
Mark Brown Dec. 11, 2023, 12:24 p.m. UTC | #6
On Mon, Dec 11, 2023 at 01:03:27PM +0100, David Hildenbrand wrote:
> On 11.12.23 12:15, Mark Brown wrote:

> > This is linux-next.  I pasted the commands used to build and sent links
> > to a full build log in the original report.

> Probably also related to "make headers-install":

> https://lkml.kernel.org/r/20231209020144.244759-1-jhubbard@nvidia.com

> The general problem is that some mm selftests are currently not written in
> way that allows them to compile with old linux headers. That's why the build
> fails if "make headers-install" was not executed, but it does not fail if
> "make headers-install" was once upon a time executed, but the headers are
> outdated.

Oh, it's obviously the new headers not being installed.  The builds
where I'm seeing the problem (my own and KernelCI's) are all fresh
containers so there shouldn't be any stale headers lying around.
Suren Baghdasaryan Dec. 11, 2023, 4:15 p.m. UTC | #7
On Mon, Dec 11, 2023 at 4:24 AM Mark Brown <broonie@kernel.org> wrote:
>
> On Mon, Dec 11, 2023 at 01:03:27PM +0100, David Hildenbrand wrote:
> > On 11.12.23 12:15, Mark Brown wrote:
>
> > > This is linux-next.  I pasted the commands used to build and sent links
> > > to a full build log in the original report.
>
> > Probably also related to "make headers-install":
>
> > https://lkml.kernel.org/r/20231209020144.244759-1-jhubbard@nvidia.com
>
> > The general problem is that some mm selftests are currently not written in
> > way that allows them to compile with old linux headers. That's why the build
> > fails if "make headers-install" was not executed, but it does not fail if
> > "make headers-install" was once upon a time executed, but the headers are
> > outdated.
>
> Oh, it's obviously the new headers not being installed.  The builds
> where I'm seeing the problem (my own and KernelCI's) are all fresh
> containers so there shouldn't be any stale headers lying around.

Ok, I was updating my headers and that's why I could not reproduce it.
David, should the test be modified to handle old linux headers
(disable the new tests #ifndef _UFFDIO_MOVE or some other way)?
Mark Brown Dec. 11, 2023, 4:25 p.m. UTC | #8
On Mon, Dec 11, 2023 at 08:15:11AM -0800, Suren Baghdasaryan wrote:
> On Mon, Dec 11, 2023 at 4:24 AM Mark Brown <broonie@kernel.org> wrote:

> > Oh, it's obviously the new headers not being installed.  The builds
> > where I'm seeing the problem (my own and KernelCI's) are all fresh
> > containers so there shouldn't be any stale headers lying around.

> Ok, I was updating my headers and that's why I could not reproduce it.
> David, should the test be modified to handle old linux headers
> (disable the new tests #ifndef _UFFDIO_MOVE or some other way)?

Are you sure we're not just missing an updated to the list of headers to
copy (under include/uapi IIRC)?
Suren Baghdasaryan Dec. 11, 2023, 4:29 p.m. UTC | #9
On Mon, Dec 11, 2023 at 8:25 AM Mark Brown <broonie@kernel.org> wrote:
>
> On Mon, Dec 11, 2023 at 08:15:11AM -0800, Suren Baghdasaryan wrote:
> > On Mon, Dec 11, 2023 at 4:24 AM Mark Brown <broonie@kernel.org> wrote:
>
> > > Oh, it's obviously the new headers not being installed.  The builds
> > > where I'm seeing the problem (my own and KernelCI's) are all fresh
> > > containers so there shouldn't be any stale headers lying around.
>
> > Ok, I was updating my headers and that's why I could not reproduce it.
> > David, should the test be modified to handle old linux headers
> > (disable the new tests #ifndef _UFFDIO_MOVE or some other way)?
>
> Are you sure we're not just missing an updated to the list of headers to
> copy (under include/uapi IIRC)?

Let me double check.

Just to rule out this possibility, linux-next was broken on Friday
(see https://lore.kernel.org/all/CAJuCfpFiEqRO4qkFZbUCmGZy-n_ucqgR5NeyvnwXqYh+RU4C6w@mail.gmail.com/).
I just checked and it's fixed now. Could you confirm that with the
latest linux-next you still see the issue?
David Hildenbrand Dec. 11, 2023, 4:32 p.m. UTC | #10
On 11.12.23 17:15, Suren Baghdasaryan wrote:
> On Mon, Dec 11, 2023 at 4:24 AM Mark Brown <broonie@kernel.org> wrote:
>>
>> On Mon, Dec 11, 2023 at 01:03:27PM +0100, David Hildenbrand wrote:
>>> On 11.12.23 12:15, Mark Brown wrote:
>>
>>>> This is linux-next.  I pasted the commands used to build and sent links
>>>> to a full build log in the original report.
>>
>>> Probably also related to "make headers-install":
>>
>>> https://lkml.kernel.org/r/20231209020144.244759-1-jhubbard@nvidia.com
>>
>>> The general problem is that some mm selftests are currently not written in
>>> way that allows them to compile with old linux headers. That's why the build
>>> fails if "make headers-install" was not executed, but it does not fail if
>>> "make headers-install" was once upon a time executed, but the headers are
>>> outdated.
>>
>> Oh, it's obviously the new headers not being installed.  The builds
>> where I'm seeing the problem (my own and KernelCI's) are all fresh
>> containers so there shouldn't be any stale headers lying around.
> 
> Ok, I was updating my headers and that's why I could not reproduce it.
> David, should the test be modified to handle old linux headers
> (disable the new tests #ifndef _UFFDIO_MOVE or some other way)?

That's an open question: do we want to be able to build selftests 
against any host headers, and not the in-tree headers that have to be 
manually installed and dirty the git tree?

One obvious drawbacks is that we'll have to deal with all that using a 
bunch of #ifdef, and the tests that will be built+run will depend on the 
host headers.

Especially the letter is relevant I think: Our upstream testing won't be 
able to build+run tests that rely on new upstream features. But that's 
what some key benefit of these selftests, and being able to run them 
automatically on a bunch of different combinations upstream.

Further, the tests are closely related to the given kernel version, they 
are not some completely separate tests.


Moving the the (MM?) selftests to a separate repository would make the 
decision easier: just like in QEMU etc, we'd simply pull in a headers 
update and only build against these archived headers.

So I see the options:

(1) Rely on installing the proper in-tree headers. Build will fail if
     that is not happening.

(2) Make the tests build with any host headers.

(3) Regularly archive the required headers in the selftest directory
     like external projects like QEMU do.


(3) avoids dirtying the tree as a "make headers_install" would, but it 
also means that each test that makes use of new uapi has to update the 
relevant headers (what people working on QEMU are used to).
Mark Brown Dec. 11, 2023, 4:34 p.m. UTC | #11
On Mon, Dec 11, 2023 at 08:29:06AM -0800, Suren Baghdasaryan wrote:

> Just to rule out this possibility, linux-next was broken on Friday
> (see https://lore.kernel.org/all/CAJuCfpFiEqRO4qkFZbUCmGZy-n_ucqgR5NeyvnwXqYh+RU4C6w@mail.gmail.com/).
> I just checked and it's fixed now. Could you confirm that with the
> latest linux-next you still see the issue?

Yes, it looks like it's fixed today - thanks!  (The fix was getting
masked by the ongoing KVM breakage.)
Mark Brown Dec. 11, 2023, 4:41 p.m. UTC | #12
On Mon, Dec 11, 2023 at 05:32:16PM +0100, David Hildenbrand wrote:
> On 11.12.23 17:15, Suren Baghdasaryan wrote:

> > Ok, I was updating my headers and that's why I could not reproduce it.
> > David, should the test be modified to handle old linux headers
> > (disable the new tests #ifndef _UFFDIO_MOVE or some other way)?

> That's an open question: do we want to be able to build selftests against
> any host headers, and not the in-tree headers that have to be manually
> installed and dirty the git tree?

Quite a lot of existing selftests rely on the headers being installed to
build...

> One obvious drawbacks is that we'll have to deal with all that using a bunch
> of #ifdef, and the tests that will be built+run will depend on the host
> headers.

> Especially the letter is relevant I think: Our upstream testing won't be
> able to build+run tests that rely on new upstream features. But that's what
> some key benefit of these selftests, and being able to run them
> automatically on a bunch of different combinations upstream.

...for exactly this reason.  It causes real pain testing new interfaces.

> Further, the tests are closely related to the given kernel version, they are
> not some completely separate tests.

Note that there's a general desire for the tests to *run* with older
kernels and use whatever feature test mechanisms exist to skip tests
that won't run.  That's often needed anyway for configurable things.

> (3) avoids dirtying the tree as a "make headers_install" would, but it also
> means that each test that makes use of new uapi has to update the relevant
> headers (what people working on QEMU are used to).

Note that you can do an out of tree build to avoid dirtying things.
Suren Baghdasaryan Dec. 11, 2023, 4:43 p.m. UTC | #13
On Mon, Dec 11, 2023 at 8:34 AM Mark Brown <broonie@kernel.org> wrote:
>
> On Mon, Dec 11, 2023 at 08:29:06AM -0800, Suren Baghdasaryan wrote:
>
> > Just to rule out this possibility, linux-next was broken on Friday
> > (see https://lore.kernel.org/all/CAJuCfpFiEqRO4qkFZbUCmGZy-n_ucqgR5NeyvnwXqYh+RU4C6w@mail.gmail.com/).
> > I just checked and it's fixed now. Could you confirm that with the
> > latest linux-next you still see the issue?
>
> Yes, it looks like it's fixed today - thanks!  (The fix was getting
> masked by the ongoing KVM breakage.)

Very good. Thanks for confirming!
David Hildenbrand Dec. 11, 2023, 4:53 p.m. UTC | #14
>> Further, the tests are closely related to the given kernel version, they are
>> not some completely separate tests.
> 
> Note that there's a general desire for the tests to *run* with older
> kernels and use whatever feature test mechanisms exist to skip tests
> that won't run.  That's often needed anyway for configurable things.

Agreed.

> 
>> (3) avoids dirtying the tree as a "make headers_install" would, but it also
>> means that each test that makes use of new uapi has to update the relevant
>> headers (what people working on QEMU are used to).
> 
> Note that you can do an out of tree build to avoid dirtying things.

Yes, but apparently the simple "make headers_install" will dirty the kernel.

See (and ideally comment on)

https://lkml.kernel.org/r/20231209020144.244759-1-jhubbard@nvidia.com
Mark Brown Dec. 11, 2023, 5:32 p.m. UTC | #15
On Mon, Dec 11, 2023 at 05:53:59PM +0100, David Hildenbrand wrote:

> > > (3) avoids dirtying the tree as a "make headers_install" would, but it also
> > > means that each test that makes use of new uapi has to update the relevant
> > > headers (what people working on QEMU are used to).

> > Note that you can do an out of tree build to avoid dirtying things.

> Yes, but apparently the simple "make headers_install" will dirty the kernel.

> See (and ideally comment on)

> https://lkml.kernel.org/r/20231209020144.244759-1-jhubbard@nvidia.com

I mean, I guess people who don't want to install the headers are just
not going to be able to build a bunch of tests?  There definitely are a
bunch of tests where it's not needed so I can see why people would not
like being forced to do the headers step if they're only interested in
those tests.
David Hildenbrand Dec. 11, 2023, 6 p.m. UTC | #16
On 11.12.23 18:32, Mark Brown wrote:
> On Mon, Dec 11, 2023 at 05:53:59PM +0100, David Hildenbrand wrote:
> 
>>>> (3) avoids dirtying the tree as a "make headers_install" would, but it also
>>>> means that each test that makes use of new uapi has to update the relevant
>>>> headers (what people working on QEMU are used to).
> 
>>> Note that you can do an out of tree build to avoid dirtying things.
> 
>> Yes, but apparently the simple "make headers_install" will dirty the kernel.
> 
>> See (and ideally comment on)
> 
>> https://lkml.kernel.org/r/20231209020144.244759-1-jhubbard@nvidia.com
> 
> I mean, I guess people who don't want to install the headers are just
> not going to be able to build a bunch of tests?  There definitely are a
> bunch of tests where it's not needed so I can see why people would not
> like being forced to do the headers step if they're only interested in
> those tests.

Yes. And before that, people mostly had no clue that headers had to be 
installed in order to compile successfully.

So maybe a warning to give at least some hint might be reasonable.
John Hubbard Dec. 11, 2023, 6:46 p.m. UTC | #17
On 12/11/23 08:32, David Hildenbrand wrote:
...
> That's an open question: do we want to be able to build selftests 
> against any host headers, and not the in-tree headers that have to be 
> manually installed and dirty the git tree?
> 
> One obvious drawbacks is that we'll have to deal with all that using a 
> bunch of #ifdef, and the tests that will be built+run will depend on the 
> host headers.
> 
> Especially the letter is relevant I think: Our upstream testing won't be 
> able to build+run tests that rely on new upstream features. But that's 
> what some key benefit of these selftests, and being able to run them 
> automatically on a bunch of different combinations upstream.
> 
> Further, the tests are closely related to the given kernel version, they 
> are not some completely separate tests.
> 
> 
> Moving the the (MM?) selftests to a separate repository would make the 
> decision easier: just like in QEMU etc, we'd simply pull in a headers 
> update and only build against these archived headers.
> 
> So I see the options:
> 
> (1) Rely on installing the proper in-tree headers. Build will fail if
>      that is not happening.
> 
> (2) Make the tests build with any host headers.
> 
> (3) Regularly archive the required headers in the selftest directory
>      like external projects like QEMU do.

Or (4) Hack in little ifdef snippets, into the selftests, like we used
to do. Peter Zijlstra seems to be asking for this, if I understand his
(much) earlier comments about this.



thanks,
Mark Brown Dec. 11, 2023, 8:01 p.m. UTC | #18
On Mon, Dec 11, 2023 at 07:00:32PM +0100, David Hildenbrand wrote:
> On 11.12.23 18:32, Mark Brown wrote:
> > On Mon, Dec 11, 2023 at 05:53:59PM +0100, David Hildenbrand wrote:

> > > https://lkml.kernel.org/r/20231209020144.244759-1-jhubbard@nvidia.com

> > I mean, I guess people who don't want to install the headers are just
> > not going to be able to build a bunch of tests?  There definitely are a
> > bunch of tests where it's not needed so I can see why people would not
> > like being forced to do the headers step if they're only interested in
> > those tests.

> Yes. And before that, people mostly had no clue that headers had to be
> installed in order to compile successfully.

> So maybe a warning to give at least some hint might be reasonable.

That sounds sensible, especially if we could arrange to flag when the
specific tests being built need it.
John Hubbard Dec. 11, 2023, 8:11 p.m. UTC | #19
On 12/11/23 12:01, Mark Brown wrote:
> On Mon, Dec 11, 2023 at 07:00:32PM +0100, David Hildenbrand wrote:
>> On 11.12.23 18:32, Mark Brown wrote:
>>> On Mon, Dec 11, 2023 at 05:53:59PM +0100, David Hildenbrand wrote:
> 
>>>> https://lkml.kernel.org/r/20231209020144.244759-1-jhubbard@nvidia.com
> 
>>> I mean, I guess people who don't want to install the headers are just
>>> not going to be able to build a bunch of tests?  There definitely are a
>>> bunch of tests where it's not needed so I can see why people would not
>>> like being forced to do the headers step if they're only interested in
>>> those tests.
> 
>> Yes. And before that, people mostly had no clue that headers had to be
>> installed in order to compile successfully.
> 
>> So maybe a warning to give at least some hint might be reasonable.
> 
> That sounds sensible, especially if we could arrange to flag when the
> specific tests being built need it.


But the end result is messy: not everything builds in some cases. If
instead we went back to the little ifdef snippets, such as this (from
v5.1):

hugepage-shm.c:

     #ifndef SHM_HUGETLB
     #define SHM_HUGETLB 04000
     #endif

...then with a bit of one-time, manual effort, we could get everything
to work at all times. And that seems better, doesn't it?


thanks,
Mark Brown Dec. 11, 2023, 8:21 p.m. UTC | #20
On Mon, Dec 11, 2023 at 10:46:23AM -0800, John Hubbard wrote:

> Or (4) Hack in little ifdef snippets, into the selftests, like we used
> to do. Peter Zijlstra seems to be asking for this, if I understand his
> (much) earlier comments about this.

I can't help but think that if we're having to manually copy bits of
the uapi headers (which are already separated out in the source) into
another part of the same source tree in order to use them then there's
room for improvement somewhere.  TBH it also doesn't seem great to add
additional variables that depend on the user's build environment, we
already have enough build issues.  It ought to be mostly tedious rather
than hard but it's still a pain, especially given the issues we have
getting kselftest fixes merged promptly.
John Hubbard Dec. 11, 2023, 8:29 p.m. UTC | #21
On 12/11/23 12:21, Mark Brown wrote:
> On Mon, Dec 11, 2023 at 10:46:23AM -0800, John Hubbard wrote:
> 
>> Or (4) Hack in little ifdef snippets, into the selftests, like we used
>> to do. Peter Zijlstra seems to be asking for this, if I understand his
>> (much) earlier comments about this.
> 
> I can't help but think that if we're having to manually copy bits of
> the uapi headers (which are already separated out in the source) into
> another part of the same source tree in order to use them then there's
> room for improvement somewhere.  TBH it also doesn't seem great to add

Yes, it feels that way to me, too.

> additional variables that depend on the user's build environment, we
> already have enough build issues.  It ought to be mostly tedious rather
> than hard but it's still a pain, especially given the issues we have
> getting kselftest fixes merged promptly.

What about David's option (3):

(3) Regularly archive the required headers in the selftest directory
     like external projects like QEMU do.

, combined with something in the build system to connect it up for
building the selftests?

Or maybe there is an innovative way to do all of this, that we have
yet to think of.


thanks,
Mark Brown Dec. 12, 2023, 3:12 p.m. UTC | #22
On Mon, Dec 11, 2023 at 12:29:58PM -0800, John Hubbard wrote:
> On 12/11/23 12:21, Mark Brown wrote:

> > additional variables that depend on the user's build environment, we
> > already have enough build issues.  It ought to be mostly tedious rather
> > than hard but it's still a pain, especially given the issues we have
> > getting kselftest fixes merged promptly.

> What about David's option (3):

> (3) Regularly archive the required headers in the selftest directory
>     like external projects like QEMU do.

> , combined with something in the build system to connect it up for
> building the selftests?

> Or maybe there is an innovative way to do all of this, that we have
> yet to think of.

We do copy files into tools/include at random times which makes sense
for things that aren't uapi, and we are putting bits of uapi there
already so we could just expand the set of files copied there.  AFAICT
the only reason we're copying the uapi files at all is that they're
directly in the same include/ directories as everything else and are
always referenced with their uapi/ prefix.
David Hildenbrand Dec. 12, 2023, 3:27 p.m. UTC | #23
On 11.12.23 21:11, John Hubbard wrote:
> On 12/11/23 12:01, Mark Brown wrote:
>> On Mon, Dec 11, 2023 at 07:00:32PM +0100, David Hildenbrand wrote:
>>> On 11.12.23 18:32, Mark Brown wrote:
>>>> On Mon, Dec 11, 2023 at 05:53:59PM +0100, David Hildenbrand wrote:
>>
>>>>> https://lkml.kernel.org/r/20231209020144.244759-1-jhubbard@nvidia.com
>>
>>>> I mean, I guess people who don't want to install the headers are just
>>>> not going to be able to build a bunch of tests?  There definitely are a
>>>> bunch of tests where it's not needed so I can see why people would not
>>>> like being forced to do the headers step if they're only interested in
>>>> those tests.
>>
>>> Yes. And before that, people mostly had no clue that headers had to be
>>> installed in order to compile successfully.
>>
>>> So maybe a warning to give at least some hint might be reasonable.
>>
>> That sounds sensible, especially if we could arrange to flag when the
>> specific tests being built need it.
> 
> 
> But the end result is messy: not everything builds in some cases. If
> instead we went back to the little ifdef snippets, such as this (from
> v5.1):
> 
> hugepage-shm.c:
> 
>       #ifndef SHM_HUGETLB
>       #define SHM_HUGETLB 04000
>       #endif
> 
> ...then with a bit of one-time, manual effort, we could get everything
> to work at all times. And that seems better, doesn't it?

I'm not a fan of fixing up host headers on a case-per-case basis using 
ifdefs. It makes the tests harder to read, write and maintain.

We do have the proper headers in the tree, just not in an consumable way 
for the tests.

Ideally, we'd either carry our own "consumable" version in the tree, or 
are able to convert the headers under the hood and place them in a 
directory where we won't have to dirty the tree -- and only tests that 
need these headers (e.g., mm selftests) will perform that conversion and 
include them.

I usually build my stuff in-tree, so I don't really have a lot of 
experience with out-of-tree selftest builds and the whole kernel header 
inclusion (and how we could avoid the "make headers" and place the 
headers somewhere else).
Mark Brown Dec. 12, 2023, 7:39 p.m. UTC | #24
On Tue, Dec 12, 2023 at 04:27:12PM +0100, David Hildenbrand wrote:

> I usually build my stuff in-tree, so I don't really have a lot of experience
> with out-of-tree selftest builds and the whole kernel header inclusion (and
> how we could avoid the "make headers" and place the headers somewhere else).

It's generally something along the lines of (from tuxmake):

   make --silent --keep-going --jobs=15 O=/build/stage/build-work INSTALL_PATH=/build/stage/build-work/kselftest_install ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- CROSS_COMPILE_COMPAT=arm-linux-gnueabihf- kselftest-install

possibly with a -C in there to find the kernel source (from KernelCI):

   make KBUILD_BUILD_USER=KernelCI FORMAT=.xz ARCH=arm64 HOSTCC=gcc CROSS_COMPILE=aarch64-linux-gnu- CROSS_COMPILE_COMPAT=arm-linux-gnueabihf- CC="ccache aarch64-linux-gnu-gcc" O=/tmp/kci/linux/build -C/tmp/kci/linux -j10 kselftest-gen_tar
John Hubbard Dec. 13, 2023, 2:14 a.m. UTC | #25
On 12/12/23 07:12, Mark Brown wrote:
> On Mon, Dec 11, 2023 at 12:29:58PM -0800, John Hubbard wrote:
>> On 12/11/23 12:21, Mark Brown wrote:
...
>> Or maybe there is an innovative way to do all of this, that we have
>> yet to think of.
> 
> We do copy files into tools/include at random times which makes sense
> for things that aren't uapi, and we are putting bits of uapi there
> already so we could just expand the set of files copied there.  AFAICT
> the only reason we're copying the uapi files at all is that they're
> directly in the same include/ directories as everything else and are
> always referenced with their uapi/ prefix.

Oh, this sounds like it would work nicely. No more "make headers"
required (hooray!). Instead, the new approach would be "selftests are
allowed to include from tools/include", and then we can just start
copying the files that we need to that location, and gradually fix up
all the selftests.

I really like this, at first reading anyway.

Muhammad, Shuah, others, what do you think?

+Cc: Muhammad Usama Anjum <usama.anjum@collabora.com>


thanks,
Muhammad Usama Anjum Dec. 13, 2023, 3:58 a.m. UTC | #26
On 12/13/23 7:14 AM, John Hubbard wrote:
> On 12/12/23 07:12, Mark Brown wrote:
>> On Mon, Dec 11, 2023 at 12:29:58PM -0800, John Hubbard wrote:
>>> On 12/11/23 12:21, Mark Brown wrote:
> ...
>>> Or maybe there is an innovative way to do all of this, that we have
>>> yet to think of.
>>
>> We do copy files into tools/include at random times which makes sense
>> for things that aren't uapi, and we are putting bits of uapi there
>> already so we could just expand the set of files copied there.  AFAICT
>> the only reason we're copying the uapi files at all is that they're
>> directly in the same include/ directories as everything else and are
>> always referenced with their uapi/ prefix.
> 
> Oh, this sounds like it would work nicely. No more "make headers"
> required (hooray!). Instead, the new approach would be "selftests are
> allowed to include from tools/include", and then we can just start
> copying the files that we need to that location, and gradually fix up
> all the selftests.
No, this wouldn't work.
* The selftests are applications which include default header files. The
application don't care from where the header files are picked up at compile
time. We should be able to build the application on normal system with
latest headers installed without any changes.
* The header files cannot be included directly as they need to be processed
first which is done by `make headers`. Here is a diff between kernel fs.h
and processed header file to be used by applications:

--- include/uapi/linux/fs.h	2023-12-12 14:45:22.857409660 +0500
+++ usr/include/linux/fs.h	2023-12-12 14:49:23.469733573 +0500
@@ -1,6 +1,6 @@
 /* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
-#ifndef _UAPI_LINUX_FS_H
-#define _UAPI_LINUX_FS_H
+#ifndef _LINUX_FS_H
+#define _LINUX_FS_H

 /*
  * This file has definitions for some important file table structures
@@ -13,14 +13,10 @@
 #include <linux/limits.h>
 #include <linux/ioctl.h>
 #include <linux/types.h>
-#ifndef __KERNEL__
 #include <linux/fscrypt.h>
-#endif

 /* Use of MS_* flags within the kernel is restricted to core mount(2) code. */
-#if !defined(__KERNEL__)
 #include <linux/mount.h>
-#endif

 /*
  * It's silly to have NR_OPEN bigger than NR_FILE, but you can change
@@ -287,19 +283,19 @@
 typedef int __bitwise __kernel_rwf_t;

 /* high priority request, poll if possible */
-#define RWF_HIPRI	((__force __kernel_rwf_t)0x00000001)
+#define RWF_HIPRI	((__kernel_rwf_t)0x00000001)

 /* per-IO O_DSYNC */
-#define RWF_DSYNC	((__force __kernel_rwf_t)0x00000002)
+#define RWF_DSYNC	((__kernel_rwf_t)0x00000002)

 /* per-IO O_SYNC */
-#define RWF_SYNC	((__force __kernel_rwf_t)0x00000004)
+#define RWF_SYNC	((__kernel_rwf_t)0x00000004)

 /* per-IO, return -EAGAIN if operation would block */
-#define RWF_NOWAIT	((__force __kernel_rwf_t)0x00000008)
+#define RWF_NOWAIT	((__kernel_rwf_t)0x00000008)

 /* per-IO O_APPEND */
-#define RWF_APPEND	((__force __kernel_rwf_t)0x00000010)
+#define RWF_APPEND	((__kernel_rwf_t)0x00000010)

 /* mask of flags supported by the kernel */
 #define RWF_SUPPORTED	(RWF_HIPRI | RWF_DSYNC | RWF_SYNC | RWF_NOWAIT |\
@@ -364,4 +360,4 @@
 	__u64 return_mask;
 };

-#endif /* _UAPI_LINUX_FS_H */
+#endif /* _LINUX_FS_H */

> 
> I really like this, at first reading anyway.
> 
> Muhammad, Shuah, others, what do you think?
> 
> +Cc: Muhammad Usama Anjum <usama.anjum@collabora.com>
> 
> 
> thanks,
John Hubbard Dec. 13, 2023, 5:52 a.m. UTC | #27
On 12/12/23 19:58, Muhammad Usama Anjum wrote:
>> ...
>> Oh, this sounds like it would work nicely. No more "make headers"
>> required (hooray!). Instead, the new approach would be "selftests are
>> allowed to include from tools/include", and then we can just start
>> copying the files that we need to that location, and gradually fix up
>> all the selftests.
> No, this wouldn't work.
> * The selftests are applications which include default header files. The
> application don't care from where the header files are picked up at compile
> time. We should be able to build the application on normal system with
> latest headers installed without any changes.
> * The header files cannot be included directly as they need to be processed
> first which is done by `make headers`. Here is a diff between kernel fs.h
> and processed header file to be used by applications:

Well, that's not the proposal. The idea is to snapshot various uapi/ headers
into tools/include, just like what is already being done:

$ diff ./include/uapi/linux/fs.h ./tools/include/uapi/linux/fs.h
$


thanks,
John Hubbard Dec. 13, 2023, 5:55 a.m. UTC | #28
On 12/12/23 21:52, John Hubbard wrote:
> On 12/12/23 19:58, Muhammad Usama Anjum wrote:
>>> ...
>>> Oh, this sounds like it would work nicely. No more "make headers"
>>> required (hooray!). Instead, the new approach would be "selftests are
>>> allowed to include from tools/include", and then we can just start
>>> copying the files that we need to that location, and gradually fix up
>>> all the selftests.
>> No, this wouldn't work.
>> * The selftests are applications which include default header files. The
>> application don't care from where the header files are picked up at 
>> compile
>> time. We should be able to build the application on normal system with
>> latest headers installed without any changes.
>> * The header files cannot be included directly as they need to be 
>> processed
>> first which is done by `make headers`. Here is a diff between kernel fs.h
>> and processed header file to be used by applications:
> 
> Well, that's not the proposal. The idea is to snapshot various uapi/ 
> headers
> into tools/include, just like what is already being done:
> 
> $ diff ./include/uapi/linux/fs.h ./tools/include/uapi/linux/fs.h
> $

Oh sorry, that's exactly what you were saying you don't want. ahem. :)

Another variation though, would be to run "make headers", and snapshot
some of those files into tools/include.


thanks,
David Hildenbrand Dec. 13, 2023, 9:59 a.m. UTC | #29
On 13.12.23 06:55, John Hubbard wrote:
> On 12/12/23 21:52, John Hubbard wrote:
>> On 12/12/23 19:58, Muhammad Usama Anjum wrote:
>>>> ...
>>>> Oh, this sounds like it would work nicely. No more "make headers"
>>>> required (hooray!). Instead, the new approach would be "selftests are
>>>> allowed to include from tools/include", and then we can just start
>>>> copying the files that we need to that location, and gradually fix up
>>>> all the selftests.
>>> No, this wouldn't work.
>>> * The selftests are applications which include default header files. The
>>> application don't care from where the header files are picked up at
>>> compile
>>> time. We should be able to build the application on normal system with
>>> latest headers installed without any changes.
>>> * The header files cannot be included directly as they need to be
>>> processed
>>> first which is done by `make headers`. Here is a diff between kernel fs.h
>>> and processed header file to be used by applications:
>>
>> Well, that's not the proposal. The idea is to snapshot various uapi/
>> headers
>> into tools/include, just like what is already being done:
>>
>> $ diff ./include/uapi/linux/fs.h ./tools/include/uapi/linux/fs.h
>> $
> 
> Oh sorry, that's exactly what you were saying you don't want. ahem. :)
> 
> Another variation though, would be to run "make headers", and snapshot
> some of those files into tools/include.

^ this is what I had in mind

If you're writing a test that needs some new fancy thing, update the 
relevant header.
Mark Brown Dec. 13, 2023, 2 p.m. UTC | #30
On Wed, Dec 13, 2023 at 08:58:06AM +0500, Muhammad Usama Anjum wrote:
> On 12/13/23 7:14 AM, John Hubbard wrote:

> > Oh, this sounds like it would work nicely. No more "make headers"
> > required (hooray!). Instead, the new approach would be "selftests are
> > allowed to include from tools/include", and then we can just start
> > copying the files that we need to that location, and gradually fix up
> > all the selftests.

> No, this wouldn't work.

Note that we have a bunch of selftests (at least arm64, hid, kvm, rseq
and sgx from a quick grep) which already use and rely on the headers in
tools/include.

> * The selftests are applications which include default header files. The
> application don't care from where the header files are picked up at compile
> time. We should be able to build the application on normal system with
> latest headers installed without any changes.

I think there is much less interest in building out of the kernel than
there is in avoiding having to handle random userspace headers...

> * The header files cannot be included directly as they need to be processed
> first which is done by `make headers`. Here is a diff between kernel fs.h
> and processed header file to be used by applications:

I guess that's another reason why the sync is done manually.  There are
also a bunch of files in tools/include that are just completely
different implementations of things (not just uapi).
John Hubbard Dec. 13, 2023, 10:01 p.m. UTC | #31
On 12/13/23 01:59, David Hildenbrand wrote:
...
>> Another variation though, would be to run "make headers", and snapshot
>> some of those files into tools/include.
> 
> ^ this is what I had in mind
> 
> If you're writing a test that needs some new fancy thing, update the relevant header.
> 

OK, and Mark Brown's nearby response [1] supports that, as well.

Thus fortified, I plan on doing the following steps:

Step 1: Do nothing about the revert patch that I sent earlier, thus
allowing it to continue in its journey (so far, Andrew has moved it into
mm-hotfixes-stable branch).

Step 2: Send out a patch for a modest part of selftests/mm, that uses
this latest approach, and see how it fares in reviews.

[1] https://lore.kernel.org/c0aa00a2-38a5-42da-9951-64131d936f7e@sirena.org.uk


thanks,
David Hildenbrand Dec. 14, 2023, 9:02 a.m. UTC | #32
On 13.12.23 23:01, John Hubbard wrote:
> On 12/13/23 01:59, David Hildenbrand wrote:
> ...
>>> Another variation though, would be to run "make headers", and snapshot
>>> some of those files into tools/include.
>>
>> ^ this is what I had in mind
>>
>> If you're writing a test that needs some new fancy thing, update the relevant header.
>>
> 
> OK, and Mark Brown's nearby response [1] supports that, as well.
> 
> Thus fortified, I plan on doing the following steps:
> 
> Step 1: Do nothing about the revert patch that I sent earlier, thus
> allowing it to continue in its journey (so far, Andrew has moved it into
> mm-hotfixes-stable branch).
> 
> Step 2: Send out a patch for a modest part of selftests/mm, that uses
> this latest approach, and see how it fares in reviews.

All sounds good to me!
diff mbox series

Patch

diff --git a/tools/testing/selftests/mm/uffd-common.c b/tools/testing/selftests/mm/uffd-common.c
index fb3bbc77fd00..b0ac0ec2356d 100644
--- a/tools/testing/selftests/mm/uffd-common.c
+++ b/tools/testing/selftests/mm/uffd-common.c
@@ -631,6 +631,30 @@  int copy_page(int ufd, unsigned long offset, bool wp)
 	return __copy_page(ufd, offset, false, wp);
 }
 
+int move_page(int ufd, unsigned long offset, unsigned long len)
+{
+	struct uffdio_move uffdio_move;
+
+	if (offset + len > nr_pages * page_size)
+		err("unexpected offset %lu and length %lu\n", offset, len);
+	uffdio_move.dst = (unsigned long) area_dst + offset;
+	uffdio_move.src = (unsigned long) area_src + offset;
+	uffdio_move.len = len;
+	uffdio_move.mode = UFFDIO_MOVE_MODE_ALLOW_SRC_HOLES;
+	uffdio_move.move = 0;
+	if (ioctl(ufd, UFFDIO_MOVE, &uffdio_move)) {
+		/* real retval in uffdio_move.move */
+		if (uffdio_move.move != -EEXIST)
+			err("UFFDIO_MOVE error: %"PRId64,
+			    (int64_t)uffdio_move.move);
+		wake_range(ufd, uffdio_move.dst, len);
+	} else if (uffdio_move.move != len) {
+		err("UFFDIO_MOVE error: %"PRId64, (int64_t)uffdio_move.move);
+	} else
+		return 1;
+	return 0;
+}
+
 int uffd_open_dev(unsigned int flags)
 {
 	int fd, uffd;
diff --git a/tools/testing/selftests/mm/uffd-common.h b/tools/testing/selftests/mm/uffd-common.h
index 774595ee629e..cb055282c89c 100644
--- a/tools/testing/selftests/mm/uffd-common.h
+++ b/tools/testing/selftests/mm/uffd-common.h
@@ -119,6 +119,7 @@  void wp_range(int ufd, __u64 start, __u64 len, bool wp);
 void uffd_handle_page_fault(struct uffd_msg *msg, struct uffd_args *args);
 int __copy_page(int ufd, unsigned long offset, bool retry, bool wp);
 int copy_page(int ufd, unsigned long offset, bool wp);
+int move_page(int ufd, unsigned long offset, unsigned long len);
 void *uffd_poll_thread(void *arg);
 
 int uffd_open_dev(unsigned int flags);
diff --git a/tools/testing/selftests/mm/uffd-unit-tests.c b/tools/testing/selftests/mm/uffd-unit-tests.c
index debc423bdbf4..d8091523c2df 100644
--- a/tools/testing/selftests/mm/uffd-unit-tests.c
+++ b/tools/testing/selftests/mm/uffd-unit-tests.c
@@ -23,6 +23,9 @@ 
 #define  MEM_ALL  (MEM_ANON | MEM_SHMEM | MEM_SHMEM_PRIVATE | \
 		   MEM_HUGETLB | MEM_HUGETLB_PRIVATE)
 
+#define ALIGN_UP(x, align_to) \
+	((__typeof__(x))((((unsigned long)(x)) + ((align_to)-1)) & ~((align_to)-1)))
+
 struct mem_type {
 	const char *name;
 	unsigned int mem_flag;
@@ -1064,6 +1067,178 @@  static void uffd_poison_test(uffd_test_args_t *targs)
 	uffd_test_pass();
 }
 
+static void
+uffd_move_handle_fault_common(struct uffd_msg *msg, struct uffd_args *args,
+			      unsigned long len)
+{
+	unsigned long offset;
+
+	if (msg->event != UFFD_EVENT_PAGEFAULT)
+		err("unexpected msg event %u", msg->event);
+
+	if (msg->arg.pagefault.flags &
+	    (UFFD_PAGEFAULT_FLAG_WP | UFFD_PAGEFAULT_FLAG_MINOR | UFFD_PAGEFAULT_FLAG_WRITE))
+		err("unexpected fault type %llu", msg->arg.pagefault.flags);
+
+	offset = (char *)(unsigned long)msg->arg.pagefault.address - area_dst;
+	offset &= ~(len-1);
+
+	if (move_page(uffd, offset, len))
+		args->missing_faults++;
+}
+
+static void uffd_move_handle_fault(struct uffd_msg *msg,
+				   struct uffd_args *args)
+{
+	uffd_move_handle_fault_common(msg, args, page_size);
+}
+
+static void uffd_move_pmd_handle_fault(struct uffd_msg *msg,
+				       struct uffd_args *args)
+{
+	uffd_move_handle_fault_common(msg, args, read_pmd_pagesize());
+}
+
+static void
+uffd_move_test_common(uffd_test_args_t *targs, unsigned long chunk_size,
+		      void (*handle_fault)(struct uffd_msg *msg, struct uffd_args *args))
+{
+	unsigned long nr;
+	pthread_t uffd_mon;
+	char c;
+	unsigned long long count;
+	struct uffd_args args = { 0 };
+	char *orig_area_src, *orig_area_dst;
+	unsigned long step_size, step_count;
+	unsigned long src_offs = 0;
+	unsigned long dst_offs = 0;
+
+	/* Prevent source pages from being mapped more than once */
+	if (madvise(area_src, nr_pages * page_size, MADV_DONTFORK))
+		err("madvise(MADV_DONTFORK) failure");
+
+	if (uffd_register(uffd, area_dst, nr_pages * page_size,
+			  true, false, false))
+		err("register failure");
+
+	args.handle_fault = handle_fault;
+	if (pthread_create(&uffd_mon, NULL, uffd_poll_thread, &args))
+		err("uffd_poll_thread create");
+
+	step_size = chunk_size / page_size;
+	step_count = nr_pages / step_size;
+
+	if (chunk_size > page_size) {
+		char *aligned_src = ALIGN_UP(area_src, chunk_size);
+		char *aligned_dst = ALIGN_UP(area_dst, chunk_size);
+
+		if (aligned_src != area_src || aligned_dst != area_dst) {
+			src_offs = (aligned_src - area_src) / page_size;
+			dst_offs = (aligned_dst - area_dst) / page_size;
+			step_count--;
+		}
+		orig_area_src = area_src;
+		orig_area_dst = area_dst;
+		area_src = aligned_src;
+		area_dst = aligned_dst;
+	}
+
+	/*
+	 * Read each of the pages back using the UFFD-registered mapping. We
+	 * expect that the first time we touch a page, it will result in a missing
+	 * fault. uffd_poll_thread will resolve the fault by moving source
+	 * page to destination.
+	 */
+	for (nr = 0; nr < step_count * step_size; nr += step_size) {
+		unsigned long i;
+
+		/* Check area_src content */
+		for (i = 0; i < step_size; i++) {
+			count = *area_count(area_src, nr + i);
+			if (count != count_verify[src_offs + nr + i])
+				err("nr %lu source memory invalid %llu %llu\n",
+				    nr + i, count, count_verify[src_offs + nr + i]);
+		}
+
+		/* Faulting into area_dst should move the page or the huge page */
+		for (i = 0; i < step_size; i++) {
+			count = *area_count(area_dst, nr + i);
+			if (count != count_verify[dst_offs + nr + i])
+				err("nr %lu memory corruption %llu %llu\n",
+				    nr, count, count_verify[dst_offs + nr + i]);
+		}
+
+		/* Re-check area_src content which should be empty */
+		for (i = 0; i < step_size; i++) {
+			count = *area_count(area_src, nr + i);
+			if (count != 0)
+				err("nr %lu move failed %llu %llu\n",
+				    nr, count, count_verify[src_offs + nr + i]);
+		}
+	}
+	if (step_size > page_size) {
+		area_src = orig_area_src;
+		area_dst = orig_area_dst;
+	}
+
+	if (write(pipefd[1], &c, sizeof(c)) != sizeof(c))
+		err("pipe write");
+	if (pthread_join(uffd_mon, NULL))
+		err("join() failed");
+
+	if (args.missing_faults != step_count || args.minor_faults != 0)
+		uffd_test_fail("stats check error");
+	else
+		uffd_test_pass();
+}
+
+static void uffd_move_test(uffd_test_args_t *targs)
+{
+	uffd_move_test_common(targs, page_size, uffd_move_handle_fault);
+}
+
+static void uffd_move_pmd_test(uffd_test_args_t *targs)
+{
+	uffd_move_test_common(targs, read_pmd_pagesize(),
+			      uffd_move_pmd_handle_fault);
+}
+
+static int prevent_hugepages(const char **errmsg)
+{
+	/* This should be done before source area is populated */
+	if (madvise(area_src, nr_pages * page_size, MADV_NOHUGEPAGE)) {
+		/* Ignore only if CONFIG_TRANSPARENT_HUGEPAGE=n */
+		if (errno != EINVAL) {
+			if (errmsg)
+				*errmsg = "madvise(MADV_NOHUGEPAGE) failed";
+			return -errno;
+		}
+	}
+	return 0;
+}
+
+static int request_hugepages(const char **errmsg)
+{
+	/* This should be done before source area is populated */
+	if (madvise(area_src, nr_pages * page_size, MADV_HUGEPAGE)) {
+		if (errmsg) {
+			*errmsg = (errno == EINVAL) ?
+				"CONFIG_TRANSPARENT_HUGEPAGE is not set" :
+				"madvise(MADV_HUGEPAGE) failed";
+		}
+		return -errno;
+	}
+	return 0;
+}
+
+struct uffd_test_case_ops uffd_move_test_case_ops = {
+	.post_alloc = prevent_hugepages,
+};
+
+struct uffd_test_case_ops uffd_move_test_pmd_case_ops = {
+	.post_alloc = request_hugepages,
+};
+
 /*
  * Test the returned uffdio_register.ioctls with different register modes.
  * Note that _UFFDIO_ZEROPAGE is tested separately in the zeropage test.
@@ -1141,6 +1316,20 @@  uffd_test_case_t uffd_tests[] = {
 		.mem_targets = MEM_ALL,
 		.uffd_feature_required = 0,
 	},
+	{
+		.name = "move",
+		.uffd_fn = uffd_move_test,
+		.mem_targets = MEM_ANON,
+		.uffd_feature_required = UFFD_FEATURE_MOVE,
+		.test_case_ops = &uffd_move_test_case_ops,
+	},
+	{
+		.name = "move-pmd",
+		.uffd_fn = uffd_move_pmd_test,
+		.mem_targets = MEM_ANON,
+		.uffd_feature_required = UFFD_FEATURE_MOVE,
+		.test_case_ops = &uffd_move_test_pmd_case_ops,
+	},
 	{
 		.name = "wp-fork",
 		.uffd_fn = uffd_wp_fork_test,