get_free_hugepages() is helpful for other hugepage tests. Export it to the common file (vm_util.c) to be reused.
Signed-off-by: Breno Leitao leitao@debian.org --- tools/testing/selftests/mm/hugetlb-madvise.c | 19 ------------------- tools/testing/selftests/mm/vm_util.c | 19 +++++++++++++++++++ tools/testing/selftests/mm/vm_util.h | 1 + 3 files changed, 20 insertions(+), 19 deletions(-)
diff --git a/tools/testing/selftests/mm/hugetlb-madvise.c b/tools/testing/selftests/mm/hugetlb-madvise.c index d55322df4b73..f32d99565c5e 100644 --- a/tools/testing/selftests/mm/hugetlb-madvise.c +++ b/tools/testing/selftests/mm/hugetlb-madvise.c @@ -36,25 +36,6 @@ unsigned long huge_page_size; unsigned long base_page_size;
-unsigned long get_free_hugepages(void) -{ - unsigned long fhp = 0; - char *line = NULL; - size_t linelen = 0; - FILE *f = fopen("/proc/meminfo", "r"); - - if (!f) - return fhp; - while (getline(&line, &linelen, f) > 0) { - if (sscanf(line, "HugePages_Free: %lu", &fhp) == 1) - break; - } - - free(line); - fclose(f); - return fhp; -} - void write_fault_pages(void *addr, unsigned long nr_pages) { unsigned long i; diff --git a/tools/testing/selftests/mm/vm_util.c b/tools/testing/selftests/mm/vm_util.c index 558c9cd8901c..3082b40492dd 100644 --- a/tools/testing/selftests/mm/vm_util.c +++ b/tools/testing/selftests/mm/vm_util.c @@ -269,3 +269,22 @@ int uffd_unregister(int uffd, void *addr, uint64_t len)
return ret; } + +unsigned long get_free_hugepages(void) +{ + unsigned long fhp = 0; + char *line = NULL; + size_t linelen = 0; + FILE *f = fopen("/proc/meminfo", "r"); + + if (!f) + return fhp; + while (getline(&line, &linelen, f) > 0) { + if (sscanf(line, "HugePages_Free: %lu", &fhp) == 1) + break; + } + + free(line); + fclose(f); + return fhp; +} diff --git a/tools/testing/selftests/mm/vm_util.h b/tools/testing/selftests/mm/vm_util.h index c7fa61f0dff8..c02990bbd56f 100644 --- a/tools/testing/selftests/mm/vm_util.h +++ b/tools/testing/selftests/mm/vm_util.h @@ -51,6 +51,7 @@ int uffd_register(int uffd, void *addr, uint64_t len, int uffd_unregister(int uffd, void *addr, uint64_t len); int uffd_register_with_ioctls(int uffd, void *addr, uint64_t len, bool miss, bool wp, bool minor, uint64_t *ioctls); +unsigned long get_free_hugepages(void);
/* * On ppc64 this will only work with radix 2M hugepage size
Create a selftest that exercises the conflict between page faults and madvise(MADV_DONTNEED) in the same huge page. Do it by running two threads that touches the huge page and madvise(MADV_DONTNEED) at the same time.
In case of a SIGBUS coming at pagefault, the test should fail, since we hit the bug.
The test doesn't have a signal handler, and if it fails, it fails like the following
---------------------------------- running ./hugetlb_fault_after_madv ---------------------------------- ./run_vmtests.sh: line 186: 595563 Bus error (core dumped) "$@" [FAIL]
This selftest goes together with the fix of the bug[1] itself.
[1] https://lore.kernel.org/all/20231001005659.2185316-1-riel@surriel.com/#r
Signed-off-by: Breno Leitao leitao@debian.org --- tools/testing/selftests/mm/Makefile | 1 + .../selftests/mm/hugetlb_fault_after_madv.c | 82 +++++++++++++++++++ tools/testing/selftests/mm/run_vmtests.sh | 4 + 3 files changed, 87 insertions(+) create mode 100644 tools/testing/selftests/mm/hugetlb_fault_after_madv.c
diff --git a/tools/testing/selftests/mm/Makefile b/tools/testing/selftests/mm/Makefile index 6a9fc5693145..e71ec9910c62 100644 --- a/tools/testing/selftests/mm/Makefile +++ b/tools/testing/selftests/mm/Makefile @@ -68,6 +68,7 @@ TEST_GEN_FILES += split_huge_page_test TEST_GEN_FILES += ksm_tests TEST_GEN_FILES += ksm_functional_tests TEST_GEN_FILES += mdwe_test +TEST_GEN_FILES += hugetlb_fault_after_madv
ifneq ($(ARCH),arm64) TEST_GEN_PROGS += soft-dirty diff --git a/tools/testing/selftests/mm/hugetlb_fault_after_madv.c b/tools/testing/selftests/mm/hugetlb_fault_after_madv.c new file mode 100644 index 000000000000..d6d38d443840 --- /dev/null +++ b/tools/testing/selftests/mm/hugetlb_fault_after_madv.c @@ -0,0 +1,82 @@ +// SPDX-License-Identifier: GPL-2.0 +#include <pthread.h> +#include <stdio.h> +#include <stdlib.h> +#include <sys/mman.h> +#include <sys/types.h> +#include <unistd.h> + +#include "vm_util.h" + +#define MMAP_SIZE (1 << 21) +#define INLOOP_ITER 100 + +char *huge_ptr; + +/* Touch the memory while it is being madvised() */ +void *touch(void *unused) +{ + char *ptr = (char *)huge_ptr; + + if (!ptr) { + fprintf(stderr, "Failed to allocate memory\n"); + perror(""); + } + + for (int i = 0; i < INLOOP_ITER; i++) + ptr[0] = '.'; + + return NULL; +} + +void *madv(void *unused) +{ + usleep(rand() % 10); + if (!huge_ptr) + return NULL; + + for (int i = 0; i < INLOOP_ITER; i++) + madvise(huge_ptr, MMAP_SIZE, MADV_DONTNEED); + + return NULL; +} + +int main(void) +{ + unsigned long free_hugepages; + pthread_t thread1, thread2; + /* + * On kernel 6.4, we are able to reproduce the problem with ~1000 + * interactions + */ + int max = 10000; + + srand(getpid()); + + free_hugepages = get_free_hugepages(); + if (free_hugepages != 1) { + fprintf(stderr, + "This test needs one and only one page to execute. Got %lu\n", + free_hugepages); + exit(1); + } + + while (max--) { + huge_ptr = mmap(NULL, MMAP_SIZE, PROT_READ | PROT_WRITE, + MAP_PRIVATE | MAP_ANONYMOUS | MAP_HUGETLB, -1, 0); + + if ((unsigned long)huge_ptr == -1) { + perror("Failed to allocate\n"); + continue; + } + + pthread_create(&thread1, NULL, madv, NULL); + pthread_create(&thread2, NULL, touch, NULL); + + pthread_join(thread1, NULL); + pthread_join(thread2, NULL); + munmap(huge_ptr, MMAP_SIZE); + } + + return 0; +} diff --git a/tools/testing/selftests/mm/run_vmtests.sh b/tools/testing/selftests/mm/run_vmtests.sh index 3e2bc818d566..9f53f7318a38 100755 --- a/tools/testing/selftests/mm/run_vmtests.sh +++ b/tools/testing/selftests/mm/run_vmtests.sh @@ -221,6 +221,10 @@ CATEGORY="hugetlb" run_test ./hugepage-mremap CATEGORY="hugetlb" run_test ./hugepage-vmemmap CATEGORY="hugetlb" run_test ./hugetlb-madvise
+# For this test, we need one and just one huge page +echo 1 > /proc/sys/vm/nr_hugepages +CATEGORY="hugetlb" run_test ./hugetlb_fault_after_madv + if test_selected "hugetlb"; then echo "NOTE: These hugetlb tests provide minimal coverage. Use" echo " https://github.com/libhugetlbfs/libhugetlbfs.git for"
On Wed, 2023-10-04 at 10:11 -0700, Breno Leitao wrote:
+char *huge_ptr;
+/* Touch the memory while it is being madvised() */ +void *touch(void *unused) +{ + char *ptr = (char *)huge_ptr;
+ if (!ptr) { + fprintf(stderr, "Failed to allocate memory\n"); + perror(""); + }
I'm not sure this error message makes a lot of sense away from where the huge page gets allocated.
+ while (max--) { + huge_ptr = mmap(NULL, MMAP_SIZE, PROT_READ | PROT_WRITE, + MAP_PRIVATE | MAP_ANONYMOUS | MAP_HUGETLB, -1, 0);
+ if ((unsigned long)huge_ptr == -1) { + perror("Failed to allocate\n"); + continue; + }
Should the test case just exit with an error here, when the allocation fails?
Looping around when it cannot get memory seems pointless, but telling the user that the allocation fails, when it should clearly have succeeded could be useful.
This test case certainly seems to do the trick in showing whether the race between MADV_DONTNEED and page faults exists in a particular kernel.
On Wed, Oct 04, 2023 at 08:22:08PM -0400, Rik van Riel wrote:
On Wed, 2023-10-04 at 10:11 -0700, Breno Leitao wrote:
+char *huge_ptr;
+/* Touch the memory while it is being madvised() */ +void *touch(void *unused) +{ + char *ptr = (char *)huge_ptr;
+ if (!ptr) { + fprintf(stderr, "Failed to allocate memory\n"); + perror(""); + }
I'm not sure this error message makes a lot of sense away from where the huge page gets allocated.
Right. I think I don't need this whole "if" clause at all. Let me remove it.
+ while (max--) { + huge_ptr = mmap(NULL, MMAP_SIZE, PROT_READ | PROT_WRITE, + MAP_PRIVATE | MAP_ANONYMOUS | MAP_HUGETLB, -1, 0);
+ if ((unsigned long)huge_ptr == -1) { + perror("Failed to allocate\n"); + continue; + }
Should the test case just exit with an error here, when the allocation fails?
Yes, probably skip the test if we are not able to allocate the memory. I just found I can use something as `ksft_exit_skip()` for this purpose.
Thanks for the great feedbacks!
linux-kselftest-mirror@lists.linaro.org
-
Breno Leitao -
Rik van Riel