Test for memfd_create() syscall with MFD_HUGETLB flag.
PATCH v2 1/1 Test for memfd_create() syscall with MFD_HUGETLB flag.
memfd_create03.c: Tests the syscall for the flag MFD_HUGETLB.
memfd_create04.c: Tests the syscall for the flags MFD_HUGE_2MB, MFD_HUGE_1GB,... used in conjunction with MFD_HUGETLB flag.
memfd_create_hugetlb.c: Contains helper functions for memfd_create03.c and memfd_create04.c.
-- major changes from v1 to v2 --
Restore the original number of hugepages in cleanup().
Modify check_mfd_new() from memfd_create_common and replace safe_huge_new() + safe_huge_x_new() from memfd_create_hugetlb with it.
Modify get_from_file() from memfd_create_hugetlb to use FILE_LINES_SCANF in code logic.
Move test dependent flags to include/lapi
--- include/lapi/memfd.h | 34 ++++ runtest/syscalls | 2 + .../kernel/syscalls/memfd_create/.gitignore | 2 + .../kernel/syscalls/memfd_create/Makefile | 4 +- .../syscalls/memfd_create/memfd_create03.c | 165 +++++++++++++++++ .../syscalls/memfd_create/memfd_create04.c | 140 ++++++++++++++ .../memfd_create/memfd_create_common.c | 20 +- .../memfd_create/memfd_create_hugetlb.c | 173 ++++++++++++++++++ .../memfd_create/memfd_create_hugetlb.h | 60 ++++++ 9 files changed, 594 insertions(+), 6 deletions(-) create mode 100644 testcases/kernel/syscalls/memfd_create/memfd_create03.c create mode 100644 testcases/kernel/syscalls/memfd_create/memfd_create04.c create mode 100644 testcases/kernel/syscalls/memfd_create/memfd_create_hugetlb.c create mode 100644 testcases/kernel/syscalls/memfd_create/memfd_create_hugetlb.h
include/lapi/memfd.h +34 -0
@@ -24,4 +24,38 @@# define MFD_ALLOW_SEALING 0x0002U#endif +/* flags for memfd_create(3) and memfd_create(4) */ +#ifndef MFD_HUGETLB +#define MFD_HUGETLB 0x0004U +#endif + +#ifndef MFD_HUGE_64KB +#define MFD_HUGE_64KB (16 << 26) +#endif +#ifndef MFD_HUGE_512KB +#define MFD_HUGE_512KB (19 << 26) +#endif +#ifndef MFD_HUGE_2MB +#define MFD_HUGE_2MB (21 << 26) +#endif +#ifndef MFD_HUGE_8MB +#define MFD_HUGE_8MB (23 << 26) +#endif +#ifndef MFD_HUGE_16MB +#define MFD_HUGE_16MB (24 << 26) +#endif +#ifndef MFD_HUGE_256MB +#define MFD_HUGE_256MB (28 << 26) +#endif +#ifndef MFD_HUGE_1GB +#define MFD_HUGE_1GB (30 << 26) +#endif +#ifndef MFD_HUGE_2GB +#define MFD_HUGE_2GB (31 << 26) +#endif +#ifndef MFD_HUGE_16GB +#define MFD_HUGE_16GB (34 << 26) +#endif + +#endif
runtest/syscalls +2 -0
@@ -1479,5 +1479,7 @@ futex_wait_bitset02 futex_wait_bitset02 memfd_create01 memfd_create01 memfd_create02 memfd_create02 +memfd_create03 memfd_create03 +memfd_create04 memfd_create04 copy_file_range01 copy_file_range01
testcases/kernel/syscalls/memfd_create/.gitignore +2 -0
@@ -1,2 +1,4 @@ /memfd_create01 /memfd_create02 +/memfd_create03 +/memfd_create04
testcases/kernel/syscalls/memfd_create/Makefile +2 -2
@@ -16,8 +16,8 @@ top_srcdir ?= ../../../.. include $(top_srcdir)/include/mk/testcases.mk -FILTER_OUT_MAKE_TARGETS := memfd_create_common +FILTER_OUT_MAKE_TARGETS := memfd_create_common memfd_create_hugetlb include $(top_srcdir)/include/mk/generic_leaf_target.mk -$(MAKE_TARGETS): %: %.o memfd_create_common.o +$(MAKE_TARGETS): %: %.o memfd_create_common.o memfd_create_hugetlb.o
testcases/kernel/syscalls/memfd_create/memfd_create03.c new file +165 -0
165 lines not on this page · raw message
testcases/kernel/syscalls/memfd_create/memfd_create04.c new file +140 -0
140 lines not on this page · raw message
testcases/kernel/syscalls/memfd_create/memfd_create_common.c +16 -4
@@ -144,21 +144,33 @@ int mfd_flags_available(const char *filename, const int lineno, }int check_mfd_new (const char *filename,const int lineno, -const char *name,loff_t sz,int flags) +const char *name,loff_t sz,int flags) {int fd; fd =sys_memfd_create (name, flags);if (fd <0 ) { +if ((flags &MFD_HUGETLB ) ==MFD_HUGETLB ) + goto end; +tst_brk_ (filename, lineno,TBROK |TERRNO , -"memfd_create(%s, %d) failed" , name, flags); +"memfd_create(%s, %d) failed" , name, flags); + } + +if ((flags &MFD_HUGETLB ) ==MFD_HUGETLB ) { +tst_res_ (filename, lineno,TINFO , +"memfd_create(%s, %d) succeeded" , + name, flags); + goto end; } -tst_res_ (filename, lineno,TPASS ,"memfd_create(%s, %d) succeeded" , - name, flags); +tst_res_ (filename, lineno,TPASS , +"memfd_create(%s, %d) succeeded" , + name, flags);check_ftruncate (filename, lineno, fd, sz); + end:return fd; }
testcases/kernel/syscalls/memfd_create/memfd_create_hugetlb.c new file +173 -0
173 lines not on this page · raw message
testcases/kernel/syscalls/memfd_create/memfd_create_hugetlb.h new file +60 -0
60 lines not on this page · raw message
Signature · 1 line
2.17.1
PATCH v2 1/1 Test for memfd_create() syscall with MFD_HUGETLB flag.
----- Original Message -----
Quoted 13 lines
memfd_create03.c: Tests the syscall for the flagMFD_HUGETLB.
memfd_create04.c: Tests the syscall for the flagsMFD_HUGE_2MB,MFD_HUGE_1GB,... used in conjunction withMFD_HUGETLBflag.
memfd_create_hugetlb.c: Contains helper functions formemfd_create03.candmemfd_create04.c.-- major changes from v1 to v2 --
Restore the original number of hugepages in
cleanup().
Quoted 8 lines
Modify
check_mfd_new()frommemfd_create_commonand replacesafe_huge_new()+safe_huge_x_new()frommemfd_create_hugetlbwith it.Modify
get_from_file()frommemfd_create_hugetlbto useFILE_LINES_SCANFin code logic.Move test dependent flags to
include/lapi
Hi,
<snip>
testcases/kernel/syscalls/memfd_create/memfd_create03.c line 53
Quoted 8 lines
testcases/kernel/syscalls/memfd_create/memfd_create03.c new file +8
-0+ +static void test_write_protect (int fd) +{ +if (CHECK_HUGE_NON_WRITEABLE (fd) ==0 ) +tst_res (TPASS ,"write call failed as expected" ); +else +tst_res (TFAIL ,"write call passed unexpectedly" ); +}
This looks unnecessarily complicated.
You have macro, that calls functions, which is printing result text as TINFO, then returning retcode based on which you decide if to report PASS or FAIL.
Why not report PASS/FAIL directly? Or why not put the test right here, without all the indirection? Are you planning on using CHECK_HUGE_NON_WRITEABLE() in other tests?
Quoted 6 lines
testcases/kernel/syscalls/memfd_create/memfd_create03.c new file +6
-0+ +static void test_def_pagesize (int fd) +{ +int ret; +long hps; +void *mem;
testcases/kernel/syscalls/memfd_create/memfd_create03.c line 69
Quoted 10 lines
testcases/kernel/syscalls/memfd_create/memfd_create03.c new file +10
-0+ + hps =GET_HUGE_DEF_PAGESIZE (); + mem =CHECK_HUGE_MMAPABLE (fd, hps); + + ret =CHECK_MUNMAP_LT_PAGE (mem, hps); +if (ret ==0 ) +tst_res (TPASS ,"File created in def pagesize %ldkB" , hps/1024 ); +else +tst_res (TFAIL ,"File is not in default pagesize" ); +}
Similar here, are you planning on using check_huge_mmapable() in other tests?
Side effect of CHECK_HUGE_MMAPABLE is that it mmaps a page. You're not freeing that page.
Quoted 54 lines
testcases/kernel/syscalls/memfd_create/memfd_create03.c new file +54
-0+ +static void test_max_hugepages (int fd) +{ +int res; +int new_fd; +int dummy_fd; +long hps; +long free_pages; +void *mem; + + free_pages =GET_HUGE_FREE_PAGES (); + hps =GET_HUGE_DEF_PAGESIZE (); + mem =CHECK_HUGE_MMAPABLE (fd, free_pages * hps); + + new_fd =CHECK_MFD_NEW ("new_file" ,0 ,MFD_HUGETLB ); +if (fd <0 ) +tst_brk (TBROK |TERRNO ,"memfd_create() failed" ); + res =CHECK_HUGE_NON_MMAPABLE (new_fd, hps); + dummy_fd = new_fd; +SAFE_CLOSE (new_fd); +tst_res (TINFO ,"close(%d) succeeded" , dummy_fd); + +SAFE_MUNMAP (mem, free_pages * hps); + +if (res ==0 ) +tst_res (TPASS ,"Hugepages creation is limited as expected" ); +else +tst_res (TFAIL ,"Hugepages creation limit failed" ); +} + +static const struct tcase { +void (*func)(int fd); +const char *desc; +} tcases[] = { + {&test_write_protect,"--TESTING WRITE CALL IN HUGEPAGES--" }, + {&test_def_pagesize,"--TESTING PAGE SIZE OF CREATED FILE--" }, + {&test_max_hugepages,"--TESTING THE NUMBER OF HUGEPAGES--" }, +}; + +static void memfd_huge_controller (unsigned int n) +{ +int fd; +int dummy_fd; +const struct tcase *tc; + + tc = &tcases[n]; + +tst_res (TINFO ,"%s" , tc->desc); + + fd =CHECK_MFD_NEW ("test_file" ,0 ,MFD_HUGETLB ); +if (fd <0 ) +tst_brk (TBROK |TERRNO ,"memfd_create() failed" ); + + tc->func (fd);
testcases/kernel/syscalls/memfd_create/memfd_create03.c line 133
Quoted 10 lines
testcases/kernel/syscalls/memfd_create/memfd_create03.c new file +10
-0+ + dummy_fd = fd; +SAFE_CLOSE (fd); +tst_res (TINFO ,"close(%d) succeeded" , dummy_fd); +} + +static void setup (void ) +{ +int fd; +
# ./memfd_create03 tst_test.c:1018: INFO: Timeout per run is 0h 05m 00s memfd_create03.c:134: BROK: Unable to get total number of hugepages
Summary:
passed 0 failed 0 skipped 0
warnings 0
Check if hugepages are supported is needed too, for example:
if (access(PATH_HUGEPAGES, F_OK))
tst_brk(TCONF, "Huge page is not supported.");
Review
Quoted 2 lines
(quoted diff) +2
-0+ if (GET_HUGE_TOTAL_PAGES() > 0) + return;
I thought you need 4 huge pages for the test. This skips setup even for 1.
testcases/kernel/syscalls/memfd_create/memfd_create03.c line 138
Quoted 3 lines
testcases/kernel/syscalls/memfd_create/memfd_create03.c new file +3
-0+ + fd =SAFE_OPEN ("/proc/sys/vm/nr_hugepages" ,O_WRONLY ); +SAFE_WRITE (1 , fd,"4" ,1 );
I'd suggest normal write, then read back the value from /proc to check if test can proceed.
testcases/kernel/syscalls/memfd_create/memfd_create03.c line 143
Quoted 5 lines
testcases/kernel/syscalls/memfd_create/memfd_create03.c new file +5
-0+ hugepages_enabled =1 ; +SAFE_CLOSE (fd); + +if (GET_HUGE_TOTAL_PAGES () ==0 ) +tst_brk (TCONF ,"Enable Hugepages manually" );
If you need 4 huge pages for test, then shouldn't this check you have at least 4?
<snip>
Quoted 33 lines
testcases/kernel/syscalls/memfd_create/memfd_create_common.c +12
-3@@ -144,21 +144,33 @@ int mfd_flags_available(const char *filename, const int lineno, }int check_mfd_new (const char *filename,const int lineno, -const char *name,loff_t sz,int flags) +const char *name,loff_t sz,int flags) {int fd; fd =sys_memfd_create (name, flags);if (fd <0 ) { +if ((flags &MFD_HUGETLB ) ==MFD_HUGETLB ) + goto end; +tst_brk_ (filename, lineno,TBROK |TERRNO , -"memfd_create(%s, %d) failed" , name, flags); +"memfd_create(%s, %d) failed" , name, flags); + } + +if ((flags &MFD_HUGETLB ) ==MFD_HUGETLB ) { +tst_res_ (filename, lineno,TINFO , +"memfd_create(%s, %d) succeeded" , + name, flags); + goto end; } - tst_res_(filename, lineno, TPASS, "memfd_create(%s, %d) succeeded",
testcases/kernel/syscalls/memfd_create/memfd_create_common.c int mfd_flags_available(const char *filename, const int · line 176
Quoted 10 lines
testcases/kernel/syscalls/memfd_create/memfd_create_common.c +4
-1- name, flags); +tst_res_ (filename, lineno,TPASS , +"memfd_create(%s, %d) succeeded" , + name, flags);check_ftruncate (filename, lineno, fd, sz); + end:return fd; }
This is ugly way of making it behave one way for your test, and other way for different test.
There are couple options:
1. two functionsmfd_new() - calls the syscall, and only returns a value check_mfd_new() - calls mfd_new(), checks returned value, calls tst_brk()
2. extra parameter, that specifies if failure should lead to tst_brk(), similar to 'strict' parameter in file_lines_scanf().
<snip>
Quoted 19 lines
testcases/kernel/syscalls/memfd_create/memfd_create_common.c +19
-0+int check_huge_non_writeable (const char *filename,const int lineno,int fd) +{ +ssize_t ret; +char test_str[] ="data" ; + + ret =write (fd, test_str,sizeof (test_str)); + +if (ret <0 ) { +if (errno !=EINVAL ) { +tst_res_ (filename, lineno,TINFO , +"write(%d, \"%s\", %zu) didn't fail as expected" , + fd, test_str,strlen (test_str)); + +return -1 ; + } + }else { +tst_res_ (filename, lineno,TINFO , +"write(%d, \"%s\", %zu) succeeded unexpectedly" , + fd, test_str,strlen (test_str));
testcases/kernel/syscalls/memfd_create/memfd_create_common.c
Quoted 10 lines
testcases/kernel/syscalls/memfd_create/memfd_create_common.c +10
-0+ +return -1 ; + } + +tst_res_ (filename, lineno,TINFO , +"write(%d, \"%s\", %zu) failed as expected" , + fd, test_str,strlen (test_str)); + +return 0 ; +}
If this reported PASS/FAIL directly, instead of TINFO, you don't need return code and extra logic in callers.
<snip>
testcases/kernel/syscalls/memfd_create/memfd_create_common.c
Quoted 9 lines
testcases/kernel/syscalls/memfd_create/memfd_create_common.c +9
-0+ +static void get_from_file (const char *filename,const int lineno, +char *pattern,char *err_msg,long *val) +{ +int ret; + + ret =FILE_LINES_SCANF (MEMINFO_PATH , pattern, val); +if (ret ==1 ) +tst_brk_ (filename, lineno,TBROK , err_msg,NULL );
As I mentioned in v1 review, there's also SAFE_FILE_LINES_SCANF, that does exactly same thing.
--
memfd_create04.c:80: INFO: Attempt to create 1GB huge pages memfd_create04.c:82: INFO: memfd_create(tfile, 2013265924) succeeded memfd_create04.c:94: INFO: mmap((nil), 1073741824, 2, 2, 3, 0) failed memfd_create04.c:96: WARN: Contiguous memory unavailable to map memfd_create04.c:100: INFO: close(3) succeeded memfd_create04.c:102: PASS: Successfully created 1GB pages
This looks like a guaranteed warning on some systems.
What do you expect user do with this warning? Report bug? Change config?
Regards,
Jan
PATCH v2 1/1 Test for memfd_create() syscall withMFD_HUGETLBflag.
Hi,
Thanks for your code review.
Quoted 8 lines
testcases/kernel/syscalls/memfd_create/memfd_create03.c new file +7
-0+static void test_write_protect (int fd) +{ +if (CHECK_HUGE_NON_WRITEABLE (fd) ==0 ) +tst_res (TPASS ,"write call failed as expected" ); +else +tst_res (TFAIL ,"write call passed unexpectedly" ); +}Why not report PASS/FAIL directly? Or why not put the test
right here, without all the indirection? Are you planning on using CHECK_HUGE_NON_WRITEABLE() in other tests?
CHECK_HUGE_NON_WRITEABLE() is dedicated to this test alone. We will move the code logic from CHECK_HUGE_NON_WRITEABLE() to test_write_protect() avoiding indirection.
Quoted 6 lines
testcases/kernel/syscalls/memfd_create/memfd_create03.c new file +5
-0+ hugepages_enabled =1 ; +SAFE_CLOSE (fd); + +if (GET_HUGE_TOTAL_PAGES () ==0 ) +tst_brk (TCONF ,"Enable Hugepages manually" );I thought you need 4 huge pages for the test. This skips setup
even for 1.
Quoted 1 line
If you need 4 huge pages for test, then shouldn't this check you have at
least 4?
There is no such demand for particular number of hugepages. Tests will work for any number of hugepages greater than 0. 4 is actually a randomly choosen number.
We will modify setup in a way that:-
1. Check the number of free_hugepages.
2. if free_hugepages is 0, increase the total hugepage count by a number (choosen 4).
3. Again, if free_hugepages is 0, tst_brk -> TCONF.
In clean-up, restore the original value of total hugepage count.
memfd_create04.c:80: INFO: Attempt to create 1GB huge pages memfd_create04.c:82: INFO: memfd_create(tfile, 2013265924) succeeded memfd_create04.c:94: INFO: mmap((nil), 1073741824, 2, 2, 3, 0) failed memfd_create04.c:96: WARN: Contiguous memory unavailable to map memfd_create04.c:100: INFO: close(3) succeeded memfd_create04.c:102: PASS: Successfully created 1GB pages
Quoted 1 line
This looks like a guaranteed warning on some systems.
What do you expect user do with this warning? Report bug? Change config?
In memfd_create04:
1. The warning is to due lack of contiguous memory resource to map
hugepage and is nothing related to syscall failure.
Map checking here is an add-on, which is not compulsory. We will remove this checking as users have nothing to do with it.
2. Hence if no map checking, the test will work without set-up & clean-up.
Regards,
Vishnu K