selftests/mm: hugetlb_madv_vs_map: add underflow test
Subsystems (3)
PATCH v2 1/1 selftests/mm: hugetlb_madv_vs_map: add underflow test
Add a test that checks for underflows when a parent unmaps the page first. Also check that when the child exits the reserve count is correct.
---
v2: factor out tests
.../testing/selftests/mm/hugepage_settings.c | 9 ++ .../testing/selftests/mm/hugepage_settings.h | 1 + .../selftests/mm/hugetlb_madv_vs_map.c | 107 ++++++++++++++++-- 3 files changed, 109 insertions(+), 8 deletions(-)
tools/testing/selftests/mm/hugepage_settings.c +9 -0
@@ -449,6 +449,15 @@ unsigned long hugetlb_free_pages(unsigned long size)return read_num (path); } +unsigned long hugetlb_nr_resv_pages (unsigned long size) +{ +char path[PATH_MAX ]; + +hugetlb_sysfs_path (path,sizeof (path), size,"resv_hugepages" ); + +return read_num (path); +} +static bool __hugetlb_setup (unsigned long size,unsigned long nr) {unsigned long free =hugetlb_free_pages (size);
tools/testing/selftests/mm/hugepage_settings.h +1 -0
@@ -98,6 +98,7 @@ unsigned long default_huge_page_size(void);unsigned long hugetlb_nr_pages (unsigned long size);void hugetlb_set_nr_pages (unsigned long size,unsigned long nr);unsigned long hugetlb_free_pages (unsigned long size); +unsigned long hugetlb_nr_resv_pages (unsigned long size);static inline void hugetlb_save_settings (void ) {
tools/testing/selftests/mm/hugetlb_madv_vs_map.c +99 -8
137 lines not on this page · raw message
Signature · 1 line
2.39.1
Mike Rapoport
reviewerQuoted 53 lines · Guillaume Morin
Add a test that checks for underflows when a parent unmaps the page first. Also check that when the child exits the reserve count is correct.
Signed-off-by Guillaume Morin <[email protected]Cc Breno Leitao <[email protected]>Cc Mike Rapoport <[email protected]>---
v2: factor out tests.../testing/selftests/mm/hugepage_settings.c | 9 ++ .../testing/selftests/mm/hugepage_settings.h | 1 + .../selftests/mm/hugetlb_madv_vs_map.c | 107 ++++++++++++++++-- 3 files changed, 109 insertions(+), 8 deletions(-)tools/testing/selftests/mm/hugepage_settings.c +9
-0@@ -449,6 +449,15 @@ unsigned long hugetlb_free_pages(unsigned long size)return read_num (path); } +unsigned long hugetlb_nr_resv_pages (unsigned long size) +{ +char path[PATH_MAX ]; + +hugetlb_sysfs_path (path,sizeof (path), size,"resv_hugepages" ); + +return read_num (path); +} +static bool __hugetlb_setup (unsigned long size,unsigned long nr) {unsigned long free =hugetlb_free_pages (size);tools/testing/selftests/mm/hugepage_settings.h +1
-0@@ -98,6 +98,7 @@ unsigned long default_huge_page_size(void);unsigned long hugetlb_nr_pages (unsigned long size);void hugetlb_set_nr_pages (unsigned long size,unsigned long nr);unsigned long hugetlb_free_pages (unsigned long size); +unsigned long hugetlb_nr_resv_pages (unsigned long size);static inline void hugetlb_save_settings (void ) {tools/testing/selftests/mm/hugetlb_madv_vs_map.c +0
-0
tools/testing/selftests/mm/hugetlb_madv_vs_map.c line 16
Quoted 2 lines
tools/testing/selftests/mm/hugetlb_madv_vs_map.c +0
-0@@ -15,13 +15,20 @@* * Touching the first page after thread3's allocation will raise a SIGBUS
The comment above is now the description of test_madv_vs_map()
tools/testing/selftests/mm/hugetlb_madv_vs_map.c line 20
Quoted 4 lines
tools/testing/selftests/mm/hugetlb_madv_vs_map.c +3
-0* +* We setup a 2nd test where we create a child process, then unmap the page in +* the parent while the child waits and verify that there is no underflow +* of the reserved count.
And this one is a description of test_underflow()
Quoted 44 lines
tools/testing/selftests/mm/hugetlb_madv_vs_map.c +8
-8+* * Author: Breno Leitao <[email protected]> */ +#include <limits.h> #include <pthread.h> +#include <signal.h> #include <stdio.h> #include <stdlib.h> #include <sys/mman.h> #include <sys/types.h> +#include <sys/wait.h> #include <unistd.h> #include "vm_util.h" @@ -74,7 +81,7 @@ void *map_extra(void *unused)return NULL ; } -int main (void ) +void test_madv_vs_map (void ) {pthread_t thread1, thread2, thread3;void *ret;@@ -85,13 +92,6 @@ int main(void)*/ int max =10 ; -ksft_print_header (); -ksft_set_plan (1 ); - -if (!hugetlb_setup_default_exact (1 )) -ksft_exit_skip ("This test needs one and only one page to execute. Got %lu\n" , -hugetlb_free_default_pages ()); - mmap_size =default_huge_page_size ();while (max--) {@@ -120,5 +120,96 @@ int main(void) }ksft_test_result_pass ("No unexpected huge page allocations\n" ); +} + +void test_underflow (void )
tools/testing/selftests/mm/hugetlb_madv_vs_map.c int main(void) · line 135
Quoted 10 lines
tools/testing/selftests/mm/hugetlb_madv_vs_map.c +10
-0+{ +pid_t pid; +int pipe_fds[2 ]; +unsigned long nr_reserved =0 ; + + huge_ptr =mmap (NULL , mmap_size,PROT_READ |PROT_WRITE , +MAP_PRIVATE |MAP_ANONYMOUS |MAP_HUGETLB ,-1 ,0 ); + +if ((unsigned long )huge_ptr ==-1 ) +ksft_exit_fail_msg ("Failed to allocate huge page\n" );
ksft_exit_fail_perror()
Quoted 5 lines
tools/testing/selftests/mm/hugetlb_madv_vs_map.c +5
-0+ + nr_reserved =hugetlb_nr_resv_pages (default_huge_page_size ()); +if (nr_reserved !=1 ) +ksft_exit_fail_msg ("Unexpected number of reserved pages: %lu, expected 1\n" , + nr_reserved);
tools/testing/selftests/mm/hugetlb_madv_vs_map.c int main(void) · line 150
Quoted 10 lines
tools/testing/selftests/mm/hugetlb_madv_vs_map.c +10
-0+ +/* Force the fault to ensure the reservation is consumed */ + *huge_ptr =0 ; + nr_reserved =hugetlb_nr_resv_pages (default_huge_page_size ()); +if (nr_reserved !=0 ) +ksft_exit_fail_msg ("Unexpected number of reserved pages: %lu, expected 0\n" , + nr_reserved); + +if (pipe (pipe_fds) !=0 ) +ksft_exit_fail_msg ("pipe failed" );
ksft_exit_fail_perror()
tools/testing/selftests/mm/hugetlb_madv_vs_map.c int main(void) · line 162
Quoted 12 lines
tools/testing/selftests/mm/hugetlb_madv_vs_map.c +12
-0+ + pid =fork (); +if (pid <0 ) +ksft_exit_fail_msg ("fork failed" ); + +if (pid ==0 ) { +/* Child: Simply wait for the parent */ +char b; + +close (pipe_fds[1 ]); +if (read (pipe_fds[0 ], &b,1 ) <0 ) +ksft_exit_fail_msg ("child read failed" );
ksft_exit_fail_perror()
tools/testing/selftests/mm/hugetlb_madv_vs_map.c int main(void) · line 172
Quoted 10 lines
tools/testing/selftests/mm/hugetlb_madv_vs_map.c +10
-0+return ; + } + +/* Parent */ +close (pipe_fds[0 ]); + +/* First unmap, this will close the vma */ +if (munmap (huge_ptr, mmap_size) !=0 ) { +kill (pid,SIGKILL ); +ksft_exit_fail_msg ("munmap failed" );
Won't this leave the child as a zombie?
Quoted 5 lines
tools/testing/selftests/mm/hugetlb_madv_vs_map.c +5
-0+ } + + nr_reserved =hugetlb_nr_resv_pages (default_huge_page_size ()); +if (nr_reserved ==ULONG_MAX ) { +ksft_test_result_fail ("After the munmap, HugePages_Rsvd underflowed!\n" );
tools/testing/selftests/mm/hugetlb_madv_vs_map.c int main(void) · line 187
Quoted 10 lines
tools/testing/selftests/mm/hugetlb_madv_vs_map.c +10
-0+ }else if (nr_reserved ==0 ) { +ksft_test_result_pass ("Underflow not present!\n" ); + }else { +ksft_exit_fail_msg ("Unexpected HugePages_Rsvd=%ld after munmap, should be 0 or -1. Repeat the test\n" , + nr_reserved); + } +/* Make the child exit, this should restore HugePages_Rsvd to 0 */ +if (write (pipe_fds[1 ], &nr_reserved,1 ) <0 ) { +kill (pid,SIGKILL ); +ksft_exit_fail_msg ("write failed" );
Same here.
tools/testing/selftests/mm/hugetlb_madv_vs_map.c int main(void) · line 195
Quoted 8 lines
tools/testing/selftests/mm/hugetlb_madv_vs_map.c +8
-0+ } +close (pipe_fds[1 ]); +if (waitpid (pid,NULL ,0 ) <=0 ) +ksft_exit_fail_msg ("write failed" ); + + nr_reserved =hugetlb_nr_resv_pages (default_huge_page_size ()); +if (nr_reserved ==0 ) { +ksft_test_result_pass ("After the child dies, HugePages_Rsvd is properly set to 0\n" );
This makes this count the test as two, right?
And do you need to re-check what happens after the child exits if you already detected the underflow?
tools/testing/selftests/mm/hugetlb_madv_vs_map.c int main(void) · line 205
Quoted 10 lines
tools/testing/selftests/mm/hugetlb_madv_vs_map.c +10
-0+ }else { +ksft_exit_fail_msg ("Unexpected HugePages_Rsvd=%ld after the child termination munmap, should be 0 or -1. Repeat the test\n" , + nr_reserved); + } +} + +int main (void ) +{ +ksft_print_header (); +ksft_set_plan (3 );
The comment at the header implies we've added the second test, so they plan should be bumped to 2.
Quoted 13 lines
tools/testing/selftests/mm/hugetlb_madv_vs_map.c +8
-0+ +if (!hugetlb_setup_default_exact (1 )) +ksft_exit_skip ("This test needs one and only one page to execute. Got %lu\n" , +hugetlb_free_default_pages ()); + +test_madv_vs_map (); +test_underflow (); +ksft_finished (); }Signature · 1 line
2.39.1
Signature · 2 lines
Sincerely yours, Mike.
Quoted 4 lines · Mike Rapoport
Quoted 4 lines
tools/testing/selftests/mm/hugetlb_madv_vs_map.c +4
-0+return ; + } + +/* Parent */
Quoted 8 lines
tools/testing/selftests/mm/hugetlb_madv_vs_map.c int main(void) · line 172
Quoted 6 lines
tools/testing/selftests/mm/hugetlb_madv_vs_map.c +6
-0+close (pipe_fds[0 ]); + +/* First unmap, this will close the vma */ +if (munmap (huge_ptr, mmap_size) !=0 ) { +kill (pid,SIGKILL ); +ksft_exit_fail_msg ("munmap failed" );Won't this leave the child as a zombie?
It will, for a brief amount of time then re-parented to init which will wait(). Happy to add a waitpid() here but I was concerned it added more complexity for not much benefit (it's very very unlikely munmap will fail). Let me know.
Quoted 10 lines
tools/testing/selftests/mm/hugetlb_madv_vs_map.c int main(void) · line 195
Quoted 8 lines
tools/testing/selftests/mm/hugetlb_madv_vs_map.c +8
-0+ } +close (pipe_fds[1 ]); +if (waitpid (pid,NULL ,0 ) <=0 ) +ksft_exit_fail_msg ("write failed" ); + + nr_reserved =hugetlb_nr_resv_pages (default_huge_page_size ()); +if (nr_reserved ==0 ) { +ksft_test_result_pass ("After the child dies, HugePages_Rsvd is properly set to 0\n" );This makes this count the test as two, right?
Yes, it checks both that there is no underflow but that the count is restorred as well. is that a problem?
Quoted 2 lines
And do you need to re-check what happens after the child exits if you already detected the underflow?
Need is a strong word. It can be done either way really.
Quoted 14 lines
tools/testing/selftests/mm/hugetlb_madv_vs_map.c int main(void) · line 205
Quoted 11 lines
tools/testing/selftests/mm/hugetlb_madv_vs_map.c +10
-0+ }else { +ksft_exit_fail_msg ("Unexpected HugePages_Rsvd=%ld after the child termination munmap, should be 0 or -1. Repeat the test\n" , + nr_reserved); + } +} + +int main (void ) +{ +ksft_print_header (); +ksft_set_plan (3 );The comment at the header implies we've added the second test, so they plan should be bumped to 2.
So you're saying that every test function should only fail or pass once?
Signature · 1 line
Guillaume Morin <[email protected]>
Mike Rapoport
reviewerQuoted 14 lines · Guillaume Morin
Quoted 4 lines · Mike Rapoport
Quoted 4 lines
tools/testing/selftests/mm/hugetlb_madv_vs_map.c +4
-0+return ; + } + +/* Parent */ Quoted 8 lines
tools/testing/selftests/mm/hugetlb_madv_vs_map.c int main(void) · line 172
Quoted 6 lines
tools/testing/selftests/mm/hugetlb_madv_vs_map.c +6
-0+close (pipe_fds[0 ]); + +/* First unmap, this will close the vma */ +if (munmap (huge_ptr, mmap_size) !=0 ) { +kill (pid,SIGKILL ); +ksft_exit_fail_msg ("munmap failed" );Won't this leave the child as a zombie?
Quoted 4 lines
It will, for a brief amount of time then re-parented to init which will
wait(). Happy to add awaitpid()here but I was concerned it added more complexity for not much benefit (it's very very unlikely munmap will fail). Let me know.
Letting init reap the child is fine, but there are other issues with killing the child without wait().
First, kill() deliverers the signal asynchronously, it may happen that the parent exits before the child and calls _atexit() cleaunps that should restore nr_hugepages. But since the child still holds the page the restore won't work.
Right now this is somewhat neutralised because the child returns to main and calls again for cleanup, but that in turn produces spurious output:
# Planned tests != run tests (3 != 1)
# Totals: pass:1 fail:0 xfail:0 xpass:0 skip:0 error:0
The child should call _exit() to avoid going through ksft_finished() and hugetlb restore callbacks.
And since you already have the pipe to synchronize the processes, maybe use that pipe instead of kill?
You can also consider using goto err_cleanup instead of doing cleanup inside if (err)
Quoted 4 lines
Quoted 4 lines
tools/testing/selftests/mm/hugetlb_madv_vs_map.c int main(void) · line 191
Quoted 4 lines
tools/testing/selftests/mm/hugetlb_madv_vs_map.c +4
-0+ } +close (pipe_fds[1 ]); +if (waitpid (pid,NULL ,0 ) <=0 ) +ksft_exit_fail_msg ("write failed" );
waitpid failed?
Quoted 30 lines
Quoted 6 lines
tools/testing/selftests/mm/hugetlb_madv_vs_map.c int main(void) · line 195
Quoted 4 lines
tools/testing/selftests/mm/hugetlb_madv_vs_map.c +4
-0+ + nr_reserved =hugetlb_nr_resv_pages (default_huge_page_size ()); +if (nr_reserved ==0 ) { +ksft_test_result_pass ("After the child dies, HugePages_Rsvd is properly set to 0\n" );This makes this count the test as two, right?
Yes, it checks both that there is no underflow but that the count is restorred as well. is that a problem?
Quoted 2 lines
And do you need to re-check what happens after the child exits if you already detected the underflow?
Need is a strong word. It can be done either way really.
Quoted 14 lines
tools/testing/selftests/mm/hugetlb_madv_vs_map.c int main(void) · line 205
Quoted 11 lines
tools/testing/selftests/mm/hugetlb_madv_vs_map.c +10
-0+ }else { +ksft_exit_fail_msg ("Unexpected HugePages_Rsvd=%ld after the child termination munmap, should be 0 or -1. Repeat the test\n" , + nr_reserved); + } +} + +int main (void ) +{ +ksft_print_header (); +ksft_set_plan (3 );The comment at the header implies we've added the second test, so they plan should be bumped to 2.
Quoted 1 line
So you're saying that every test function should only fail or pass once?
Not necessarily, but comment saying "setup a second test" and the test_underflow() name imply that this is one test rather than two.
Quoted 3 lines
Signature · 1 line
Guillaume Morin <[email protected]>
Signature · 2 lines
Sincerely yours, Mike.