| Message ID | 20240608021023.176027-1-jhubbard@nvidia.com |
|---|---|
| Headers | show |
| Series | cleanups, fixes, and progress towards avoiding "make headers" | expand |
On 6/10/24 9:45 PM, Jeff Xu wrote: > On Mon, Jun 10, 2024 at 9:34 PM John Hubbard <jhubbard@nvidia.com> wrote: >> On 6/10/24 9:21 PM, Jeff Xu wrote: >>> Hi >>> >>> >>> On Fri, Jun 7, 2024 at 7:10 PM John Hubbard <jhubbard@nvidia.com> wrote: >>>> >>>> Eventually, once the build succeeds on a sufficiently old distro, the >>>> idea is to delete $(KHDR_INCLUDES) from the selftests/mm build, and then >>>> after that, from selftests/lib.mk and all of the other selftest builds. >>>> >>>> For now, this series merely achieves a clean build of selftests/mm on a >>>> not-so-old distro: Ubuntu 23.04: >>>> >>>> 1. Add __NR_mseal. >>>> >>>> 2. Add fs.h, taken as usual from a snapshot of ./usr/include/linux/fs.h >>>> after running "make headers". This is how we have agreed to do this sort >>>> of thing, see [1]. >>>> >>> What is the "official" way to build selftests/mm ? >> >> From Documentation/dev-tools/kselftest.rst, it is: >> >> $ make headers >> $ make -C tools/testing/selftests >> >>> I tried a few ways, but it never worked, i.e. due to head missing. >> >> You are correct. Today's rules require "make headers" first. But >> I'm working on getting rid of that requirement, because it causes >> problems for some people and situations. >> >> (Even worse is the follow-up rule, in today's documentation, >> that tells us to *run* the selftests from within Make! This >> is just madness. > > That is hilarious! :) :) > >> Because the tests need to run as root in >> many cases. And Make will try to rebuild if necessary...thus >> filling your tree full of root-owned files...but that's for >> another time.) >> >>> >>> 1> >>> cd tools/testing/selftests/mm >>> make >>> >>> migration.c:10:10: fatal error: numa.h: No such file or directory >>> 10 | #include <numa.h> >>> | ^~~~~~~~ >>> compilation terminated. >>> >>> 2> >>> make headers >>> make -C tools/testing/selftests >>> >>> make[1]: Entering directory >>> '/usr/local/google/home/jeffxu/mm/tools/testing/selftests/mm' >>> CC migration >>> migration.c:10:10: fatal error: numa.h: No such file or directory >>> 10 | #include <numa.h> >>> >> >> Well, actually, for these, one should install libnuma-dev and >> numactl (those are Ubuntu package names. Arch Linux would be: >> numactl). >> >> I think. The idea is: use system headers if they are there, and >> local kernel tree header files if the items are so new that they >> haven't made it to $OLDEST_DISTO_REASONABLE. >> >> Something like that. >> > But I don't want to install random packages if possible. Well...keep in mind that it's not really random. If a test program requires numa.h, it's typically because it also links against libnuma, which *must* be supplied by the distro if you want to build. Because it doesn't come with the kernel, of course. So what you're really saying is that you'd like to build and run whatever you can at the moment--the build should soldier on past failures as much as possible. > > Can makefile rule continue to the next target in case of failure though ? That could be done, in general. The question is if that's really what we want, or should want. Maybe... > right now it stopped at migration.c , if it continues to the next target, then > I don't need to use gcc to manually build mseal_test. Let me take a peek at it in the morning. thanks,
On 11.06.24 08:25, John Hubbard wrote: > On 6/10/24 9:45 PM, Jeff Xu wrote: >> On Mon, Jun 10, 2024 at 9:34 PM John Hubbard <jhubbard@nvidia.com> wrote: >>> On 6/10/24 9:21 PM, Jeff Xu wrote: >>>> Hi >>>> >>>> >>>> On Fri, Jun 7, 2024 at 7:10 PM John Hubbard <jhubbard@nvidia.com> wrote: >>>>> >>>>> Eventually, once the build succeeds on a sufficiently old distro, the >>>>> idea is to delete $(KHDR_INCLUDES) from the selftests/mm build, and then >>>>> after that, from selftests/lib.mk and all of the other selftest builds. >>>>> >>>>> For now, this series merely achieves a clean build of selftests/mm on a >>>>> not-so-old distro: Ubuntu 23.04: >>>>> >>>>> 1. Add __NR_mseal. >>>>> >>>>> 2. Add fs.h, taken as usual from a snapshot of ./usr/include/linux/fs.h >>>>> after running "make headers". This is how we have agreed to do this sort >>>>> of thing, see [1]. >>>>> >>>> What is the "official" way to build selftests/mm ? >>> >>> From Documentation/dev-tools/kselftest.rst, it is: >>> >>> $ make headers >>> $ make -C tools/testing/selftests >>> >>>> I tried a few ways, but it never worked, i.e. due to head missing. >>> >>> You are correct. Today's rules require "make headers" first. But >>> I'm working on getting rid of that requirement, because it causes >>> problems for some people and situations. >>> >>> (Even worse is the follow-up rule, in today's documentation, >>> that tells us to *run* the selftests from within Make! This >>> is just madness. >> >> That is hilarious! :) > > :) > >> >>> Because the tests need to run as root in >>> many cases. And Make will try to rebuild if necessary...thus >>> filling your tree full of root-owned files...but that's for >>> another time.) >>> >>>> >>>> 1> >>>> cd tools/testing/selftests/mm >>>> make >>>> >>>> migration.c:10:10: fatal error: numa.h: No such file or directory >>>> 10 | #include <numa.h> >>>> | ^~~~~~~~ >>>> compilation terminated. >>>> >>>> 2> >>>> make headers >>>> make -C tools/testing/selftests >>>> >>>> make[1]: Entering directory >>>> '/usr/local/google/home/jeffxu/mm/tools/testing/selftests/mm' >>>> CC migration >>>> migration.c:10:10: fatal error: numa.h: No such file or directory >>>> 10 | #include <numa.h> >>>> >>> >>> Well, actually, for these, one should install libnuma-dev and >>> numactl (those are Ubuntu package names. Arch Linux would be: >>> numactl). >>> >>> I think. The idea is: use system headers if they are there, and >>> local kernel tree header files if the items are so new that they >>> haven't made it to $OLDEST_DISTO_REASONABLE. >>> >>> Something like that. >>> >> But I don't want to install random packages if possible. > > Well...keep in mind that it's not really random. If a test program > requires numa.h, it's typically because it also links against libnuma, > which *must* be supplied by the distro if you want to build. Because > it doesn't come with the kernel, of course. > > So what you're really saying is that you'd like to build and run > whatever you can at the moment--the build should soldier on past > failures as much as possible. > >> >> Can makefile rule continue to the next target in case of failure though ? > > That could be done, in general. The question is if that's really what > we want, or should want. Maybe... In cow.c, we warn if liburing is not around and build the test without these test cases. check_config.sh senses support. We could do the same for numactl (numa.h), but maybe there would not be any test case to run in there without libnuma (did not check). Some tests also require lcap.
On 08.06.24 04:10, John Hubbard wrote: > Eventually, once the build succeeds on a sufficiently old distro, the > idea is to delete $(KHDR_INCLUDES) from the selftests/mm build, and then > after that, from selftests/lib.mk and all of the other selftest builds. > > For now, this series merely achieves a clean build of selftests/mm on a > not-so-old distro: Ubuntu 23.04: Wasn't the plan to rely on the tools/include headers, and pull in there whatever we need? > > 1. Add __NR_mseal. > For example, making sure that tools/include/uapi/asm-generic/unistd.h is updated to contain __NR_mseal? ... to avoid hand-crafted defines we have to maintain for selftests. But maybe I am remembering something outdated.
On Mon, Jun 10, 2024 at 11:26 PM John Hubbard <jhubbard@nvidia.com> wrote: > > On 6/10/24 9:45 PM, Jeff Xu wrote: > > On Mon, Jun 10, 2024 at 9:34 PM John Hubbard <jhubbard@nvidia.com> wrote: > >> On 6/10/24 9:21 PM, Jeff Xu wrote: > >>> Hi > >>> > >>> > >>> On Fri, Jun 7, 2024 at 7:10 PM John Hubbard <jhubbard@nvidia.com> wrote: > >>>> > >>>> Eventually, once the build succeeds on a sufficiently old distro, the > >>>> idea is to delete $(KHDR_INCLUDES) from the selftests/mm build, and then > >>>> after that, from selftests/lib.mk and all of the other selftest builds. > >>>> > >>>> For now, this series merely achieves a clean build of selftests/mm on a > >>>> not-so-old distro: Ubuntu 23.04: > >>>> > >>>> 1. Add __NR_mseal. > >>>> > >>>> 2. Add fs.h, taken as usual from a snapshot of ./usr/include/linux/fs.h > >>>> after running "make headers". This is how we have agreed to do this sort > >>>> of thing, see [1]. > >>>> > >>> What is the "official" way to build selftests/mm ? > >> > >> From Documentation/dev-tools/kselftest.rst, it is: > >> > >> $ make headers > >> $ make -C tools/testing/selftests > >> > >>> I tried a few ways, but it never worked, i.e. due to head missing. > >> > >> You are correct. Today's rules require "make headers" first. But > >> I'm working on getting rid of that requirement, because it causes > >> problems for some people and situations. > >> > >> (Even worse is the follow-up rule, in today's documentation, > >> that tells us to *run* the selftests from within Make! This > >> is just madness. > > > > That is hilarious! :) > > :) > > > > >> Because the tests need to run as root in > >> many cases. And Make will try to rebuild if necessary...thus > >> filling your tree full of root-owned files...but that's for > >> another time.) > >> > >>> > >>> 1> > >>> cd tools/testing/selftests/mm > >>> make > >>> > >>> migration.c:10:10: fatal error: numa.h: No such file or directory > >>> 10 | #include <numa.h> > >>> | ^~~~~~~~ > >>> compilation terminated. > >>> > >>> 2> > >>> make headers > >>> make -C tools/testing/selftests > >>> > >>> make[1]: Entering directory > >>> '/usr/local/google/home/jeffxu/mm/tools/testing/selftests/mm' > >>> CC migration > >>> migration.c:10:10: fatal error: numa.h: No such file or directory > >>> 10 | #include <numa.h> > >>> > >> > >> Well, actually, for these, one should install libnuma-dev and > >> numactl (those are Ubuntu package names. Arch Linux would be: > >> numactl). > >> > >> I think. The idea is: use system headers if they are there, and > >> local kernel tree header files if the items are so new that they > >> haven't made it to $OLDEST_DISTO_REASONABLE. > >> > >> Something like that. > >> > > But I don't want to install random packages if possible. > > Well...keep in mind that it's not really random. If a test program > requires numa.h, it's typically because it also links against libnuma, > which *must* be supplied by the distro if you want to build. Because > it doesn't come with the kernel, of course. > Agreed. > So what you're really saying is that you'd like to build and run > whatever you can at the moment--the build should soldier on past > failures as much as possible. > Yes. That is what I meant. It would be a convenient feature. > > > > Can makefile rule continue to the next target in case of failure though ? > > That could be done, in general. The question is if that's really what > we want, or should want. Maybe... > > > right now it stopped at migration.c , if it continues to the next target, then > > I don't need to use gcc to manually build mseal_test. > > Let me take a peek at it in the morning. > > > > thanks, > -- > John Hubbard > NVIDIA >
On 6/11/24 2:36 AM, David Hildenbrand wrote: > On 08.06.24 04:10, John Hubbard wrote: >> Eventually, once the build succeeds on a sufficiently old distro, the >> idea is to delete $(KHDR_INCLUDES) from the selftests/mm build, and then >> after that, from selftests/lib.mk and all of the other selftest builds. >> >> For now, this series merely achieves a clean build of selftests/mm on a >> not-so-old distro: Ubuntu 23.04: > > Wasn't the plan to rely on the tools/include headers, and pull in there whatever we need? Yes, it is. You are correct. > >> >> 1. Add __NR_mseal. >> > > For example, making sure that tools/include/uapi/asm-generic/unistd.h is updated to contain __NR_mseal? Well, here it gets less clear cut, because the selftests pull in *lots* of system headers. In this case /usr/include/unistd.h gets pulled in. If we force tools/include/uapi/asm-generic/unistd.h to be included, then we'll get many many warnings of redefinitions of __NR_* items. So what's really going on here is that we have this uneasy mix of system headers from the test machine, and newer versions of some of those headers in the kernel tree. And some of those are easier to combine with system headers, than others. unistd.h is clearly not going quietly, which is why, I believe, the "#ifndef __NR_* " approach has flowered in the selftests. > > ... to avoid hand-crafted defines we have to maintain for selftests. > > But maybe I am remembering something outdated. > You remembered correctly, but the situation is slighly muddier than one would prefer. :) thanks,
On 11.06.24 22:54, John Hubbard wrote: > On 6/11/24 2:36 AM, David Hildenbrand wrote: >> On 08.06.24 04:10, John Hubbard wrote: >>> Eventually, once the build succeeds on a sufficiently old distro, the >>> idea is to delete $(KHDR_INCLUDES) from the selftests/mm build, and then >>> after that, from selftests/lib.mk and all of the other selftest builds. >>> >>> For now, this series merely achieves a clean build of selftests/mm on a >>> not-so-old distro: Ubuntu 23.04: >> >> Wasn't the plan to rely on the tools/include headers, and pull in there whatever we need? > > Yes, it is. You are correct. > >> >>> >>> 1. Add __NR_mseal. >>> >> >> For example, making sure that tools/include/uapi/asm-generic/unistd.h is updated to contain __NR_mseal? > > Well, here it gets less clear cut, because the selftests pull in *lots* of > system headers. In this case /usr/include/unistd.h gets pulled in. If we > force tools/include/uapi/asm-generic/unistd.h to be included, then we'll > get many many warnings of redefinitions of __NR_* items. I think, there is a difference between unistd.h and linux/unistd.h. We want to continue including unistd.h from the distro, but might want to stop including the linux one from the distro. My thinking was that we start maintaining our own linux headers copy in-tree, and start converting our tests from including <linux/> supplied by the distro to include the in-tree ones. For mseal_test.c, that might mean stopping including "linux/mman.h", and instead including the in-tree one. > > So what's really going on here is that we have this uneasy mix of system > headers from the test machine, and newer versions of some of those headers > in the kernel tree. And some of those are easier to combine with system > headers, than others. unistd.h is clearly not going quietly, which is > why, I believe, the "#ifndef __NR_* " approach has flowered in the > selftests. Right, these mixtures are not what we want I think. But I have no idea how easy it would be to convert individual tests. Maybe all it takes is updating the in-tree headers and then including "TBD/linux/whatever.h" instead of <linux/whatever.h> In QEMU, we maintain some (not all) kernel headers ourselves, and include them via "standard-headers/linux/whatever.h" > >> >> ... to avoid hand-crafted defines we have to maintain for selftests. >> >> But maybe I am remembering something outdated. >> > > You remembered correctly, but the situation is slighly muddier than > one would prefer. :) Absolutely, and I appreciate that you are trying to improve the situation.
On 6/12/24 1:24 AM, David Hildenbrand wrote: > On 11.06.24 22:54, John Hubbard wrote: >> On 6/11/24 2:36 AM, David Hildenbrand wrote: >>> On 08.06.24 04:10, John Hubbard wrote: >>>> Eventually, once the build succeeds on a sufficiently old distro, the >>>> idea is to delete $(KHDR_INCLUDES) from the selftests/mm build, and then >>>> after that, from selftests/lib.mk and all of the other selftest builds. >>>> >>>> For now, this series merely achieves a clean build of selftests/mm on a >>>> not-so-old distro: Ubuntu 23.04: >>> >>> Wasn't the plan to rely on the tools/include headers, and pull in there whatever we need? >> >> Yes, it is. You are correct. >> >>> >>>> >>>> 1. Add __NR_mseal. >>>> >>> >>> For example, making sure that tools/include/uapi/asm-generic/unistd.h is updated to contain __NR_mseal? >> >> Well, here it gets less clear cut, because the selftests pull in *lots* of >> system headers. In this case /usr/include/unistd.h gets pulled in. If we >> force tools/include/uapi/asm-generic/unistd.h to be included, then we'll >> get many many warnings of redefinitions of __NR_* items. > > I think, there is a difference between unistd.h and linux/unistd.h. We want to continue including unistd.h from the distro, but might want to stop including the linux one from the distro. > > My thinking was that we start maintaining our own linux headers copy in-tree, and start converting our tests from including <linux/> supplied by the distro to include the in-tree ones. > > For mseal_test.c, that might mean stopping including "linux/mman.h", and instead including the in-tree one. Yes. Something like that. $ find /usr -name 'unistd*.h' | wc -l 14 $ find /kernel_work/linux-github/ -name 'unistd*.h' | wc -l 54 heh. :) > >> >> So what's really going on here is that we have this uneasy mix of system >> headers from the test machine, and newer versions of some of those headers >> in the kernel tree. And some of those are easier to combine with system >> headers, than others. unistd.h is clearly not going quietly, which is >> why, I believe, the "#ifndef __NR_* " approach has flowered in the >> selftests. > > Right, these mixtures are not what we want I think. But I have no idea how easy it would be to convert individual tests. > > Maybe all it takes is updating the in-tree headers and then including "TBD/linux/whatever.h" instead of <linux/whatever.h> > > In QEMU, we maintain some (not all) kernel headers ourselves, and include them via > > "standard-headers/linux/whatever.h" Let me look into it. Maybe it's fairly simple, we shall see. > >> >>> >>> ... to avoid hand-crafted defines we have to maintain for selftests. >>> >>> But maybe I am remembering something outdated. >>> >> >> You remembered correctly, but the situation is slighly muddier than >> one would prefer. :) > > > Absolutely, and I appreciate that you are trying to improve the situation. > I think the attempts to further tease apart the include headers could go into a separate, subsequent series, yes? And let this one go in unmolested for now? thanks,
On 6/12/24 7:11 PM, John Hubbard wrote: > On 6/12/24 1:24 AM, David Hildenbrand wrote: >> On 11.06.24 22:54, John Hubbard wrote: >>> On 6/11/24 2:36 AM, David Hildenbrand wrote: >>>> On 08.06.24 04:10, John Hubbard wrote: ... >>> You remembered correctly, but the situation is slighly muddier than >>> one would prefer. :) >> >> >> Absolutely, and I appreciate that you are trying to improve the situation. >> > > I think the attempts to further tease apart the include headers could > go into a separate, subsequent series, yes? And let this one go in > unmolested for now? On second thought, it is actually much easier than I thought, let me post a v2 with the unistd.h header fixes, after all. thanks,
On 13.06.24 23:27, John Hubbard wrote: > On 6/12/24 7:11 PM, John Hubbard wrote: >> On 6/12/24 1:24 AM, David Hildenbrand wrote: >>> On 11.06.24 22:54, John Hubbard wrote: >>>> On 6/11/24 2:36 AM, David Hildenbrand wrote: >>>>> On 08.06.24 04:10, John Hubbard wrote: > ... >>>> You remembered correctly, but the situation is slighly muddier than >>>> one would prefer. :) >>> >>> >>> Absolutely, and I appreciate that you are trying to improve the situation. >>> >> >> I think the attempts to further tease apart the include headers could >> go into a separate, subsequent series, yes? And let this one go in >> unmolested for now? > > > On second thought, it is actually much easier than I thought, let me > post a v2 with the unistd.h header fixes, after all. Great! :)
Eventually, once the build succeeds on a sufficiently old distro, the idea is to delete $(KHDR_INCLUDES) from the selftests/mm build, and then after that, from selftests/lib.mk and all of the other selftest builds. For now, this series merely achieves a clean build of selftests/mm on a not-so-old distro: Ubuntu 23.04: 1. Add __NR_mseal. 2. Add fs.h, taken as usual from a snapshot of ./usr/include/linux/fs.h after running "make headers". This is how we have agreed to do this sort of thing, see [1]. 3. Add a few selected prctl.h values that the ksm and mdwe tests require. [1] commit e076eaca5906 ("selftests: break the dependency upon local header files") John Hubbard (5): selftests/mm: mseal, self_elf: fix missing __NR_mseal selftests/mm: fix vm_util.c build failures: add snapshot of fs.h mm/selftests: kvm, mdwe fixes to avoid requiring "make headers" selftests/mm: mseal, self_elf: factor out test macros and other duplicated items selftests/mm: mseal, self_elf: rename TEST_END_CHECK to REPORT_TEST_PASS tools/include/uapi/linux/fs.h | 392 +++++++++++++++++++++ tools/testing/selftests/mm/mdwe_test.c | 1 + tools/testing/selftests/mm/mseal_helpers.h | 45 +++ tools/testing/selftests/mm/mseal_test.c | 141 +++----- tools/testing/selftests/mm/seal_elf.c | 35 +- tools/testing/selftests/mm/vm_util.h | 15 + 6 files changed, 502 insertions(+), 127 deletions(-) create mode 100644 tools/include/uapi/linux/fs.h create mode 100644 tools/testing/selftests/mm/mseal_helpers.h base-commit: 8a92980606e3585d72d510a03b59906e96755b8a