| Message ID | 20220201033459.156944-1-mike.kravetz@oracle.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | selftests/vm: cleanup hugetlb file after mremap test | expand |
On 1/31/22 8:34 PM, Mike Kravetz wrote: > The hugepage-mremap test will create a file in a hugetlb filesystem. > In a default 'run_vmtests' run, the file will contain all the hugetlb > pages. After the test, the file remains and there are no free hugetlb > pages for subsequent tests. This causes those hugetlb tests to fail. > > Change hugepage-mremap to take the name of the hugetlb file as an > argument. Unlink the file within the test, and just to be sure remove > the file in the run_vmtests script. > > Signed-off-by: Mike Kravetz <mike.kravetz@oracle.com> > --- > tools/testing/selftests/vm/hugepage-mremap.c | 26 ++++++++++++++------ > tools/testing/selftests/vm/run_vmtests.sh | 3 ++- > 2 files changed, 21 insertions(+), 8 deletions(-) > > diff --git a/tools/testing/selftests/vm/hugepage-mremap.c b/tools/testing/selftests/vm/hugepage-mremap.c > index 2a7c33631a29..1d689084a54b 100644 > --- a/tools/testing/selftests/vm/hugepage-mremap.c > +++ b/tools/testing/selftests/vm/hugepage-mremap.c > @@ -3,9 +3,10 @@ > * hugepage-mremap: > * > * Example of remapping huge page memory in a user application using the > - * mremap system call. Code assumes a hugetlbfs filesystem is mounted > - * at './huge'. The amount of memory used by this test is decided by a command > - * line argument in MBs. If missing, the default amount is 10MB. > + * mremap system call. The path to a file in a hugetlbfs filesystem must > + * be passed as the last argument to this test. The amount of memory used > + * by this test in MBs can optionally be passed as an argument. If no memory > + * amount is passed, the default amount is 10MB. > * > * To make sure the test triggers pmd sharing and goes through the 'unshare' > * path in the mremap code use 1GB (1024) or more. > @@ -25,7 +26,6 @@ > #define DEFAULT_LENGTH_MB 10UL > #define MB_TO_BYTES(x) (x * 1024 * 1024) > > -#define FILE_NAME "huge/hugepagefile" > #define PROTECTION (PROT_READ | PROT_WRITE | PROT_EXEC) > #define FLAGS (MAP_SHARED | MAP_ANONYMOUS) > > @@ -107,17 +107,26 @@ static void register_region_with_uffd(char *addr, size_t len) > > int main(int argc, char *argv[]) > { > + size_t length; > + > + if (argc != 2 && argc != 3) { > + printf("Usage: %s [length_in_MB] <hugetlb_file>\n", argv[0]); > + exit(1); > + } > + > /* Read memory length as the first arg if valid, otherwise fallback to > - * the default length. Any additional args are ignored. > + * the default length. > */ > - size_t length = argc > 1 ? (size_t)atoi(argv[1]) : 0UL; > + if (argc == 3) > + length = argc > 2 ? (size_t)atoi(argv[1]) : 0UL; > Looks like getopt() might be useful - we have a few arguments now. Makes it easier to maintain the test. > length = length > 0 ? length : DEFAULT_LENGTH_MB; > length = MB_TO_BYTES(length); > > int ret = 0; > > - int fd = open(FILE_NAME, O_CREAT | O_RDWR, 0755); > + /* last arg is the hugetlb file name */ > + int fd = open(argv[argc-1], O_CREAT | O_RDWR, 0755); > > if (fd < 0) { > perror("Open failed"); > @@ -169,5 +178,8 @@ int main(int argc, char *argv[]) > > munmap(addr, length); > > + close(fd); > + unlink(argv[argc-1]); > + > return ret; > } > diff --git a/tools/testing/selftests/vm/run_vmtests.sh b/tools/testing/selftests/vm/run_vmtests.sh > index e09040a3dc08..e10d50e0b8e8 100755 > --- a/tools/testing/selftests/vm/run_vmtests.sh > +++ b/tools/testing/selftests/vm/run_vmtests.sh > @@ -111,13 +111,14 @@ fi > echo "-----------------------" > echo "running hugepage-mremap" > echo "-----------------------" > -./hugepage-mremap 256 > +./hugepage-mremap $mnt/huge_mremap > if [ $? -ne 0 ]; then > echo "[FAIL]" > exitcode=1 > else > echo "[PASS]" > fi > +rm -f $mnt/huge_mremap > > echo "------------------------" > echo "running hugepage-vmemmap" > Reviewed-by: Shuah Khan <skhan@linuxfoundation.org> thanks, -- Shuah
On Mon, Jan 31, 2022 at 7:35 PM Mike Kravetz <mike.kravetz@oracle.com> wrote: > > The hugepage-mremap test will create a file in a hugetlb filesystem. > In a default 'run_vmtests' run, the file will contain all the hugetlb > pages. After the test, the file remains and there are no free hugetlb > pages for subsequent tests. This causes those hugetlb tests to fail. > > Change hugepage-mremap to take the name of the hugetlb file as an > argument. Unlink the file within the test, and just to be sure remove > the file in the run_vmtests script. > > Signed-off-by: Mike Kravetz <mike.kravetz@oracle.com> Thanks Mike! Reviewed-by: Mina Almasry <almasrymina@google.com> > --- > tools/testing/selftests/vm/hugepage-mremap.c | 26 ++++++++++++++------ > tools/testing/selftests/vm/run_vmtests.sh | 3 ++- > 2 files changed, 21 insertions(+), 8 deletions(-) > > diff --git a/tools/testing/selftests/vm/hugepage-mremap.c b/tools/testing/selftests/vm/hugepage-mremap.c > index 2a7c33631a29..1d689084a54b 100644 > --- a/tools/testing/selftests/vm/hugepage-mremap.c > +++ b/tools/testing/selftests/vm/hugepage-mremap.c > @@ -3,9 +3,10 @@ > * hugepage-mremap: > * > * Example of remapping huge page memory in a user application using the > - * mremap system call. Code assumes a hugetlbfs filesystem is mounted > - * at './huge'. The amount of memory used by this test is decided by a command > - * line argument in MBs. If missing, the default amount is 10MB. > + * mremap system call. The path to a file in a hugetlbfs filesystem must > + * be passed as the last argument to this test. The amount of memory used > + * by this test in MBs can optionally be passed as an argument. If no memory > + * amount is passed, the default amount is 10MB. > * > * To make sure the test triggers pmd sharing and goes through the 'unshare' > * path in the mremap code use 1GB (1024) or more. > @@ -25,7 +26,6 @@ > #define DEFAULT_LENGTH_MB 10UL > #define MB_TO_BYTES(x) (x * 1024 * 1024) > > -#define FILE_NAME "huge/hugepagefile" > #define PROTECTION (PROT_READ | PROT_WRITE | PROT_EXEC) > #define FLAGS (MAP_SHARED | MAP_ANONYMOUS) > > @@ -107,17 +107,26 @@ static void register_region_with_uffd(char *addr, size_t len) > > int main(int argc, char *argv[]) > { > + size_t length; > + > + if (argc != 2 && argc != 3) { > + printf("Usage: %s [length_in_MB] <hugetlb_file>\n", argv[0]); > + exit(1); > + } > + > /* Read memory length as the first arg if valid, otherwise fallback to > - * the default length. Any additional args are ignored. > + * the default length. > */ > - size_t length = argc > 1 ? (size_t)atoi(argv[1]) : 0UL; > + if (argc == 3) > + length = argc > 2 ? (size_t)atoi(argv[1]) : 0UL; > > length = length > 0 ? length : DEFAULT_LENGTH_MB; > length = MB_TO_BYTES(length); > > int ret = 0; > > - int fd = open(FILE_NAME, O_CREAT | O_RDWR, 0755); > + /* last arg is the hugetlb file name */ > + int fd = open(argv[argc-1], O_CREAT | O_RDWR, 0755); > > if (fd < 0) { > perror("Open failed"); > @@ -169,5 +178,8 @@ int main(int argc, char *argv[]) > > munmap(addr, length); > > + close(fd); > + unlink(argv[argc-1]); > + > return ret; > } > diff --git a/tools/testing/selftests/vm/run_vmtests.sh b/tools/testing/selftests/vm/run_vmtests.sh > index e09040a3dc08..e10d50e0b8e8 100755 > --- a/tools/testing/selftests/vm/run_vmtests.sh > +++ b/tools/testing/selftests/vm/run_vmtests.sh > @@ -111,13 +111,14 @@ fi > echo "-----------------------" > echo "running hugepage-mremap" > echo "-----------------------" > -./hugepage-mremap 256 > +./hugepage-mremap $mnt/huge_mremap > if [ $? -ne 0 ]; then > echo "[FAIL]" > exitcode=1 > else > echo "[PASS]" > fi > +rm -f $mnt/huge_mremap > > echo "------------------------" > echo "running hugepage-vmemmap" > -- > 2.34.1 >
On Tue, Feb 1, 2022 at 11:35 AM Mike Kravetz <mike.kravetz@oracle.com> wrote: > > The hugepage-mremap test will create a file in a hugetlb filesystem. > In a default 'run_vmtests' run, the file will contain all the hugetlb > pages. After the test, the file remains and there are no free hugetlb > pages for subsequent tests. This causes those hugetlb tests to fail. > > Change hugepage-mremap to take the name of the hugetlb file as an > argument. Unlink the file within the test, and just to be sure remove > the file in the run_vmtests script. > > Signed-off-by: Mike Kravetz <mike.kravetz@oracle.com> > --- [...] > diff --git a/tools/testing/selftests/vm/run_vmtests.sh b/tools/testing/selftests/vm/run_vmtests.sh > index e09040a3dc08..e10d50e0b8e8 100755 > --- a/tools/testing/selftests/vm/run_vmtests.sh > +++ b/tools/testing/selftests/vm/run_vmtests.sh > @@ -111,13 +111,14 @@ fi > echo "-----------------------" > echo "running hugepage-mremap" > echo "-----------------------" > -./hugepage-mremap 256 > +./hugepage-mremap $mnt/huge_mremap > if [ $? -ne 0 ]; then > echo "[FAIL]" > exitcode=1 > else > echo "[PASS]" > fi > +rm -f $mnt/huge_mremap > A little strange (since file creation and deletion are not in the same place). I know $mnt/huge_mremap is created in hugepage-mremap. But I think hugepage-mremap should do the housekeeping instead of deleting the file here. Or we can create the file before doing the test. Like: + touch $mnt/huge_mremap -./hugepage-mremap 256 +./hugepage-mremap $mnt/huge_mremap +rm -f $mnt/huge_mremap Otherwise LGTM. Reviewed-by: Muchun Song <songmuchun@bytedance.com> Thanks.
Thanks Mike for working on this! On Mon, Jan 31, 2022 at 7:35 PM Mike Kravetz <mike.kravetz@oracle.com> wrote: > > The hugepage-mremap test will create a file in a hugetlb filesystem. > In a default 'run_vmtests' run, the file will contain all the hugetlb > pages. After the test, the file remains and there are no free hugetlb > pages for subsequent tests. This causes those hugetlb tests to fail. > > Change hugepage-mremap to take the name of the hugetlb file as an > argument. Unlink the file within the test, and just to be sure remove > the file in the run_vmtests script. > The hugepage-mmap test also uses a hugetlb file in the same fashion. I think the missing close() and unlink() calls were the only difference (thanks for catching that). I would suggest for consistency purposes to either make both tests take the filename as an argument or not, fine either way. > Signed-off-by: Mike Kravetz <mike.kravetz@oracle.com> > --- > tools/testing/selftests/vm/hugepage-mremap.c | 26 ++++++++++++++------ > tools/testing/selftests/vm/run_vmtests.sh | 3 ++- > 2 files changed, 21 insertions(+), 8 deletions(-) > > diff --git a/tools/testing/selftests/vm/hugepage-mremap.c b/tools/testing/selftests/vm/hugepage-mremap.c > index 2a7c33631a29..1d689084a54b 100644 > --- a/tools/testing/selftests/vm/hugepage-mremap.c > +++ b/tools/testing/selftests/vm/hugepage-mremap.c > @@ -3,9 +3,10 @@ > * hugepage-mremap: > * > * Example of remapping huge page memory in a user application using the > - * mremap system call. Code assumes a hugetlbfs filesystem is mounted > - * at './huge'. The amount of memory used by this test is decided by a command > - * line argument in MBs. If missing, the default amount is 10MB. > + * mremap system call. The path to a file in a hugetlbfs filesystem must > + * be passed as the last argument to this test. The amount of memory used > + * by this test in MBs can optionally be passed as an argument. If no memory > + * amount is passed, the default amount is 10MB. > * > * To make sure the test triggers pmd sharing and goes through the 'unshare' > * path in the mremap code use 1GB (1024) or more. > @@ -25,7 +26,6 @@ > #define DEFAULT_LENGTH_MB 10UL > #define MB_TO_BYTES(x) (x * 1024 * 1024) > > -#define FILE_NAME "huge/hugepagefile" > #define PROTECTION (PROT_READ | PROT_WRITE | PROT_EXEC) > #define FLAGS (MAP_SHARED | MAP_ANONYMOUS) > > @@ -107,17 +107,26 @@ static void register_region_with_uffd(char *addr, size_t len) > > int main(int argc, char *argv[]) > { > + size_t length; > + > + if (argc != 2 && argc != 3) { > + printf("Usage: %s [length_in_MB] <hugetlb_file>\n", argv[0]); > + exit(1); > + } > + > /* Read memory length as the first arg if valid, otherwise fallback to > - * the default length. Any additional args are ignored. > + * the default length. > */ > - size_t length = argc > 1 ? (size_t)atoi(argv[1]) : 0UL; > + if (argc == 3) > + length = argc > 2 ? (size_t)atoi(argv[1]) : 0UL; I think the ternary operator is not needed here as it will always be true if we enter the if block. Otherwise, LGTM. Acked-by: Yosry Ahmed <yosryahmed@google.com> > > length = length > 0 ? length : DEFAULT_LENGTH_MB; > length = MB_TO_BYTES(length); > > int ret = 0; > > - int fd = open(FILE_NAME, O_CREAT | O_RDWR, 0755); > + /* last arg is the hugetlb file name */ > + int fd = open(argv[argc-1], O_CREAT | O_RDWR, 0755); > > if (fd < 0) { > perror("Open failed"); > @@ -169,5 +178,8 @@ int main(int argc, char *argv[]) > > munmap(addr, length); > > + close(fd); > + unlink(argv[argc-1]); > + > return ret; > } > diff --git a/tools/testing/selftests/vm/run_vmtests.sh b/tools/testing/selftests/vm/run_vmtests.sh > index e09040a3dc08..e10d50e0b8e8 100755 > --- a/tools/testing/selftests/vm/run_vmtests.sh > +++ b/tools/testing/selftests/vm/run_vmtests.sh > @@ -111,13 +111,14 @@ fi > echo "-----------------------" > echo "running hugepage-mremap" > echo "-----------------------" > -./hugepage-mremap 256 > +./hugepage-mremap $mnt/huge_mremap > if [ $? -ne 0 ]; then > echo "[FAIL]" > exitcode=1 > else > echo "[PASS]" > fi > +rm -f $mnt/huge_mremap > > echo "------------------------" > echo "running hugepage-vmemmap" > -- > 2.34.1 >
diff --git a/tools/testing/selftests/vm/hugepage-mremap.c b/tools/testing/selftests/vm/hugepage-mremap.c index 2a7c33631a29..1d689084a54b 100644 --- a/tools/testing/selftests/vm/hugepage-mremap.c +++ b/tools/testing/selftests/vm/hugepage-mremap.c @@ -3,9 +3,10 @@ * hugepage-mremap: * * Example of remapping huge page memory in a user application using the - * mremap system call. Code assumes a hugetlbfs filesystem is mounted - * at './huge'. The amount of memory used by this test is decided by a command - * line argument in MBs. If missing, the default amount is 10MB. + * mremap system call. The path to a file in a hugetlbfs filesystem must + * be passed as the last argument to this test. The amount of memory used + * by this test in MBs can optionally be passed as an argument. If no memory + * amount is passed, the default amount is 10MB. * * To make sure the test triggers pmd sharing and goes through the 'unshare' * path in the mremap code use 1GB (1024) or more. @@ -25,7 +26,6 @@ #define DEFAULT_LENGTH_MB 10UL #define MB_TO_BYTES(x) (x * 1024 * 1024) -#define FILE_NAME "huge/hugepagefile" #define PROTECTION (PROT_READ | PROT_WRITE | PROT_EXEC) #define FLAGS (MAP_SHARED | MAP_ANONYMOUS) @@ -107,17 +107,26 @@ static void register_region_with_uffd(char *addr, size_t len) int main(int argc, char *argv[]) { + size_t length; + + if (argc != 2 && argc != 3) { + printf("Usage: %s [length_in_MB] <hugetlb_file>\n", argv[0]); + exit(1); + } + /* Read memory length as the first arg if valid, otherwise fallback to - * the default length. Any additional args are ignored. + * the default length. */ - size_t length = argc > 1 ? (size_t)atoi(argv[1]) : 0UL; + if (argc == 3) + length = argc > 2 ? (size_t)atoi(argv[1]) : 0UL; length = length > 0 ? length : DEFAULT_LENGTH_MB; length = MB_TO_BYTES(length); int ret = 0; - int fd = open(FILE_NAME, O_CREAT | O_RDWR, 0755); + /* last arg is the hugetlb file name */ + int fd = open(argv[argc-1], O_CREAT | O_RDWR, 0755); if (fd < 0) { perror("Open failed"); @@ -169,5 +178,8 @@ int main(int argc, char *argv[]) munmap(addr, length); + close(fd); + unlink(argv[argc-1]); + return ret; } diff --git a/tools/testing/selftests/vm/run_vmtests.sh b/tools/testing/selftests/vm/run_vmtests.sh index e09040a3dc08..e10d50e0b8e8 100755 --- a/tools/testing/selftests/vm/run_vmtests.sh +++ b/tools/testing/selftests/vm/run_vmtests.sh @@ -111,13 +111,14 @@ fi echo "-----------------------" echo "running hugepage-mremap" echo "-----------------------" -./hugepage-mremap 256 +./hugepage-mremap $mnt/huge_mremap if [ $? -ne 0 ]; then echo "[FAIL]" exitcode=1 else echo "[PASS]" fi +rm -f $mnt/huge_mremap echo "------------------------" echo "running hugepage-vmemmap"
The hugepage-mremap test will create a file in a hugetlb filesystem. In a default 'run_vmtests' run, the file will contain all the hugetlb pages. After the test, the file remains and there are no free hugetlb pages for subsequent tests. This causes those hugetlb tests to fail. Change hugepage-mremap to take the name of the hugetlb file as an argument. Unlink the file within the test, and just to be sure remove the file in the run_vmtests script. Signed-off-by: Mike Kravetz <mike.kravetz@oracle.com> --- tools/testing/selftests/vm/hugepage-mremap.c | 26 ++++++++++++++------ tools/testing/selftests/vm/run_vmtests.sh | 3 ++- 2 files changed, 21 insertions(+), 8 deletions(-)