| Message ID | 20231004171127.106056-2-leitao@debian.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [1/2] selftests/mm: export get_free_hugepages() | expand |
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!
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"
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