06 May, 2021
1 commit
-
In gup_test both gup_flags and test_flags use the same flags field.
This is broken.Farther, in the actual gup_test.c all the passed gup_flags are erased
and unconditionally replaced with FOLL_WRITE.Which means that test_flags are ignored, and code like this always
performs pin dump test:155 if (gup->flags & GUP_TEST_FLAG_DUMP_PAGES_USE_PIN)
156 nr = pin_user_pages(addr, nr, gup->flags,
157 pages + i, NULL);
158 else
159 nr = get_user_pages(addr, nr, gup->flags,
160 pages + i, NULL);
161 break;Add a new test_flags field, to allow raw gup_flags to work. Add a new
subcommand for DUMP_USER_PAGES_TEST to specify that pin test should be
performed.Remove unconditional overwriting of gup_flags via FOLL_WRITE. But,
preserve the previous behaviour where FOLL_WRITE was the default flag,
and add a new option "-W" to unset FOLL_WRITE.Rename flags with gup_flags.
With the fix, dump works like this:
root@virtme:/# gup_test -c
---- page #0, starting from user virt addr: 0x7f8acb9e4000
page:00000000d3d2ee27 refcount:2 mapcount:1 mapping:0000000000000000
index:0x0 pfn:0x100bcf
anon flags: 0x300000000080016(referenced|uptodate|lru|swapbacked)
raw: 0300000000080016 ffffd0e204021608 ffffd0e208df2e88 ffff8ea04243ec61
raw: 0000000000000000 0000000000000000 0000000200000000 0000000000000000
page dumped because: gup_test: dump_pages() test
DUMP_USER_PAGES_TEST: doneroot@virtme:/# gup_test -c -p
---- page #0, starting from user virt addr: 0x7fd19701b000
page:00000000baed3c7d refcount:1025 mapcount:1 mapping:0000000000000000
index:0x0 pfn:0x108008
anon flags: 0x300000000080014(uptodate|lru|swapbacked)
raw: 0300000000080014 ffffd0e204200188 ffffd0e205e09088 ffff8ea04243ee71
raw: 0000000000000000 0000000000000000 0000040100000000 0000000000000000
page dumped because: gup_test: dump_pages() test
DUMP_USER_PAGES_TEST: doneRefcount shows the difference between pin vs no-pin case.
Also change type of nr from int to long, as it counts number of pages.Link: https://lkml.kernel.org/r/20210215161349.246722-14-pasha.tatashin@soleen.com
Signed-off-by: Pavel Tatashin
Reviewed-by: John Hubbard
Cc: Dan Williams
Cc: David Hildenbrand
Cc: David Rientjes
Cc: Ingo Molnar
Cc: Ira Weiny
Cc: James Morris
Cc: Jason Gunthorpe
Cc: Jason Gunthorpe
Cc: Joonsoo Kim
Cc: Matthew Wilcox
Cc: Mel Gorman
Cc: Michal Hocko
Cc: Michal Hocko
Cc: Mike Kravetz
Cc: Oscar Salvador
Cc: Peter Zijlstra
Cc: Sasha Levin
Cc: Steven Rostedt (VMware)
Cc: Tyler Hicks
Cc: Vlastimil Babka
Signed-off-by: Andrew Morton
Signed-off-by: Linus Torvalds
16 Dec, 2020
3 commits
-
For quite a while, I was doing a quick hack to gup_test.c (previously,
gup_benchmark.c) whenever I wanted to try out my changes to dump_page().
This makes that hack unnecessary, and instead allows anyone to easily get
the same coverage from a user space program. That saves a lot of time
because you don't have to change the kernel, in order to test different
pages and options.The new sub-test takes advantage of the existing gup_test infrastructure,
which already provides a simple user space program, some allocated user
space pages, an ioctl call, pinning of those pages (via either
get_user_pages or pin_user_pages) and a corresponding kernel-side test
invocation. There's not much more required, mainly just a couple of
inputs from the user.In fact, the new test re-uses the existing command line options in order
to get various helpful combinations (THP or normal, _fast or slow gup, gup
vs. pup, and more).New command line options are: which pages to dump, and what type of
"get/pin" to use.In order to figure out which pages to dump, the logic is:
* If the user doesn't specify anything, the page 0 (the first page in
the address range that the program sets up for testing) is dumped.* Or, the user can type up to 8 page indices anywhere on the command
line. If you type more than 8, then it uses the first 8 and ignores the
remaining items.For example:
./gup_test -ct -F 1 0 19 0x1000
Meaning:
-c: dump pages sub-test
-t: use THP pages
-F 1: use pin_user_pages() instead of get_user_pages()
0 19 0x1000: dump pages 0, 19, and 4096Link: https://lkml.kernel.org/r/20201026064021.3545418-7-jhubbard@nvidia.com
Signed-off-by: John Hubbard
Cc: Jérôme Glisse
Cc: Jonathan Corbet
Cc: Ralph Campbell
Cc: Shuah Khan
Signed-off-by: Andrew Morton
Signed-off-by: Linus Torvalds -
Therefore, some minor cleanup and improvements are in order:
1. Rename the other items appropriately.
2. Stop reporting timing information on the non-benchmark items. It's
still being recorded and is available, but there's no point in
cluttering up the report with data that no one reasonably needs to
check.3. Don't do iterations, for non-benchmark items.
4. Print out a shorter, more appropriate report for the non-benchmark
tests.5. Add the command that was run, to the report. This really helps, as
there are quite a lot of options now.6. Use a larger integer type for cmd, now that it's being compared
Otherwise it doesn't work, because in this case cmd is about 3 billion,
which is the perfect size for problems with signed vs unsigned int.Link: https://lkml.kernel.org/r/20201026064021.3545418-6-jhubbard@nvidia.com
Signed-off-by: John Hubbard
Cc: Jérôme Glisse
Cc: Jonathan Corbet
Cc: Ralph Campbell
Cc: Shuah Khan
Signed-off-by: Andrew Morton
Signed-off-by: Linus Torvalds -
Avoid the need to copy-paste the gup_test ioctl commands and the struct
gup_test definition, between the kernel and the user space application, by
providing a new header file for these. This allows easier and safer
adding of new ioctl calls, as well as reducing the overall line count.Details: The header file has to be able to compile independently, because
of the arguably unfortunate way that the Makefile is written: the Makefile
tries to build all of its prerequisites, when really it should be only
building the .c files, and leaving the other prerequisites (LOCAL_HDRS) as
pure dependencies.That Makefile limitation is probably not worth fixing, but it explains why
one of the includes had to be moved into the new header file.Also: simplify the ioctl struct (struct gup_test), by deleting the unused
__expansion[10] field. This sort of thing is what you might see in a
stable ABI, but this low-level, kernel-developer-oriented selftests/vm
system is very much not subject to ABI stability. So "expansion" and
"reserved" fields are unnecessary here.Link: https://lkml.kernel.org/r/20201026064021.3545418-3-jhubbard@nvidia.com
Signed-off-by: John Hubbard
Cc: Jérôme Glisse
Cc: Jonathan Corbet
Cc: Ralph Campbell
Cc: Shuah Khan
Signed-off-by: Andrew Morton
Signed-off-by: Linus Torvalds
