| Message ID | 20200928062159.923212-1-jhubbard@nvidia.com |
|---|---|
| Headers | show |
| Series | selftests/vm: gup_test, hmm-tests, assorted improvements | expand |
On Sun, Sep 27, 2020 at 11:21:53PM -0700, John Hubbard wrote: > diff --git a/tools/testing/selftests/vm/Makefile b/tools/testing/selftests/vm/Makefile > index d1ae706d9927..9cc6bc087461 100644 > +++ b/tools/testing/selftests/vm/Makefile > @@ -130,3 +130,5 @@ endif > $(OUTPUT)/userfaultfd: LDLIBS += -lpthread > > $(OUTPUT)/mlock-random-test: LDLIBS += -lcap > + > +$(OUTPUT)/gup_test: ../../../../mm/gup_test.h There is no reason to do this, the auto depends will pick up header files, and gup_test.h isn't a generated file Jason
On Sun, Sep 27, 2020 at 11:21:59PM -0700, John Hubbard wrote: > @@ -76,8 +79,6 @@ TEST_FILES := test_vmalloc.sh > KSFT_KHDR_INSTALL := 1 > include ../lib.mk > > -$(OUTPUT)/hmm-tests: LDLIBS += -lhugetlbfs > - > ifeq ($(ARCH),x86_64) > BINARIES_32 := $(patsubst %,$(OUTPUT)/%,$(BINARIES_32)) > BINARIES_64 := $(patsubst %,$(OUTPUT)/%,$(BINARIES_64)) > @@ -130,3 +131,22 @@ endif > $(OUTPUT)/mlock-random-test: LDLIBS += -lcap > > $(OUTPUT)/gup_test: ../../../../mm/gup_test.h > + > +$(OUTPUT)/hmm-tests: local_config.h > + > +# HMM_EXTRA_LIBS may get set in local_config.mk, or it may be left empty. > +$(OUTPUT)/hmm-tests: LDLIBS += $(HMM_EXTRA_LIBS) > + > +local_config.mk local_config.h: check_config.sh > + ./check_config.sh > + > +EXTRA_CLEAN += local_config.mk local_config.h > + > +ifeq ($(HMM_EXTRA_LIBS),) > +all: warn_missing_hugelibs > + > +warn_missing_hugelibs: > + @echo ; \ > + echo "Warning: missing libhugetlbfs support. Some HMM tests will be skipped." ; \ > + echo > +endif > diff --git a/tools/testing/selftests/vm/check_config.sh b/tools/testing/selftests/vm/check_config.sh > new file mode 100755 > index 000000000000..651a4b192479 > +++ b/tools/testing/selftests/vm/check_config.sh > @@ -0,0 +1,30 @@ > +#!/bin/sh > +# SPDX-License-Identifier: GPL-2.0 > +# > +# Probe for libraries and create header files to record the results. Both C > +# header files and Makefile include fragments are created. > + > +OUTPUT_H_FILE=local_config.h > +OUTPUT_MKFILE=local_config.mk > + > +# libhugetlbfs > +tmpname=$(mktemp) > +tmpfile_c=${tmpname}.c > +tmpfile_o=${tmpname}.o > + > +echo "#include <sys/types.h>" > $tmpfile_c > +echo "#include <hugetlbfs.h>" >> $tmpfile_c > +echo "int func(void) { return 0; }" >> $tmpfile_c > + > +gcc -c $tmpfile_c -o $tmpfile_o >/dev/null 2>&1 This gcc has to come from some makefile variable This is kind of janky :\ Could we just not use libhugetlbfs? Doesn't it all just boil down to creating a file in /dev/huge? Eg look at tools/testing/selftests/vm/hugepage-mmap.c Jason
On Sun, Sep 27, 2020 at 11:21:58PM -0700, John Hubbard wrote: > Run benchmarks on the _fast variants of gup and pup, as originally > intended. > > Run the new gup_test sub-test: dump pages. In addition to exercising the > dump_page() call, it also demonstrates the various options you can use > to specify which pages to dump, and how. > > Signed-off-by: John Hubbard <jhubbard@nvidia.com> > --- > tools/testing/selftests/vm/run_vmtest.sh | 24 ++++++++++++++++++------ > 1 file changed, 18 insertions(+), 6 deletions(-) > > diff --git a/tools/testing/selftests/vm/run_vmtest.sh b/tools/testing/selftests/vm/run_vmtest.sh > index d1843d5f3c30..e3a8b14d9df6 100755 > --- a/tools/testing/selftests/vm/run_vmtest.sh > +++ b/tools/testing/selftests/vm/run_vmtest.sh > @@ -124,9 +124,9 @@ else > fi > > echo "--------------------------------------------" > -echo "running 'gup_test -U' (normal/slow gup)" > +echo "running 'gup_test -u' (fast gup benchmark)" > echo "--------------------------------------------" > -./gup_test -U > +./gup_test -u > if [ $? -ne 0 ]; then > echo "[FAIL]" > exitcode=1 > @@ -134,10 +134,22 @@ else > echo "[PASS]" > fi > > -echo "------------------------------------------" > -echo "running gup_test -b (pin_user_pages)" > -echo "------------------------------------------" > -./gup_test -b > +echo "---------------------------------------------------" > +echo "running gup_test -a (pin_user_pages_fast benchmark)" > +echo "---------------------------------------------------" > +./gup_test -a > +if [ $? -ne 0 ]; then > + echo "[FAIL]" > + exitcode=1 > +else > + echo "[PASS]" > +fi > + > +echo "--------------------------------------------------------------" > +echo "running gup_test -ct -F 0x1 0 19 0x1000" > +echo " Dumps pages 0, 19, and 4096, using pin_user_pages (-F 0x1)" > +echo "--------------------------------------------------------------" > +./gup_test -ct -F 0x1 0 19 0x1000 Ah here it is... Maybe just remove that from the previous commit message. Ira > if [ $? -ne 0 ]; then > echo "[FAIL]" > exitcode=1 > -- > 2.28.0 > >
On 9/28/20 12:26 PM, Ira Weiny wrote: > On Sun, Sep 27, 2020 at 11:21:58PM -0700, John Hubbard wrote: ... >> +echo "--------------------------------------------------------------" >> +echo "running gup_test -ct -F 0x1 0 19 0x1000" >> +echo " Dumps pages 0, 19, and 4096, using pin_user_pages (-F 0x1)" >> +echo "--------------------------------------------------------------" >> +./gup_test -ct -F 0x1 0 19 0x1000 > > Ah here it is... Maybe just remove that from the previous commit message. > > Ira Yes, will do, thanks for spotting that. thanks,
On 9/28/20 5:57 AM, Jason Gunthorpe wrote: > On Sun, Sep 27, 2020 at 11:21:53PM -0700, John Hubbard wrote: >> diff --git a/tools/testing/selftests/vm/Makefile b/tools/testing/selftests/vm/Makefile >> index d1ae706d9927..9cc6bc087461 100644 >> +++ b/tools/testing/selftests/vm/Makefile >> @@ -130,3 +130,5 @@ endif >> $(OUTPUT)/userfaultfd: LDLIBS += -lpthread >> >> $(OUTPUT)/mlock-random-test: LDLIBS += -lcap >> + >> +$(OUTPUT)/gup_test: ../../../../mm/gup_test.h > > There is no reason to do this, the auto depends will pick up header > files, and gup_test.h isn't a generated file > It is less capable than you might think. Without the admittedly ugly technique above, it fails to build, and as you can see, the include paths that are fed to gcc are just a single one: usr/include: $ make make --no-builtin-rules ARCH=x86 -C ../../../.. headers_install gcc -Wall -I ../../../../usr/include gup_test.c /kernel_work/linux-next-github/tools/testing/selftests/kselftest_harness.h /kernel_work/linux-next-github/tools/testing/selftests/kselftest.h ../../../../mm/gup_test.h -lrt -o /kernel_work/linux-next-github/tools/testing/selftests/vm/gup_test make[1]: Entering directory '/kernel_work/linux-next-github' gup_test.c:10:10: fatal error: gup_test.h: No such file or directory 10 | #include "gup_test.h" | ^~~~~~~~~~~~ thanks,
On 9/28/20 6:02 AM, Jason Gunthorpe wrote: > On Sun, Sep 27, 2020 at 11:21:59PM -0700, John Hubbard wrote: ... >> +gcc -c $tmpfile_c -o $tmpfile_o >/dev/null 2>&1 > > This gcc has to come from some makefile variable ahem, yes, that really should have just been $(CC), will change to that. > > This is kind of janky :\ > > Could we just not use libhugetlbfs? Doesn't it all just boil down to > creating a file in /dev/huge? Eg look at tools/testing/selftests/vm/hugepage-mmap.c > Well, the situation is a lot worse than that, because hmm-tests.c is using a few helper functions that end up pulling in more and more. My first attempt was actually in your direction: just grab a bit of code from the library and drop it in. But that ended up turning into several pages of code from quite a few functions and definitions, and it was looking maybe excessive. (I can look at it again, though. Maybe it feels less excessive if there are no other acceptible alternatives.) So then I thought, why not just *delete* those two routines from hmm-tests.c? But Ralph didn't like that because he notes that hmm_range_fault() loses some useful test coverage by being exercised with hugetlbfs. So finally I landed here, which is so far, the smallest change that would be potentially acceptible: a couple dozen lines, in order to selectively disable the problematic routines. Anyway, thoughts? I like the current approach but am open to anything that makes hmm-test Just Work for more people, on the *first* try. thanks, -- John Hubbard NVIDIA
On 9/28/20 1:18 PM, John Hubbard wrote: > On 9/28/20 6:02 AM, Jason Gunthorpe wrote: >> On Sun, Sep 27, 2020 at 11:21:59PM -0700, John Hubbard wrote: > ... >>> +gcc -c $tmpfile_c -o $tmpfile_o >/dev/null 2>&1 >> >> This gcc has to come from some makefile variable I plan on posting a v2 with this additional change, to fix the above point: diff --git a/tools/testing/selftests/vm/Makefile b/tools/testing/selftests/vm/Makefile index 986a90fa9653..019cbb7f3cf8 100644 --- a/tools/testing/selftests/vm/Makefile +++ b/tools/testing/selftests/vm/Makefile @@ -138,7 +138,7 @@ $(OUTPUT)/hmm-tests: local_config.h $(OUTPUT)/hmm-tests: LDLIBS += $(HMM_EXTRA_LIBS) local_config.mk local_config.h: check_config.sh - ./check_config.sh + ./check_config.sh $(CC) EXTRA_CLEAN += local_config.mk local_config.h diff --git a/tools/testing/selftests/vm/check_config.sh b/tools/testing/selftests/vm/check_config.sh index 651a4b192479..079c8a40b85d 100755 --- a/tools/testing/selftests/vm/check_config.sh +++ b/tools/testing/selftests/vm/check_config.sh @@ -16,7 +16,8 @@ echo "#include <sys/types.h>" > $tmpfile_c echo "#include <hugetlbfs.h>" >> $tmpfile_c echo "int func(void) { return 0; }" >> $tmpfile_c -gcc -c $tmpfile_c -o $tmpfile_o >/dev/null 2>&1 +CC=${1:?"Usage: $0 <compiler> # example compiler: gcc"} +$CC -c $tmpfile_c -o $tmpfile_o >/dev/null 2>&1 if [ -f $tmpfile_o ]; then echo "#define LOCAL_CONFIG_HAVE_LIBHUGETLBFS 1" > $OUTPUT_H_FILE thanks, -- John Hubbard NVIDIA
On Mon, Sep 28, 2020 at 01:10:24PM -0700, John Hubbard wrote: > On 9/28/20 5:57 AM, Jason Gunthorpe wrote: > > On Sun, Sep 27, 2020 at 11:21:53PM -0700, John Hubbard wrote: > > > diff --git a/tools/testing/selftests/vm/Makefile b/tools/testing/selftests/vm/Makefile > > > index d1ae706d9927..9cc6bc087461 100644 > > > +++ b/tools/testing/selftests/vm/Makefile > > > @@ -130,3 +130,5 @@ endif > > > $(OUTPUT)/userfaultfd: LDLIBS += -lpthread > > > $(OUTPUT)/mlock-random-test: LDLIBS += -lcap > > > + > > > +$(OUTPUT)/gup_test: ../../../../mm/gup_test.h > > > > There is no reason to do this, the auto depends will pick up header > > files, and gup_test.h isn't a generated file > > > > It is less capable than you might think. Without the admittedly ugly technique > above, it fails to build, and as you can see, the include paths that are fed to > gcc are just a single one: usr/include: > > $ make > make --no-builtin-rules ARCH=x86 -C ../../../.. headers_install > gcc -Wall -I ../../../../usr/include gup_test.c > /kernel_work/linux-next-github/tools/testing/selftests/kselftest_harness.h > /kernel_work/linux-next-github/tools/testing/selftests/kselftest.h > ../../../../mm/gup_test.h -lrt -o > /kernel_work/linux-next-github/tools/testing/selftests/vm/gup_test > make[1]: Entering directory '/kernel_work/linux-next-github' > gup_test.c:10:10: fatal error: gup_test.h: No such file or directory > 10 | #include "gup_test.h" > | ^~~~~~~~~~~~ You are supposed to use #include "../../../../mm/gup_test.h" I have no idea what weird behavior the makefile is triggering that the above include works Jason
On 9/29/20 9:35 AM, Jason Gunthorpe wrote: > On Mon, Sep 28, 2020 at 01:10:24PM -0700, John Hubbard wrote: >> On 9/28/20 5:57 AM, Jason Gunthorpe wrote: >>> On Sun, Sep 27, 2020 at 11:21:53PM -0700, John Hubbard wrote: >>>> diff --git a/tools/testing/selftests/vm/Makefile b/tools/testing/selftests/vm/Makefile >>>> index d1ae706d9927..9cc6bc087461 100644 >>>> +++ b/tools/testing/selftests/vm/Makefile >>>> @@ -130,3 +130,5 @@ endif >>>> $(OUTPUT)/userfaultfd: LDLIBS += -lpthread >>>> $(OUTPUT)/mlock-random-test: LDLIBS += -lcap >>>> + >>>> +$(OUTPUT)/gup_test: ../../../../mm/gup_test.h >>> >>> There is no reason to do this, the auto depends will pick up header >>> files, and gup_test.h isn't a generated file >>> Oh, I misread your comment! You were talking about this Makefile dependency that I'm adding, rather than the ../'s in the path. Well, for that though, it also has to stay as shown in this patch, because of this: I don't see any "gcc -m" type of dependency generation pass happening in this relatively simple Make system. And so, without including an explicit header file dependency (at least, that's the simplest way), changes to gup_test.h are not detected. Both the Makefile code and the observed behavior back this up. (I expect that this is because there is less use of header files in this area, because most unit tests are self-contained within a single .c file.) >> >> It is less capable than you might think. Without the admittedly ugly technique >> above, it fails to build, and as you can see, the include paths that are fed to >> gcc are just a single one: usr/include: >> >> $ make >> make --no-builtin-rules ARCH=x86 -C ../../../.. headers_install >> gcc -Wall -I ../../../../usr/include gup_test.c >> /kernel_work/linux-next-github/tools/testing/selftests/kselftest_harness.h >> /kernel_work/linux-next-github/tools/testing/selftests/kselftest.h >> ../../../../mm/gup_test.h -lrt -o >> /kernel_work/linux-next-github/tools/testing/selftests/vm/gup_test >> make[1]: Entering directory '/kernel_work/linux-next-github' >> gup_test.c:10:10: fatal error: gup_test.h: No such file or directory >> 10 | #include "gup_test.h" >> | ^~~~~~~~~~~~ > > You are supposed to use > > #include "../../../../mm/gup_test.h" Good, I'll leave it as I had it. > I have no idea what weird behavior the makefile is triggering that the > above include works > See the commit description for yet another Makefile weird behavior point. :) thanks, -- John Hubbard NVIDIA
On Tue, Sep 29, 2020 at 10:44:31AM -0700, John Hubbard wrote: > On 9/29/20 9:35 AM, Jason Gunthorpe wrote: > > On Mon, Sep 28, 2020 at 01:10:24PM -0700, John Hubbard wrote: > > > On 9/28/20 5:57 AM, Jason Gunthorpe wrote: > > > > On Sun, Sep 27, 2020 at 11:21:53PM -0700, John Hubbard wrote: > > > > > diff --git a/tools/testing/selftests/vm/Makefile b/tools/testing/selftests/vm/Makefile > > > > > index d1ae706d9927..9cc6bc087461 100644 > > > > > +++ b/tools/testing/selftests/vm/Makefile > > > > > @@ -130,3 +130,5 @@ endif > > > > > $(OUTPUT)/userfaultfd: LDLIBS += -lpthread > > > > > $(OUTPUT)/mlock-random-test: LDLIBS += -lcap > > > > > + > > > > > +$(OUTPUT)/gup_test: ../../../../mm/gup_test.h > > > > > > > > There is no reason to do this, the auto depends will pick up header > > > > files, and gup_test.h isn't a generated file > > > > > > Oh, I misread your comment! You were talking about this Makefile > dependency that I'm adding, rather than the ../'s in the path. > > Well, for that though, it also has to stay as shown in this patch, > because of this: > > I don't see any "gcc -m" type of dependency generation pass happening > in this relatively simple Make system. It happens with -MD, all the deps are stored in files like mm/.init-mm.o.cmd and sucked into the build. > And so, without including an explicit header file dependency (at > least, that's the simplest way), changes to gup_test.h are not > detected. Shouldn't be > Both the Makefile code and the observed behavior back this up. (I > expect that this is because there is less use of header files in > this area, because most unit tests are self-contained within a > single .c file.) Something else is very wrong then. Jason
On 9/29/20 10:55 AM, Jason Gunthorpe wrote: > On Tue, Sep 29, 2020 at 10:44:31AM -0700, John Hubbard wrote: >> On 9/29/20 9:35 AM, Jason Gunthorpe wrote: >>> On Mon, Sep 28, 2020 at 01:10:24PM -0700, John Hubbard wrote: >>>> On 9/28/20 5:57 AM, Jason Gunthorpe wrote: >>>>> On Sun, Sep 27, 2020 at 11:21:53PM -0700, John Hubbard wrote: ... >> I don't see any "gcc -m" type of dependency generation pass happening >> in this relatively simple Make system. > > It happens with -MD, all the deps are stored in files like mm/.init-mm.o.cmd > and sucked into the build. You are thinking of kbuild. This is not kbuild. There are no such artifacts being generated. >> And so, without including an explicit header file dependency (at >> least, that's the simplest way), changes to gup_test.h are not >> detected. > > Shouldn't be > >> Both the Makefile code and the observed behavior back this up. (I >> expect that this is because there is less use of header files in >> this area, because most unit tests are self-contained within a >> single .c file.) > > Something else is very wrong then. > Not really, it's just a less-cabable system than kbuild. thanks, -- John Hubbard NVIDIA
On Tue, Sep 29, 2020 at 11:59:55AM -0700, John Hubbard wrote: > On 9/29/20 10:55 AM, Jason Gunthorpe wrote: > > On Tue, Sep 29, 2020 at 10:44:31AM -0700, John Hubbard wrote: > > > On 9/29/20 9:35 AM, Jason Gunthorpe wrote: > > > > On Mon, Sep 28, 2020 at 01:10:24PM -0700, John Hubbard wrote: > > > > > On 9/28/20 5:57 AM, Jason Gunthorpe wrote: > > > > > > On Sun, Sep 27, 2020 at 11:21:53PM -0700, John Hubbard wrote: > ... > > > I don't see any "gcc -m" type of dependency generation pass happening > > > in this relatively simple Make system. > > > > It happens with -MD, all the deps are stored in files like mm/.init-mm.o.cmd > > and sucked into the build. > > You are thinking of kbuild. This is not kbuild. There are no such artifacts > being generated. Oh. Really? That's horrible. Jason
On 9/29/20 12:08 PM, Jason Gunthorpe wrote: > On Tue, Sep 29, 2020 at 11:59:55AM -0700, John Hubbard wrote: >> On 9/29/20 10:55 AM, Jason Gunthorpe wrote: >>> On Tue, Sep 29, 2020 at 10:44:31AM -0700, John Hubbard wrote: >>>> On 9/29/20 9:35 AM, Jason Gunthorpe wrote: >>>>> On Mon, Sep 28, 2020 at 01:10:24PM -0700, John Hubbard wrote: >>>>>> On 9/28/20 5:57 AM, Jason Gunthorpe wrote: >>>>>>> On Sun, Sep 27, 2020 at 11:21:53PM -0700, John Hubbard wrote: >> ... >>>> I don't see any "gcc -m" type of dependency generation pass happening >>>> in this relatively simple Make system. >>> >>> It happens with -MD, all the deps are stored in files like mm/.init-mm.o.cmd >>> and sucked into the build. >> >> You are thinking of kbuild. This is not kbuild. There are no such artifacts >> being generated. > > Oh. Really? That's horrible. > Well, yes, it's not a perfect build system down here in selftests/. Are you saying that it is worth upgrading? I'm open to suggestions and ideas for improvements, and at the moment, I have the miniature build system here mostly loaded into my head. So for a brief shining moment I can probably understand it well enough to work on it. :) thanks, -- John Hubbard NVIDIA
On Tue, Sep 29, 2020 at 12:48:43PM -0700, John Hubbard wrote: > On 9/29/20 12:08 PM, Jason Gunthorpe wrote: > > On Tue, Sep 29, 2020 at 11:59:55AM -0700, John Hubbard wrote: > > > On 9/29/20 10:55 AM, Jason Gunthorpe wrote: > > > > On Tue, Sep 29, 2020 at 10:44:31AM -0700, John Hubbard wrote: > > > > > On 9/29/20 9:35 AM, Jason Gunthorpe wrote: > > > > > > On Mon, Sep 28, 2020 at 01:10:24PM -0700, John Hubbard wrote: > > > > > > > On 9/28/20 5:57 AM, Jason Gunthorpe wrote: > > > > > > > > On Sun, Sep 27, 2020 at 11:21:53PM -0700, John Hubbard wrote: > > > ... > > > > > I don't see any "gcc -m" type of dependency generation pass happening > > > > > in this relatively simple Make system. > > > > > > > > It happens with -MD, all the deps are stored in files like mm/.init-mm.o.cmd > > > > and sucked into the build. > > > > > > You are thinking of kbuild. This is not kbuild. There are no such artifacts > > > being generated. > > > > Oh. Really? That's horrible. > > > > Well, yes, it's not a perfect build system down here in selftests/. Are you saying > that it is worth upgrading? I'm open to suggestions and ideas for improvements, > and at the moment, I have the miniature build system here mostly loaded into my > head. So for a brief shining moment I can probably understand it well enough to > work on it. :) I only remarked because I didn't know it wasn't using kbuild. I thought it would have used the existing HOSTCC stuff, not sure why it is special. The only investment that seems worthwhile would be to switch it to use the normal kbuild stuff?? Jason
On 9/29/20 1:53 PM, Jason Gunthorpe wrote: > On Tue, Sep 29, 2020 at 12:48:43PM -0700, John Hubbard wrote: >> On 9/29/20 12:08 PM, Jason Gunthorpe wrote: >>> On Tue, Sep 29, 2020 at 11:59:55AM -0700, John Hubbard wrote: >>>> On 9/29/20 10:55 AM, Jason Gunthorpe wrote: >>>>> On Tue, Sep 29, 2020 at 10:44:31AM -0700, John Hubbard wrote: >>>>>> On 9/29/20 9:35 AM, Jason Gunthorpe wrote: >>>>>>> On Mon, Sep 28, 2020 at 01:10:24PM -0700, John Hubbard wrote: >>>>>>>> On 9/28/20 5:57 AM, Jason Gunthorpe wrote: >>>>>>>>> On Sun, Sep 27, 2020 at 11:21:53PM -0700, John Hubbard wrote: >>>> ... >>>>>> I don't see any "gcc -m" type of dependency generation pass happening >>>>>> in this relatively simple Make system. >>>>> >>>>> It happens with -MD, all the deps are stored in files like mm/.init-mm.o.cmd >>>>> and sucked into the build. >>>> >>>> You are thinking of kbuild. This is not kbuild. There are no such artifacts >>>> being generated. >>> >>> Oh. Really? That's horrible. >>> >> >> Well, yes, it's not a perfect build system down here in selftests/. Are you saying >> that it is worth upgrading? I'm open to suggestions and ideas for improvements, >> and at the moment, I have the miniature build system here mostly loaded into my >> head. So for a brief shining moment I can probably understand it well enough to >> work on it. :) > > I only remarked because I didn't know it wasn't using kbuild. I > thought it would have used the existing HOSTCC stuff, not sure why it > is special. > > The only investment that seems worthwhile would be to switch it to use > the normal kbuild stuff?? > I explored switching to kbuild at the kernel summit last year during my kselftest where are we talk. There was push back from several developers. We can definitely explore it as long as we can still support being able to build and run individual subsystem tests and doesn't break workflow for developers. If you are up for it, propose a patch and we can discuss it. thanks, -- Shuah
On 9/29/20 1:00 PM, Shuah Khan wrote: > On 9/29/20 1:53 PM, Jason Gunthorpe wrote: >> I only remarked because I didn't know it wasn't using kbuild. I >> thought it would have used the existing HOSTCC stuff, not sure why it >> is special. >> >> The only investment that seems worthwhile would be to switch it to use >> the normal kbuild stuff?? >> > > I explored switching to kbuild at the kernel summit last year during > my kselftest where are we talk. > > There was push back from several developers. We can definitely explore > it as long as we can still support being able to build and run > individual subsystem tests and doesn't break workflow for developers. > Do you have a link or two for that? Especially about the pushback, and conclusions reached, if any. thanks, -- John Hubbard NVIDIA
On 9/29/20 2:11 PM, John Hubbard wrote: > On 9/29/20 1:00 PM, Shuah Khan wrote: >> On 9/29/20 1:53 PM, Jason Gunthorpe wrote: >>> I only remarked because I didn't know it wasn't using kbuild. I >>> thought it would have used the existing HOSTCC stuff, not sure why it >>> is special. >>> >>> The only investment that seems worthwhile would be to switch it to use >>> the normal kbuild stuff?? >>> >> >> I explored switching to kbuild at the kernel summit last year during >> my kselftest where are we talk. >> >> There was push back from several developers. We can definitely explore >> it as long as we can still support being able to build and run >> individual subsystem tests and doesn't break workflow for developers. >> > > Do you have a link or two for that? Especially about the pushback, and > conclusions reached, if any. > Unfortunately no. A I recall it was workflow related issues and ease of running individual subsystem tests and backwards compatibility with stables. Let's start a new discussion and see where we land. thanks, -- Shuah
On 9/29/20 1:20 PM, Shuah Khan wrote: > On 9/29/20 2:11 PM, John Hubbard wrote: ... >> Do you have a link or two for that? Especially about the pushback, and >> conclusions reached, if any. >> > > Unfortunately no. A I recall it was workflow related issues and ease of > running individual subsystem tests and backwards compatibility with > stables. > > Let's start a new discussion and see where we land. > OK, sure. I can take a quick pass at converting just the selftests/vm directory to kbuild, and post that as an RFC to start the discussion. thanks, -- John Hubbard NVIDIA
This is based on the latest mmotm. Summary: This series provides two main things, and a number of smaller supporting goodies. The two main points are: 1) Add a new sub-test to gup_test, which in turn is a renamed version of gup_benchmark. This sub-test allows nicer testing of dump_pages(), at least on user-space pages. For quite a while, I was doing a quick hack to gup_test.c whenever I wanted to try out changes to dump_page(). Then Matthew Wilcox asked me what I meant when I said "I used my dump_page() unit test", and I realized that it might be nice to check in a polished up version of that. Details about how it works and how to use it are in the commit description for patch #6. 2) Fixes a limitation of hmm-tests: these tests are incredibly useful, but only if people actually build and run them. And it turns out that libhugetlbfs is a little too effective at throwing a wrench in the works, there. So I've added a little configuration check that removes just two of the 21 hmm-tests, if libhugetlbfs is not available. Further details in the commit description of patch #8. Other smaller things that this series does: a) Remove code duplication by creating gup_test.h. b) Clear up the sub-test organization, and their invocation within run_vmtests.sh. c) Other minor assorted improvements. John Hubbard (8): mm/gup_benchmark: rename to mm/gup_test selftests/vm: use a common gup_test.h selftests/vm: rename run_vmtests --> run_vmtests.sh selftests/vm: minor cleanup: Makefile and gup_test.c selftests/vm: only some gup_test items are really benchmarks selftests/vm: gup_test: introduce the dump_pages() sub-test selftests/vm: run_vmtest.sh: update and clean up gup_test invocation selftests/vm: hmm-tests: remove the libhugetlbfs dependency Documentation/core-api/pin_user_pages.rst | 6 +- arch/s390/configs/debug_defconfig | 2 +- arch/s390/configs/defconfig | 2 +- mm/Kconfig | 21 +- mm/Makefile | 2 +- mm/{gup_benchmark.c => gup_test.c} | 109 ++++++---- mm/gup_test.h | 32 +++ tools/testing/selftests/vm/.gitignore | 3 +- tools/testing/selftests/vm/Makefile | 38 +++- tools/testing/selftests/vm/check_config.sh | 30 +++ tools/testing/selftests/vm/config | 2 +- tools/testing/selftests/vm/gup_benchmark.c | 137 ------------- tools/testing/selftests/vm/gup_test.c | 188 ++++++++++++++++++ tools/testing/selftests/vm/hmm-tests.c | 10 +- .../vm/{run_vmtests => run_vmtest.sh} | 24 ++- 15 files changed, 403 insertions(+), 203 deletions(-) rename mm/{gup_benchmark.c => gup_test.c} (59%) create mode 100644 mm/gup_test.h create mode 100755 tools/testing/selftests/vm/check_config.sh delete mode 100644 tools/testing/selftests/vm/gup_benchmark.c create mode 100644 tools/testing/selftests/vm/gup_test.c rename tools/testing/selftests/vm/{run_vmtests => run_vmtest.sh} (91%)