Message ID | 20231206103702.3873743-6-surenb@google.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | userfaultfd move option | expand |
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; | ^~~~~~~~~~~
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.
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.
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.
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.
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.
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)?
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)?
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?
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).
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.)
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.
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!
>> 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
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.
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.
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,
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.
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,
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.
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,
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.
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).
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
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,
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,
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,
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,
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.
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).
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,
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 --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,
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(+)