Hi,
This is v2 of a series that fixes up build errors and warnings for at least the 64-bit builds on x86 with clang.
There are lots of changes since v1 [1], thanks to reviews from Peter Xu, David Hildenbrand, and Muhammad Usama Anjum. These include:
* Using "make headers", and documenting that prerequisite as well. * Better ways to avoid clang's Wformat-security warnings * Added Cc's, ack-by's, reviewed-by's. * Updated commit log messages.
The series also includes an optional "improvement" of moving some uffd code into uffd-common.[ch], which is proving to be somewhat controversial, and so if that doesn't get resolved, then patches 9 and 10 may just get dropped. They are not required in order to get a clean build, now that "make headers" is happening.
[1]: https://lore.kernel.org/all/20230602013358.900637-1-jhubbard@nvidia.com/
thanks,
John Hubbard NVIDIA
John Hubbard (11): selftests/mm: fix uffd-stress unused function warning selftests/mm: fix unused variable warnings in hugetlb-madvise.c, migration.c selftests/mm: fix "warning: expression which evaluates to zero..." in mlock2-tests.c selftests/mm: fix invocation of tests that are run via shell scripts selftests/mm: .gitignore: add mkdirty, va_high_addr_switch selftests/mm: fix two -Wformat-security warnings in uffd builds selftests/mm: fix a "possibly uninitialized" warning in pkey-x86.h selftests/mm: fix uffd-unit-tests.c build failure due to missing MADV_COLLAPSE selftests/mm: move psize(), pshift() into vm_utils.c selftests/mm: move uffd* routines from vm_util.c to uffd-common.c Documentation: kselftest: "make headers" is a prerequisite
Documentation/dev-tools/kselftest.rst | 1 + tools/testing/selftests/mm/.gitignore | 2 + tools/testing/selftests/mm/Makefile | 7 +- tools/testing/selftests/mm/cow.c | 7 -- tools/testing/selftests/mm/hugepage-mremap.c | 2 +- tools/testing/selftests/mm/hugetlb-madvise.c | 8 +- tools/testing/selftests/mm/khugepaged.c | 10 -- .../selftests/mm/ksm_functional_tests.c | 2 +- tools/testing/selftests/mm/migration.c | 5 +- tools/testing/selftests/mm/mlock2-tests.c | 1 - tools/testing/selftests/mm/pkey-x86.h | 2 +- tools/testing/selftests/mm/run_vmtests.sh | 6 +- tools/testing/selftests/mm/uffd-common.c | 105 +++++++++++++++++ tools/testing/selftests/mm/uffd-common.h | 12 +- tools/testing/selftests/mm/uffd-stress.c | 10 -- tools/testing/selftests/mm/uffd-unit-tests.c | 16 +-- tools/testing/selftests/mm/vm_util.c | 106 ++---------------- tools/testing/selftests/mm/vm_util.h | 36 ++---- 18 files changed, 165 insertions(+), 173 deletions(-)
base-commit: 929ed21dfdb6ee94391db51c9eedb63314ef6847
uffd_minor_feature() was unused. Remove it in order to fix the associated clang build warning.
Reviewed-by: David Hildenbrand david@redhat.com Reviewed-by: Peter Xu peterx@redhat.com Signed-off-by: John Hubbard jhubbard@nvidia.com --- tools/testing/selftests/mm/uffd-stress.c | 10 ---------- 1 file changed, 10 deletions(-)
diff --git a/tools/testing/selftests/mm/uffd-stress.c b/tools/testing/selftests/mm/uffd-stress.c index f1ad9eef1c3a..995ff13e74c7 100644 --- a/tools/testing/selftests/mm/uffd-stress.c +++ b/tools/testing/selftests/mm/uffd-stress.c @@ -88,16 +88,6 @@ static void uffd_stats_reset(struct uffd_args *args, unsigned long n_cpus) } }
-static inline uint64_t uffd_minor_feature(void) -{ - if (test_type == TEST_HUGETLB && map_shared) - return UFFD_FEATURE_MINOR_HUGETLBFS; - else if (test_type == TEST_SHMEM) - return UFFD_FEATURE_MINOR_SHMEM; - else - return 0; -} - static void *locking_thread(void *arg) { unsigned long cpu = (unsigned long) arg;
Dummy variables are required in order to make these two (similar) routines work, so in both cases, declare the variables as volatile in order to avoid the clang compiler warning.
Furthermore, in order to ensure that each test actually does what is intended, add an asm volatile invocation (thanks to David Hildenbrand for the suggestion), with a clarifying comment so that it survives future maintenance.
Cc: David Hildenbrand david@redhat.com Cc: Peter Xu peterx@redhat.com Signed-off-by: John Hubbard jhubbard@nvidia.com --- tools/testing/selftests/mm/hugetlb-madvise.c | 8 ++++++-- tools/testing/selftests/mm/migration.c | 5 ++++- 2 files changed, 10 insertions(+), 3 deletions(-)
diff --git a/tools/testing/selftests/mm/hugetlb-madvise.c b/tools/testing/selftests/mm/hugetlb-madvise.c index 28426e30d9bc..d55322df4b73 100644 --- a/tools/testing/selftests/mm/hugetlb-madvise.c +++ b/tools/testing/selftests/mm/hugetlb-madvise.c @@ -65,11 +65,15 @@ void write_fault_pages(void *addr, unsigned long nr_pages)
void read_fault_pages(void *addr, unsigned long nr_pages) { - unsigned long dummy = 0; + volatile unsigned long dummy = 0; unsigned long i;
- for (i = 0; i < nr_pages; i++) + for (i = 0; i < nr_pages; i++) { dummy += *((unsigned long *)(addr + (i * huge_page_size))); + + /* Prevent the compiler from optimizing out the entire loop: */ + asm volatile("" : "+r" (dummy)); + } }
int main(int argc, char **argv) diff --git a/tools/testing/selftests/mm/migration.c b/tools/testing/selftests/mm/migration.c index 1cec8425e3ca..379581567f27 100644 --- a/tools/testing/selftests/mm/migration.c +++ b/tools/testing/selftests/mm/migration.c @@ -95,12 +95,15 @@ int migrate(uint64_t *ptr, int n1, int n2)
void *access_mem(void *ptr) { - uint64_t y = 0; + volatile uint64_t y = 0; volatile uint64_t *x = ptr;
while (1) { pthread_testcancel(); y += *x; + + /* Prevent the compiler from optimizing out the writes to y: */ + asm volatile("" : "+r" (y)); }
return NULL;
On 03.06.23 04:15, John Hubbard wrote:
Dummy variables are required in order to make these two (similar) routines work, so in both cases, declare the variables as volatile in order to avoid the clang compiler warning.
Furthermore, in order to ensure that each test actually does what is intended, add an asm volatile invocation (thanks to David Hildenbrand for the suggestion), with a clarifying comment so that it survives future maintenance.
Cc: David Hildenbrand david@redhat.com Cc: Peter Xu peterx@redhat.com Signed-off-by: John Hubbard jhubbard@nvidia.com
tools/testing/selftests/mm/hugetlb-madvise.c | 8 ++++++-- tools/testing/selftests/mm/migration.c | 5 ++++- 2 files changed, 10 insertions(+), 3 deletions(-)
diff --git a/tools/testing/selftests/mm/hugetlb-madvise.c b/tools/testing/selftests/mm/hugetlb-madvise.c index 28426e30d9bc..d55322df4b73 100644 --- a/tools/testing/selftests/mm/hugetlb-madvise.c +++ b/tools/testing/selftests/mm/hugetlb-madvise.c @@ -65,11 +65,15 @@ void write_fault_pages(void *addr, unsigned long nr_pages) void read_fault_pages(void *addr, unsigned long nr_pages) {
- unsigned long dummy = 0;
- volatile unsigned long dummy = 0; unsigned long i;
- for (i = 0; i < nr_pages; i++)
- for (i = 0; i < nr_pages; i++) { dummy += *((unsigned long *)(addr + (i * huge_page_size)));
/* Prevent the compiler from optimizing out the entire loop: */asm volatile("" : "+r" (dummy));- } }
int main(int argc, char **argv) diff --git a/tools/testing/selftests/mm/migration.c b/tools/testing/selftests/mm/migration.c index 1cec8425e3ca..379581567f27 100644 --- a/tools/testing/selftests/mm/migration.c +++ b/tools/testing/selftests/mm/migration.c @@ -95,12 +95,15 @@ int migrate(uint64_t *ptr, int n1, int n2) void *access_mem(void *ptr) {
- uint64_t y = 0;
- volatile uint64_t y = 0; volatile uint64_t *x = ptr;
while (1) { pthread_testcancel(); y += *x;
/* Prevent the compiler from optimizing out the writes to y: */asm volatile("" : "+r" (y));return NULL;
With the asm, I think the "volatile" might be completely unnecessary. But it doesn't hurt.
Reviewed-by: David Hildenbrand david@redhat.com
On Fri, Jun 02, 2023 at 07:15:49PM -0700, John Hubbard wrote:
Dummy variables are required in order to make these two (similar) routines work, so in both cases, declare the variables as volatile in order to avoid the clang compiler warning.
Furthermore, in order to ensure that each test actually does what is intended, add an asm volatile invocation (thanks to David Hildenbrand for the suggestion), with a clarifying comment so that it survives future maintenance.
Cc: David Hildenbrand david@redhat.com Cc: Peter Xu peterx@redhat.com Signed-off-by: John Hubbard jhubbard@nvidia.com
Reviewed-by: Peter Xu peterx@redhat.com
The stop variable is a char*, and the code was assigning a char value to it. This was generating a warning when compiling with clang.
However, as both David and Peter pointed out, stop is not even used after the problematic assignment to a char type. So just delete that line entirely.
Cc: David Hildenbrand david@redhat.com Cc: Peter Xu peterx@redhat.com Signed-off-by: John Hubbard jhubbard@nvidia.com --- tools/testing/selftests/mm/mlock2-tests.c | 1 - 1 file changed, 1 deletion(-)
diff --git a/tools/testing/selftests/mm/mlock2-tests.c b/tools/testing/selftests/mm/mlock2-tests.c index 11b2301f3aa3..80cddc0de206 100644 --- a/tools/testing/selftests/mm/mlock2-tests.c +++ b/tools/testing/selftests/mm/mlock2-tests.c @@ -50,7 +50,6 @@ static int get_vm_area(unsigned long addr, struct vm_boundaries *area) printf("cannot parse /proc/self/maps\n"); goto out; } - stop = '\0';
sscanf(line, "%lx", &start); sscanf(end_addr, "%lx", &end);
On 03.06.23 04:15, John Hubbard wrote:
The stop variable is a char*, and the code was assigning a char value to it. This was generating a warning when compiling with clang.
However, as both David and Peter pointed out, stop is not even used after the problematic assignment to a char type. So just delete that line entirely.
Cc: David Hildenbrand david@redhat.com Cc: Peter Xu peterx@redhat.com Signed-off-by: John Hubbard jhubbard@nvidia.com
Reviewed-by: David Hildenbrand david@redhat.com
On Fri, Jun 02, 2023 at 07:15:50PM -0700, John Hubbard wrote:
The stop variable is a char*, and the code was assigning a char value to it. This was generating a warning when compiling with clang.
However, as both David and Peter pointed out, stop is not even used after the problematic assignment to a char type. So just delete that line entirely.
Cc: David Hildenbrand david@redhat.com Cc: Peter Xu peterx@redhat.com Signed-off-by: John Hubbard jhubbard@nvidia.com
tools/testing/selftests/mm/mlock2-tests.c | 1 - 1 file changed, 1 deletion(-)
diff --git a/tools/testing/selftests/mm/mlock2-tests.c b/tools/testing/selftests/mm/mlock2-tests.c index 11b2301f3aa3..80cddc0de206 100644 --- a/tools/testing/selftests/mm/mlock2-tests.c +++ b/tools/testing/selftests/mm/mlock2-tests.c @@ -50,7 +50,6 @@ static int get_vm_area(unsigned long addr, struct vm_boundaries *area) printf("cannot parse /proc/self/maps\n"); goto out; }
stop = '\0';sscanf(line, "%lx", &start); sscanf(end_addr, "%lx", &end);
I'd rather simply make it "*stop = '\0'", or as David suggested dropping stop completely when we're it (assumes that scanf() will always work with number ending with space ' ').
No strong opinion here, though.
On 6/5/23 08:43, Peter Xu wrote:
On Fri, Jun 02, 2023 at 07:15:50PM -0700, John Hubbard wrote:
The stop variable is a char*, and the code was assigning a char value to it. This was generating a warning when compiling with clang.
However, as both David and Peter pointed out, stop is not even used after the problematic assignment to a char type. So just delete that line entirely.
Cc: David Hildenbrand david@redhat.com Cc: Peter Xu peterx@redhat.com Signed-off-by: John Hubbard jhubbard@nvidia.com
tools/testing/selftests/mm/mlock2-tests.c | 1 - 1 file changed, 1 deletion(-)
diff --git a/tools/testing/selftests/mm/mlock2-tests.c b/tools/testing/selftests/mm/mlock2-tests.c index 11b2301f3aa3..80cddc0de206 100644 --- a/tools/testing/selftests/mm/mlock2-tests.c +++ b/tools/testing/selftests/mm/mlock2-tests.c @@ -50,7 +50,6 @@ static int get_vm_area(unsigned long addr, struct vm_boundaries *area) printf("cannot parse /proc/self/maps\n"); goto out; }
stop = '\0';sscanf(line, "%lx", &start); sscanf(end_addr, "%lx", &end);
I'd rather simply make it "*stop = '\0'", or as David suggested dropping stop completely when we're it (assumes that scanf() will always work with number ending with space ' ').
Actually it does not assume that. Rather, it follows the documented behavior of strchr(3), which is:
The strchr() and strrchr() functions return a pointer to the matched character or NULL if the character is not found. The terminating null byte is considered part of the string, so that if c is specified as '\0', these functions return a pointer to the terminator.
And we have this code now:
stop = strchr(end_addr, ' '); if (!stop) { printf("cannot parse /proc/self/maps\n"); goto out; }
So, either stop has a valid char* in it, or we goto out. There are no fragile assumptions in there, as far as I can see anyway.
No strong opinion here, though.
OK, I think it's kind of a flip of the coin whether to write this:
stop = strchr(end_addr, ' '); if (!stop) {
or this:
if (!strchr(end_addr, ' ')) {
So I'll just leave it as the first one, which (depending on the day of the week) might read slightly clearer. :)
thanks,
We cannot depend upon git to reliably retain the executable bit on shell scripts, or so I was told several years ago while working on this same run_vmtests.sh script. And sure enough, things such as test_hmm.sh are lately failing to run, due to lacking execute permissions.
Fix this by explicitly adding "bash" to each of the shell script invocations. Leave fixing the overall approach to another day.
Acked-by: David Hildenbrand david@redhat.com Cc: Peter Xu peterx@redhat.com Signed-off-by: John Hubbard jhubbard@nvidia.com --- tools/testing/selftests/mm/run_vmtests.sh | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/tools/testing/selftests/mm/run_vmtests.sh b/tools/testing/selftests/mm/run_vmtests.sh index 4893eb60d96d..8f81432e4bac 100644 --- a/tools/testing/selftests/mm/run_vmtests.sh +++ b/tools/testing/selftests/mm/run_vmtests.sh @@ -242,18 +242,18 @@ if [ $VADDR64 -ne 0 ]; then if [ "$ARCH" == "$ARCH_ARM64" ]; then echo 6 > /proc/sys/vm/nr_hugepages fi - CATEGORY="hugevm" run_test ./va_high_addr_switch.sh + CATEGORY="hugevm" run_test bash ./va_high_addr_switch.sh if [ "$ARCH" == "$ARCH_ARM64" ]; then echo $prev_nr_hugepages > /proc/sys/vm/nr_hugepages fi fi # VADDR64
# vmalloc stability smoke test -CATEGORY="vmalloc" run_test ./test_vmalloc.sh smoke +CATEGORY="vmalloc" run_test bash ./test_vmalloc.sh smoke
CATEGORY="mremap" run_test ./mremap_dontunmap
-CATEGORY="hmm" run_test ./test_hmm.sh smoke +CATEGORY="hmm" run_test bash ./test_hmm.sh smoke
# MADV_POPULATE_READ and MADV_POPULATE_WRITE tests CATEGORY="madv_populate" run_test ./madv_populate
These new build products were left out of .gitignore, so add them now.
Reviewed-by: David Hildenbrand david@redhat.com Cc: Peter Xu peterx@redhat.com Signed-off-by: John Hubbard jhubbard@nvidia.com --- tools/testing/selftests/mm/.gitignore | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/tools/testing/selftests/mm/.gitignore b/tools/testing/selftests/mm/.gitignore index 8917455f4f51..ab215303d8e9 100644 --- a/tools/testing/selftests/mm/.gitignore +++ b/tools/testing/selftests/mm/.gitignore @@ -39,3 +39,5 @@ local_config.h local_config.mk ksm_functional_tests mdwe_test +mkdirty +va_high_addr_switch \ No newline at end of file
On Fri, Jun 02, 2023 at 07:15:52PM -0700, John Hubbard wrote:
These new build products were left out of .gitignore, so add them now.
Reviewed-by: David Hildenbrand david@redhat.com Cc: Peter Xu peterx@redhat.com Signed-off-by: John Hubbard jhubbard@nvidia.com
tools/testing/selftests/mm/.gitignore | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/tools/testing/selftests/mm/.gitignore b/tools/testing/selftests/mm/.gitignore index 8917455f4f51..ab215303d8e9 100644 --- a/tools/testing/selftests/mm/.gitignore +++ b/tools/testing/selftests/mm/.gitignore @@ -39,3 +39,5 @@ local_config.h local_config.mk ksm_functional_tests mdwe_test +mkdirty +va_high_addr_switch \ No newline at end of file
Maybe also the new gup_longterm?
Reviewed-by: Peter Xu peterx@redhat.com
On 6/5/23 08:53, Peter Xu wrote:
On Fri, Jun 02, 2023 at 07:15:52PM -0700, John Hubbard wrote:
These new build products were left out of .gitignore, so add them now.
Reviewed-by: David Hildenbrand david@redhat.com Cc: Peter Xu peterx@redhat.com Signed-off-by: John Hubbard jhubbard@nvidia.com
tools/testing/selftests/mm/.gitignore | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/tools/testing/selftests/mm/.gitignore b/tools/testing/selftests/mm/.gitignore index 8917455f4f51..ab215303d8e9 100644 --- a/tools/testing/selftests/mm/.gitignore +++ b/tools/testing/selftests/mm/.gitignore @@ -39,3 +39,5 @@ local_config.h local_config.mk ksm_functional_tests mdwe_test +mkdirty +va_high_addr_switch \ No newline at end of file
Maybe also the new gup_longterm?
Yes I could add that. It might make slightly easier for Andrew because it would preemptively avoid a minor merge conflict.
Unless David is thinking of adding it to another version of that series?
Reviewed-by: Peter Xu peterx@redhat.com
Thanks for this (and the other) reviews!
thanks,
The uffd tests generate two compile time warnings from clang's -Wformat-security setting. These trigger at the call sites for uffd_test_start() and uffd_test_skip().
1) Fix the uffd_test_start() issue by removing the intermediate test_name variable (thanks to David Hildenbrand for showing how to do this).
2) Fix the uffd_test_skip() issue by observing that there is no need for a macro and a variable args approach, because all callers of uffd_test_skip() pass in a simple char* string, without any format specifiers. So just change uffd_test_skip() into a regular C function.
Cc: David Hildenbrand david@redhat.com Cc: Peter Xu peterx@redhat.com Signed-off-by: John Hubbard jhubbard@nvidia.com --- tools/testing/selftests/mm/uffd-unit-tests.c | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-)
diff --git a/tools/testing/selftests/mm/uffd-unit-tests.c b/tools/testing/selftests/mm/uffd-unit-tests.c index 269c86768a02..04d91f144d1c 100644 --- a/tools/testing/selftests/mm/uffd-unit-tests.c +++ b/tools/testing/selftests/mm/uffd-unit-tests.c @@ -109,12 +109,11 @@ static void uffd_test_pass(void) ksft_inc_fail_cnt(); \ } while (0)
-#define uffd_test_skip(...) do { \ - printf("skipped [reason: "); \ - printf(__VA_ARGS__); \ - printf("]\n"); \ - ksft_inc_xskip_cnt(); \ - } while (0) +static void uffd_test_skip(const char *message) +{ + printf("skipped [reason: %s]\n", message); + ksft_inc_xskip_cnt(); +}
/* * Returns 1 if specific userfaultfd supported, 0 otherwise. Note, we'll @@ -1149,7 +1148,6 @@ int main(int argc, char *argv[]) uffd_test_case_t *test; mem_type_t *mem_type; uffd_test_args_t args; - char test_name[128]; const char *errmsg; int has_uffd, opt; int i, j; @@ -1192,10 +1190,8 @@ int main(int argc, char *argv[]) mem_type = &mem_types[j]; if (!(test->mem_targets & mem_type->mem_flag)) continue; - snprintf(test_name, sizeof(test_name), - "%s on %s", test->name, mem_type->name);
- uffd_test_start(test_name); + uffd_test_start("%s on %s", test->name, mem_type->name); if (!uffd_feature_supported(test)) { uffd_test_skip("feature missing"); continue;
On 03.06.23 04:15, John Hubbard wrote:
The uffd tests generate two compile time warnings from clang's -Wformat-security setting. These trigger at the call sites for uffd_test_start() and uffd_test_skip().
- Fix the uffd_test_start() issue by removing the intermediate
test_name variable (thanks to David Hildenbrand for showing how to do this).
- Fix the uffd_test_skip() issue by observing that there is no need for
a macro and a variable args approach, because all callers of uffd_test_skip() pass in a simple char* string, without any format specifiers. So just change uffd_test_skip() into a regular C function.
Cc: David Hildenbrand david@redhat.com Cc: Peter Xu peterx@redhat.com Signed-off-by: John Hubbard jhubbard@nvidia.com
tools/testing/selftests/mm/uffd-unit-tests.c | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-)
diff --git a/tools/testing/selftests/mm/uffd-unit-tests.c b/tools/testing/selftests/mm/uffd-unit-tests.c index 269c86768a02..04d91f144d1c 100644 --- a/tools/testing/selftests/mm/uffd-unit-tests.c +++ b/tools/testing/selftests/mm/uffd-unit-tests.c @@ -109,12 +109,11 @@ static void uffd_test_pass(void) ksft_inc_fail_cnt(); \ } while (0) -#define uffd_test_skip(...) do { \
printf("skipped [reason: "); \printf(__VA_ARGS__); \printf("]\n"); \ksft_inc_xskip_cnt(); \- } while (0)
+static void uffd_test_skip(const char *message) +{
- printf("skipped [reason: %s]\n", message);
- ksft_inc_xskip_cnt();
+} /*
- Returns 1 if specific userfaultfd supported, 0 otherwise. Note, we'll
@@ -1149,7 +1148,6 @@ int main(int argc, char *argv[]) uffd_test_case_t *test; mem_type_t *mem_type; uffd_test_args_t args;
- char test_name[128]; const char *errmsg; int has_uffd, opt; int i, j;
@@ -1192,10 +1190,8 @@ int main(int argc, char *argv[]) mem_type = &mem_types[j]; if (!(test->mem_targets & mem_type->mem_flag)) continue;
snprintf(test_name, sizeof(test_name),"%s on %s", test->name, mem_type->name);
uffd_test_start(test_name);
uffd_test_start("%s on %s", test->name, mem_type->name); if (!uffd_feature_supported(test)) { uffd_test_skip("feature missing"); continue;
Reviewed-by: David Hildenbrand david@redhat.com
On Fri, Jun 02, 2023 at 07:15:53PM -0700, John Hubbard wrote:
The uffd tests generate two compile time warnings from clang's -Wformat-security setting. These trigger at the call sites for uffd_test_start() and uffd_test_skip().
- Fix the uffd_test_start() issue by removing the intermediate
test_name variable (thanks to David Hildenbrand for showing how to do this).
- Fix the uffd_test_skip() issue by observing that there is no need for
a macro and a variable args approach, because all callers of uffd_test_skip() pass in a simple char* string, without any format specifiers. So just change uffd_test_skip() into a regular C function.
Cc: David Hildenbrand david@redhat.com Cc: Peter Xu peterx@redhat.com Signed-off-by: John Hubbard jhubbard@nvidia.com
Reviewed-by: Peter Xu peterx@redhat.com
Thanks,
This fixes a real bug, too, because xstate_size() was assuming that the stack variable xstate_size was initialized to zero. That's not guaranteed nor even especially likely.
Reviewed-by: David Hildenbrand david@redhat.com Cc: Peter Xu peterx@redhat.com Signed-off-by: John Hubbard jhubbard@nvidia.com --- tools/testing/selftests/mm/pkey-x86.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/testing/selftests/mm/pkey-x86.h b/tools/testing/selftests/mm/pkey-x86.h index 72c14cd3ddc7..e32ae8a1cd99 100644 --- a/tools/testing/selftests/mm/pkey-x86.h +++ b/tools/testing/selftests/mm/pkey-x86.h @@ -132,7 +132,7 @@ int pkey_reg_xstate_offset(void) unsigned int ecx; unsigned int edx; int xstate_offset; - int xstate_size; + int xstate_size = 0; unsigned long XSTATE_CPUID = 0xd; int leaf;
MADV_PAGEOUT, MADV_POPULATE_READ, MADV_COLLAPSE are conditionally defined as necessary. However, that was being done in .c files, and a new build failure came up that would have been automatically avoided had these been in a common header file.
So consolidate and move them all to vm_util.h, which fixes the build failure.
An alternative approach from Muhammad Usama Anjum was: rely on "make headers" being required, and include asm-generic/mman-common.h. This works in the sense that it builds, but it still generates warnings about duplicate MADV_* symbols, and the goal here is to get a fully clean (no warnings) build here.
Reviewed-by: David Hildenbrand david@redhat.com Cc: Peter Xu peterx@redhat.com Cc: Muhammad Usama Anjum usama.anjum@collabora.com Signed-off-by: John Hubbard jhubbard@nvidia.com --- tools/testing/selftests/mm/cow.c | 7 ------- tools/testing/selftests/mm/khugepaged.c | 10 ---------- tools/testing/selftests/mm/vm_util.h | 10 ++++++++++ 3 files changed, 10 insertions(+), 17 deletions(-)
diff --git a/tools/testing/selftests/mm/cow.c b/tools/testing/selftests/mm/cow.c index dc9d6fe86028..8882b05ec9c8 100644 --- a/tools/testing/selftests/mm/cow.c +++ b/tools/testing/selftests/mm/cow.c @@ -30,13 +30,6 @@ #include "../kselftest.h" #include "vm_util.h"
-#ifndef MADV_PAGEOUT -#define MADV_PAGEOUT 21 -#endif -#ifndef MADV_COLLAPSE -#define MADV_COLLAPSE 25 -#endif - static size_t pagesize; static int pagemap_fd; static size_t thpsize; diff --git a/tools/testing/selftests/mm/khugepaged.c b/tools/testing/selftests/mm/khugepaged.c index 97adc0f34f9c..e88ee039d0eb 100644 --- a/tools/testing/selftests/mm/khugepaged.c +++ b/tools/testing/selftests/mm/khugepaged.c @@ -22,16 +22,6 @@
#include "vm_util.h"
-#ifndef MADV_PAGEOUT -#define MADV_PAGEOUT 21 -#endif -#ifndef MADV_POPULATE_READ -#define MADV_POPULATE_READ 22 -#endif -#ifndef MADV_COLLAPSE -#define MADV_COLLAPSE 25 -#endif - #define BASE_ADDR ((void *)(1UL << 30)) static unsigned long hpage_pmd_size; static unsigned long page_size; diff --git a/tools/testing/selftests/mm/vm_util.h b/tools/testing/selftests/mm/vm_util.h index b950bd16083a..07f39ed2efba 100644 --- a/tools/testing/selftests/mm/vm_util.h +++ b/tools/testing/selftests/mm/vm_util.h @@ -63,3 +63,13 @@ int uffd_register_with_ioctls(int uffd, void *addr, uint64_t len,
#define PAGEMAP_PRESENT(ent) (((ent) & (1ull << 63)) != 0) #define PAGEMAP_PFN(ent) ((ent) & ((1ull << 55) - 1)) + +#ifndef MADV_PAGEOUT +#define MADV_PAGEOUT 21 +#endif +#ifndef MADV_POPULATE_READ +#define MADV_POPULATE_READ 22 +#endif +#ifndef MADV_COLLAPSE +#define MADV_COLLAPSE 25 +#endif
Correction: due to refactoring things a bit in v2, this patch's subject line is no longer accurate. It should be changed to:
[PATCH v2 08/11] selftests/mm: fix build failures due to missing MADV_COLLAPSE
thanks,
This is in preparation for linking test programs with both vm_utils.c and uffd-common.c. The static inline routines, while normally not a problem, in this case complicate an already fragile header file situation: the header files including other header files leads to compilation failures in a subsequent patch that moves code around.
Anyway, there is no particular need for inlining here, so turn these into normal functions, as a workaround to avoid refactoring the header file includes for now.
Cc: David Hildenbrand david@redhat.com Cc: Peter Xu peterx@redhat.com Signed-off-by: John Hubbard jhubbard@nvidia.com --- tools/testing/selftests/mm/vm_util.c | 14 ++++++++++++++ tools/testing/selftests/mm/vm_util.h | 16 ++-------------- 2 files changed, 16 insertions(+), 14 deletions(-)
diff --git a/tools/testing/selftests/mm/vm_util.c b/tools/testing/selftests/mm/vm_util.c index 9b06a5034808..01296c17df02 100644 --- a/tools/testing/selftests/mm/vm_util.c +++ b/tools/testing/selftests/mm/vm_util.c @@ -301,3 +301,17 @@ int uffd_get_features(uint64_t *features)
return 0; } + +unsigned int psize(void) +{ + if (!__page_size) + __page_size = sysconf(_SC_PAGESIZE); + return __page_size; +} + +unsigned int pshift(void) +{ + if (!__page_shift) + __page_shift = (ffsl(psize()) - 1); + return __page_shift; +} diff --git a/tools/testing/selftests/mm/vm_util.h b/tools/testing/selftests/mm/vm_util.h index 07f39ed2efba..8aa543a3678b 100644 --- a/tools/testing/selftests/mm/vm_util.h +++ b/tools/testing/selftests/mm/vm_util.h @@ -3,7 +3,6 @@ #include <stdbool.h> #include <sys/mman.h> #include <err.h> -#include <string.h> /* ffsl() */ #include <unistd.h> /* _SC_PAGESIZE */
#define BIT_ULL(nr) (1ULL << (nr)) @@ -17,19 +16,8 @@ extern unsigned int __page_size; extern unsigned int __page_shift;
-static inline unsigned int psize(void) -{ - if (!__page_size) - __page_size = sysconf(_SC_PAGESIZE); - return __page_size; -} - -static inline unsigned int pshift(void) -{ - if (!__page_shift) - __page_shift = (ffsl(psize()) - 1); - return __page_shift; -} +unsigned int psize(void); +unsigned int pshift(void);
uint64_t pagemap_get_entry(int fd, char *start); bool pagemap_is_softdirty(int fd, char *start);
Move the uffd*() routines to their natural home. Note that ksm_functional_tests.c also depend, intentionally (due to a recent commit [1]), upon uffd-common.[ch].
Cc: David Hildenbrand david@redhat.com Cc: Peter Xu peterx@redhat.com Signed-off-by: John Hubbard jhubbard@nvidia.com --- tools/testing/selftests/mm/Makefile | 7 +- tools/testing/selftests/mm/hugepage-mremap.c | 2 +- .../selftests/mm/ksm_functional_tests.c | 2 +- tools/testing/selftests/mm/uffd-common.c | 105 ++++++++++++++++++ tools/testing/selftests/mm/uffd-common.h | 12 +- tools/testing/selftests/mm/vm_util.c | 104 ----------------- tools/testing/selftests/mm/vm_util.h | 10 -- 7 files changed, 122 insertions(+), 120 deletions(-)
diff --git a/tools/testing/selftests/mm/Makefile b/tools/testing/selftests/mm/Makefile index 23af4633f0f4..a15572758954 100644 --- a/tools/testing/selftests/mm/Makefile +++ b/tools/testing/selftests/mm/Makefile @@ -109,8 +109,11 @@ include ../lib.mk
$(TEST_GEN_PROGS): vm_util.c
-$(OUTPUT)/uffd-stress: uffd-common.c -$(OUTPUT)/uffd-unit-tests: uffd-common.c +$(OUTPUT)/uffd-stress: uffd-common.c +$(OUTPUT)/uffd-unit-tests: uffd-common.c +$(OUTPUT)/hugepage-mremap: uffd-common.c +$(OUTPUT)/write_to_hugetlbfs: uffd-common.c +$(OUTPUT)/ksm_functional_tests: uffd-common.c
ifeq ($(MACHINE),x86_64) BINARIES_32 := $(patsubst %,$(OUTPUT)/%,$(BINARIES_32)) diff --git a/tools/testing/selftests/mm/hugepage-mremap.c b/tools/testing/selftests/mm/hugepage-mremap.c index cabd0084f57b..8158fe909f5e 100644 --- a/tools/testing/selftests/mm/hugepage-mremap.c +++ b/tools/testing/selftests/mm/hugepage-mremap.c @@ -24,7 +24,7 @@ #include <sys/ioctl.h> #include <string.h> #include <stdbool.h> -#include "vm_util.h" +#include "uffd-common.h"
#define DEFAULT_LENGTH_MB 10UL #define MB_TO_BYTES(x) (x * 1024 * 1024) diff --git a/tools/testing/selftests/mm/ksm_functional_tests.c b/tools/testing/selftests/mm/ksm_functional_tests.c index 26853badae70..648188ad73fa 100644 --- a/tools/testing/selftests/mm/ksm_functional_tests.c +++ b/tools/testing/selftests/mm/ksm_functional_tests.c @@ -22,7 +22,7 @@ #include <linux/userfaultfd.h>
#include "../kselftest.h" -#include "vm_util.h" +#include "uffd-common.h"
#define KiB 1024u #define MiB (1024 * KiB) diff --git a/tools/testing/selftests/mm/uffd-common.c b/tools/testing/selftests/mm/uffd-common.c index 61c6250adf93..e1ad63668a05 100644 --- a/tools/testing/selftests/mm/uffd-common.c +++ b/tools/testing/selftests/mm/uffd-common.c @@ -6,6 +6,7 @@ */
#include "uffd-common.h" +#include "vm_util.h"
#define BASE_PMD_ADDR ((void *)(1UL << 30))
@@ -616,3 +617,107 @@ int copy_page(int ufd, unsigned long offset, bool wp) { return __copy_page(ufd, offset, false, wp); } + +/* If `ioctls' non-NULL, the allowed ioctls will be returned into the var */ +int uffd_register_with_ioctls(int uffd, void *addr, uint64_t len, + bool miss, bool wp, bool minor, uint64_t *ioctls) +{ + struct uffdio_register uffdio_register = { 0 }; + uint64_t mode = 0; + int ret = 0; + + if (miss) + mode |= UFFDIO_REGISTER_MODE_MISSING; + if (wp) + mode |= UFFDIO_REGISTER_MODE_WP; + if (minor) + mode |= UFFDIO_REGISTER_MODE_MINOR; + + uffdio_register.range.start = (unsigned long)addr; + uffdio_register.range.len = len; + uffdio_register.mode = mode; + + if (ioctl(uffd, UFFDIO_REGISTER, &uffdio_register) == -1) + ret = -errno; + else if (ioctls) + *ioctls = uffdio_register.ioctls; + + return ret; +} + +int uffd_register(int uffd, void *addr, uint64_t len, + bool miss, bool wp, bool minor) +{ + return uffd_register_with_ioctls(uffd, addr, len, + miss, wp, minor, NULL); +} + +int uffd_unregister(int uffd, void *addr, uint64_t len) +{ + struct uffdio_range range = { .start = (uintptr_t)addr, .len = len }; + int ret = 0; + + if (ioctl(uffd, UFFDIO_UNREGISTER, &range) == -1) + ret = -errno; + + return ret; +} + +int uffd_open_dev(unsigned int flags) +{ + int fd, uffd; + + fd = open("/dev/userfaultfd", O_RDWR | O_CLOEXEC); + if (fd < 0) + return fd; + uffd = ioctl(fd, USERFAULTFD_IOC_NEW, flags); + close(fd); + + return uffd; +} + +int uffd_open_sys(unsigned int flags) +{ +#ifdef __NR_userfaultfd + return syscall(__NR_userfaultfd, flags); +#else + return -1; +#endif +} + +int uffd_open(unsigned int flags) +{ + int uffd = uffd_open_sys(flags); + + if (uffd < 0) + uffd = uffd_open_dev(flags); + + return uffd; +} + +int uffd_get_features(uint64_t *features) +{ + struct uffdio_api uffdio_api = { .api = UFFD_API, .features = 0 }; + /* + * This should by default work in most kernels; the feature list + * will be the same no matter what we pass in here. + */ + int fd = uffd_open(UFFD_USER_MODE_ONLY); + + if (fd < 0) + /* Maybe the kernel is older than user-only mode? */ + fd = uffd_open(0); + + if (fd < 0) + return fd; + + if (ioctl(fd, UFFDIO_API, &uffdio_api)) { + close(fd); + return -errno; + } + + *features = uffdio_api.features; + close(fd); + + return 0; +} diff --git a/tools/testing/selftests/mm/uffd-common.h b/tools/testing/selftests/mm/uffd-common.h index 6068f2346b86..a1cdb78c0762 100644 --- a/tools/testing/selftests/mm/uffd-common.h +++ b/tools/testing/selftests/mm/uffd-common.h @@ -19,8 +19,6 @@ #include <signal.h> #include <poll.h> #include <string.h> -#include <linux/mman.h> -#include <sys/mman.h> #include <sys/syscall.h> #include <sys/ioctl.h> #include <sys/wait.h> @@ -110,6 +108,16 @@ int __copy_page(int ufd, unsigned long offset, bool retry, bool wp); int copy_page(int ufd, unsigned long offset, bool wp); void *uffd_poll_thread(void *arg);
+int uffd_register(int uffd, void *addr, uint64_t len, + bool miss, bool wp, bool minor); +int uffd_unregister(int uffd, void *addr, uint64_t len); +int uffd_open_dev(unsigned int flags); +int uffd_open_sys(unsigned int flags); +int uffd_open(unsigned int flags); +int uffd_get_features(uint64_t *features); +int uffd_register_with_ioctls(int uffd, void *addr, uint64_t len, + bool miss, bool wp, bool minor, uint64_t *ioctls); + #define TEST_ANON 1 #define TEST_HUGETLB 2 #define TEST_SHMEM 3 diff --git a/tools/testing/selftests/mm/vm_util.c b/tools/testing/selftests/mm/vm_util.c index 01296c17df02..c64a0134f83c 100644 --- a/tools/testing/selftests/mm/vm_util.c +++ b/tools/testing/selftests/mm/vm_util.c @@ -198,110 +198,6 @@ unsigned long default_huge_page_size(void) return hps; }
-/* If `ioctls' non-NULL, the allowed ioctls will be returned into the var */ -int uffd_register_with_ioctls(int uffd, void *addr, uint64_t len, - bool miss, bool wp, bool minor, uint64_t *ioctls) -{ - struct uffdio_register uffdio_register = { 0 }; - uint64_t mode = 0; - int ret = 0; - - if (miss) - mode |= UFFDIO_REGISTER_MODE_MISSING; - if (wp) - mode |= UFFDIO_REGISTER_MODE_WP; - if (minor) - mode |= UFFDIO_REGISTER_MODE_MINOR; - - uffdio_register.range.start = (unsigned long)addr; - uffdio_register.range.len = len; - uffdio_register.mode = mode; - - if (ioctl(uffd, UFFDIO_REGISTER, &uffdio_register) == -1) - ret = -errno; - else if (ioctls) - *ioctls = uffdio_register.ioctls; - - return ret; -} - -int uffd_register(int uffd, void *addr, uint64_t len, - bool miss, bool wp, bool minor) -{ - return uffd_register_with_ioctls(uffd, addr, len, - miss, wp, minor, NULL); -} - -int uffd_unregister(int uffd, void *addr, uint64_t len) -{ - struct uffdio_range range = { .start = (uintptr_t)addr, .len = len }; - int ret = 0; - - if (ioctl(uffd, UFFDIO_UNREGISTER, &range) == -1) - ret = -errno; - - return ret; -} - -int uffd_open_dev(unsigned int flags) -{ - int fd, uffd; - - fd = open("/dev/userfaultfd", O_RDWR | O_CLOEXEC); - if (fd < 0) - return fd; - uffd = ioctl(fd, USERFAULTFD_IOC_NEW, flags); - close(fd); - - return uffd; -} - -int uffd_open_sys(unsigned int flags) -{ -#ifdef __NR_userfaultfd - return syscall(__NR_userfaultfd, flags); -#else - return -1; -#endif -} - -int uffd_open(unsigned int flags) -{ - int uffd = uffd_open_sys(flags); - - if (uffd < 0) - uffd = uffd_open_dev(flags); - - return uffd; -} - -int uffd_get_features(uint64_t *features) -{ - struct uffdio_api uffdio_api = { .api = UFFD_API, .features = 0 }; - /* - * This should by default work in most kernels; the feature list - * will be the same no matter what we pass in here. - */ - int fd = uffd_open(UFFD_USER_MODE_ONLY); - - if (fd < 0) - /* Maybe the kernel is older than user-only mode? */ - fd = uffd_open(0); - - if (fd < 0) - return fd; - - if (ioctl(fd, UFFDIO_API, &uffdio_api)) { - close(fd); - return -errno; - } - - *features = uffdio_api.features; - close(fd); - - return 0; -} - unsigned int psize(void) { if (!__page_size) diff --git a/tools/testing/selftests/mm/vm_util.h b/tools/testing/selftests/mm/vm_util.h index 8aa543a3678b..f04f82771cd0 100644 --- a/tools/testing/selftests/mm/vm_util.h +++ b/tools/testing/selftests/mm/vm_util.h @@ -33,16 +33,6 @@ bool check_huge_shmem(void *addr, int nr_hpages, uint64_t hpage_size); int64_t allocate_transhuge(void *ptr, int pagemap_fd); unsigned long default_huge_page_size(void);
-int uffd_register(int uffd, void *addr, uint64_t len, - bool miss, bool wp, bool minor); -int uffd_unregister(int uffd, void *addr, uint64_t len); -int uffd_open_dev(unsigned int flags); -int uffd_open_sys(unsigned int flags); -int uffd_open(unsigned int flags); -int uffd_get_features(uint64_t *features); -int uffd_register_with_ioctls(int uffd, void *addr, uint64_t len, - bool miss, bool wp, bool minor, uint64_t *ioctls); - /* * On ppc64 this will only work with radix 2M hugepage size */
On 03.06.23 04:15, John Hubbard wrote:
Move the uffd*() routines to their natural home. Note that ksm_functional_tests.c also depend, intentionally (due to a recent commit [1]), upon uffd-common.[ch].
Cc: David Hildenbrand david@redhat.com Cc: Peter Xu peterx@redhat.com Signed-off-by: John Hubbard jhubbard@nvidia.com
Acked-by: David Hildenbrand david@redhat.com
On Fri, Jun 02, 2023 at 07:15:57PM -0700, John Hubbard wrote:
Move the uffd*() routines to their natural home. Note that ksm_functional_tests.c also depend, intentionally (due to a recent commit [1]), upon uffd-common.[ch].
Cc: David Hildenbrand david@redhat.com Cc: Peter Xu peterx@redhat.com Signed-off-by: John Hubbard jhubbard@nvidia.com
tools/testing/selftests/mm/Makefile | 7 +- tools/testing/selftests/mm/hugepage-mremap.c | 2 +- .../selftests/mm/ksm_functional_tests.c | 2 +- tools/testing/selftests/mm/uffd-common.c | 105 ++++++++++++++++++ tools/testing/selftests/mm/uffd-common.h | 12 +- tools/testing/selftests/mm/vm_util.c | 104 ----------------- tools/testing/selftests/mm/vm_util.h | 10 -- 7 files changed, 122 insertions(+), 120 deletions(-)
diff --git a/tools/testing/selftests/mm/Makefile b/tools/testing/selftests/mm/Makefile index 23af4633f0f4..a15572758954 100644 --- a/tools/testing/selftests/mm/Makefile +++ b/tools/testing/selftests/mm/Makefile @@ -109,8 +109,11 @@ include ../lib.mk $(TEST_GEN_PROGS): vm_util.c -$(OUTPUT)/uffd-stress: uffd-common.c -$(OUTPUT)/uffd-unit-tests: uffd-common.c +$(OUTPUT)/uffd-stress: uffd-common.c +$(OUTPUT)/uffd-unit-tests: uffd-common.c +$(OUTPUT)/hugepage-mremap: uffd-common.c +$(OUTPUT)/write_to_hugetlbfs: uffd-common.c +$(OUTPUT)/ksm_functional_tests: uffd-common.c
Sorry, John, I still cannot follow..
As I said before uffd-common.[ch] was for uffd stress/unit tests. I confess my fault to not have named it uffd-test-common.[ch] already.
I think it's fine to keep uffd_*() helpers in vm_util.[ch] for now, until it grows. Just like if one day we'll have a pagemap.c test we don't necessary need to move pagemap_*() helpers from vm_utils.[ch] into pagemap.[ch]. It just keeps common test helpers.
Can we avoid linking those into other tests in whatever way? Maybe renaming it to uffd-test-common.[ch] may be cleaner?
On 6/5/23 08:59, Peter Xu wrote: ...
-$(OUTPUT)/uffd-stress: uffd-common.c -$(OUTPUT)/uffd-unit-tests: uffd-common.c +$(OUTPUT)/uffd-stress: uffd-common.c +$(OUTPUT)/uffd-unit-tests: uffd-common.c +$(OUTPUT)/hugepage-mremap: uffd-common.c +$(OUTPUT)/write_to_hugetlbfs: uffd-common.c +$(OUTPUT)/ksm_functional_tests: uffd-common.c
Sorry, John, I still cannot follow..
As I said before uffd-common.[ch] was for uffd stress/unit tests. I confess my fault to not have named it uffd-test-common.[ch] already.
Actually, given that there is nothing *except* test code in this directory, I think your original choice of file names is just right.
I think it's fine to keep uffd_*() helpers in vm_util.[ch] for now, until it grows. Just like if one day we'll have a pagemap.c test we don't necessary need to move pagemap_*() helpers from vm_utils.[ch] into pagemap.[ch]. It just keeps common test helpers.
Can we avoid linking those into other tests in whatever way? Maybe renaming it to uffd-test-common.[ch] may be cleaner?
It sounds like you are suggesting this:
$(OUTPUT)/uffd-stress: uffd-common.c uffd-test-common.c $(OUTPUT)/uffd-unit-tests: uffd-common.c uffd-test-common.c $(OUTPUT)/hugepage-mremap: uffd-test-common.c $(OUTPUT)/write_to_hugetlbfs: uffd-test-common.c $(OUTPUT)/ksm_functional_tests: uffd-test-common.c
...approximately. Do I have that correct? I can arrange it that way if you feel it's a better end result. (And it's better than leaving uffd*() helpers in vm_utils, imho.)
thanks,
On Mon, Jun 05, 2023 at 12:09:56PM -0700, John Hubbard wrote:
On 6/5/23 08:59, Peter Xu wrote: ...
-$(OUTPUT)/uffd-stress: uffd-common.c -$(OUTPUT)/uffd-unit-tests: uffd-common.c +$(OUTPUT)/uffd-stress: uffd-common.c +$(OUTPUT)/uffd-unit-tests: uffd-common.c +$(OUTPUT)/hugepage-mremap: uffd-common.c +$(OUTPUT)/write_to_hugetlbfs: uffd-common.c +$(OUTPUT)/ksm_functional_tests: uffd-common.c
Sorry, John, I still cannot follow..
As I said before uffd-common.[ch] was for uffd stress/unit tests. I confess my fault to not have named it uffd-test-common.[ch] already.
Actually, given that there is nothing *except* test code in this directory, I think your original choice of file names is just right.
I think it's fine to keep uffd_*() helpers in vm_util.[ch] for now, until it grows. Just like if one day we'll have a pagemap.c test we don't necessary need to move pagemap_*() helpers from vm_utils.[ch] into pagemap.[ch]. It just keeps common test helpers.
Can we avoid linking those into other tests in whatever way? Maybe renaming it to uffd-test-common.[ch] may be cleaner?
It sounds like you are suggesting this:
$(OUTPUT)/uffd-stress: uffd-common.c uffd-test-common.c $(OUTPUT)/uffd-unit-tests: uffd-common.c uffd-test-common.c $(OUTPUT)/hugepage-mremap: uffd-test-common.c $(OUTPUT)/write_to_hugetlbfs: uffd-test-common.c $(OUTPUT)/ksm_functional_tests: uffd-test-common.c
...approximately. Do I have that correct? I can arrange it that way if you feel it's a better end result. (And it's better than leaving uffd*() helpers in vm_utils, imho.)
Yes, as long as we don't link (especially) the uffd test specific globals into non-uffd test programs I'll have no issue. Thanks.
On 6/5/23 12:24, Peter Xu wrote: ...
It sounds like you are suggesting this:
$(OUTPUT)/uffd-stress: uffd-common.c uffd-test-common.c $(OUTPUT)/uffd-unit-tests: uffd-common.c uffd-test-common.c $(OUTPUT)/hugepage-mremap: uffd-test-common.c $(OUTPUT)/write_to_hugetlbfs: uffd-test-common.c $(OUTPUT)/ksm_functional_tests: uffd-test-common.c
...approximately. Do I have that correct? I can arrange it that way if you feel it's a better end result. (And it's better than leaving uffd*() helpers in vm_utils, imho.)
Yes, as long as we don't link (especially) the uffd test specific globals into non-uffd test programs I'll have no issue. Thanks.
OK very good. I'll put that together.
thanks,
As per a discussion with Muhammad Usama Anjum [1], the following is how one is supposed to build selftests:
make headers && make -C tools/testing/selftests/mm
However, that's not yet documented anywhere. So add it to Documentation/dev-tools/kselftest.rst .
[1] https://lore.kernel.org/all/bf910fa5-0c96-3707-cce4-5bcc656b6274@collabora.c...
Cc: David Hildenbrand david@redhat.com Cc: Peter Xu peterx@redhat.com Cc: Muhammad Usama Anjum usama.anjum@collabora.com Cc: Jonathan Corbet corbet@lwn.net Cc: linux-doc@vger.kernel.org Signed-off-by: John Hubbard jhubbard@nvidia.com --- Documentation/dev-tools/kselftest.rst | 1 + 1 file changed, 1 insertion(+)
diff --git a/Documentation/dev-tools/kselftest.rst b/Documentation/dev-tools/kselftest.rst index 12b575b76b20..6e35d042199c 100644 --- a/Documentation/dev-tools/kselftest.rst +++ b/Documentation/dev-tools/kselftest.rst @@ -36,6 +36,7 @@ Running the selftests (hotplug tests are run in limited mode)
To build the tests::
+ $ make headers $ make -C tools/testing/selftests
To run the tests::
On 03.06.23 04:15, John Hubbard wrote:
As per a discussion with Muhammad Usama Anjum [1], the following is how one is supposed to build selftests:
make headers && make -C tools/testing/selftests/mmHowever, that's not yet documented anywhere. So add it to Documentation/dev-tools/kselftest.rst .
[1] https://lore.kernel.org/all/bf910fa5-0c96-3707-cce4-5bcc656b6274@collabora.c...
Cc: David Hildenbrand david@redhat.com Cc: Peter Xu peterx@redhat.com Cc: Muhammad Usama Anjum usama.anjum@collabora.com Cc: Jonathan Corbet corbet@lwn.net Cc: linux-doc@vger.kernel.org Signed-off-by: John Hubbard jhubbard@nvidia.com
Documentation/dev-tools/kselftest.rst | 1 + 1 file changed, 1 insertion(+)
diff --git a/Documentation/dev-tools/kselftest.rst b/Documentation/dev-tools/kselftest.rst index 12b575b76b20..6e35d042199c 100644 --- a/Documentation/dev-tools/kselftest.rst +++ b/Documentation/dev-tools/kselftest.rst @@ -36,6 +36,7 @@ Running the selftests (hotplug tests are run in limited mode) To build the tests::
- $ make headers $ make -C tools/testing/selftests
To run the tests::
Reviewed-by: David Hildenbrand david@redhat.com
linux-kselftest-mirror@lists.linaro.org
-
David Hildenbrand -
John Hubbard -
Peter Xu