| Message ID | 20240112072144.620098-5-usama.anjum@collabora.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v2,1/7] selftests/mm: hugepage-shm: conform test to TAP format output | expand |
On Thu, Jan 11, 2024 at 11:21 PM Muhammad Usama Anjum <usama.anjum@collabora.com> wrote: > > Conform the layout, informational and status messages to TAP. No > functional change is intended other than the layout of output messages. > > Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com> > --- > Tested this by reverting the patch a08c7193e4f18dc8508f2d07d0de2c5b94cb39a3 > ("mm/filemap: remove hugetlb special casing in filemap.c") as it has > broken the test. The bug report can be found at [1]. > > Tested with proposed fix as well [2]. > > [1] https://lore.kernel.org/all/079335ab-190f-41f7-b832-6ffe7528fd8b@collabora.com > [2] https://lore.kernel.org/all/a20e7bdb-7344-306d-e8f5-5ee69af7d5ea@oracle.com > --- > .../selftests/mm/hugetlb-read-hwpoison.c | 116 +++++++++--------- > 1 file changed, 56 insertions(+), 60 deletions(-) > > diff --git a/tools/testing/selftests/mm/hugetlb-read-hwpoison.c b/tools/testing/selftests/mm/hugetlb-read-hwpoison.c > index ba6cc6f9cabc..193ad7275df5 100644 > --- a/tools/testing/selftests/mm/hugetlb-read-hwpoison.c > +++ b/tools/testing/selftests/mm/hugetlb-read-hwpoison.c > @@ -58,8 +58,8 @@ static bool verify_chunk(char *buf, size_t len, char val) > > for (i = 0; i < len; ++i) { > if (buf[i] != val) { > - printf(PREFIX ERROR_PREFIX "check fail: buf[%lu] = %u != %u\n", > - i, buf[i], val); > + ksft_print_msg(PREFIX ERROR_PREFIX "check fail: buf[%lu] = %u != %u\n", > + i, buf[i], val); > return false; > } > } > @@ -75,9 +75,9 @@ static bool seek_read_hugepage_filemap(int fd, size_t len, size_t wr_chunk_size, > ssize_t total_ret_count = 0; > char val = offset / wr_chunk_size + offset % wr_chunk_size; > > - printf(PREFIX PREFIX "init val=%u with offset=0x%lx\n", val, offset); > - printf(PREFIX PREFIX "expect to read 0x%lx bytes of data in total\n", > - expected); > + ksft_print_msg(PREFIX PREFIX "init val=%u with offset=0x%lx\n", val, offset); > + ksft_print_msg(PREFIX PREFIX "expect to read 0x%lx bytes of data in total\n", > + expected); > if (lseek(fd, offset, SEEK_SET) < 0) { > perror(PREFIX ERROR_PREFIX "seek failed"); > return false; > @@ -86,7 +86,7 @@ static bool seek_read_hugepage_filemap(int fd, size_t len, size_t wr_chunk_size, > while (offset + total_ret_count < len) { > ret_count = read(fd, buf, wr_chunk_size); > if (ret_count == 0) { > - printf(PREFIX PREFIX "read reach end of the file\n"); > + ksft_print_msg(PREFIX PREFIX "read reach end of the file\n"); > break; > } else if (ret_count < 0) { > perror(PREFIX ERROR_PREFIX "read failed"); > @@ -98,8 +98,8 @@ static bool seek_read_hugepage_filemap(int fd, size_t len, size_t wr_chunk_size, > > total_ret_count += ret_count; > } > - printf(PREFIX PREFIX "actually read 0x%lx bytes of data in total\n", > - total_ret_count); > + ksft_print_msg(PREFIX PREFIX "actually read 0x%lx bytes of data in total\n", > + total_ret_count); > > return total_ret_count == expected; > } > @@ -112,15 +112,15 @@ static bool read_hugepage_filemap(int fd, size_t len, > ssize_t total_ret_count = 0; > char val = 0; > > - printf(PREFIX PREFIX "expect to read 0x%lx bytes of data in total\n", > - expected); > + ksft_print_msg(PREFIX PREFIX "expect to read 0x%lx bytes of data in total\n", > + expected); > while (total_ret_count < len) { > ret_count = read(fd, buf, wr_chunk_size); > if (ret_count == 0) { > - printf(PREFIX PREFIX "read reach end of the file\n"); > + ksft_print_msg(PREFIX PREFIX "read reach end of the file\n"); > break; > } else if (ret_count < 0) { > - perror(PREFIX ERROR_PREFIX "read failed"); > + ksft_print_msg(PREFIX ERROR_PREFIX "read failed"); Should we also include strerror(errno) in log msg, like you did below? Actually, would ksft_perror be a better choice for perror()s? > > break; > } > ++val; > @@ -129,8 +129,8 @@ static bool read_hugepage_filemap(int fd, size_t len, > > total_ret_count += ret_count; > } > - printf(PREFIX PREFIX "actually read 0x%lx bytes of data in total\n", > - total_ret_count); > + ksft_print_msg(PREFIX PREFIX "actually read 0x%lx bytes of data in total\n", > + total_ret_count); > > return total_ret_count == expected; > } > @@ -142,14 +142,15 @@ test_hugetlb_read(int fd, size_t len, size_t wr_chunk_size) > char *filemap = NULL; > > if (ftruncate(fd, len) < 0) { > - perror(PREFIX ERROR_PREFIX "ftruncate failed"); > + ksft_print_msg(PREFIX ERROR_PREFIX "ftruncate failed: %s\n", strerror(errno)); > return status; > } > > filemap = mmap(NULL, len, PROT_READ | PROT_WRITE, > MAP_SHARED | MAP_POPULATE, fd, 0); > if (filemap == MAP_FAILED) { > - perror(PREFIX ERROR_PREFIX "mmap for primary mapping failed"); > + ksft_print_msg(PREFIX ERROR_PREFIX "mmap for primary mapping failed: %s\n", > + strerror(errno)); > goto done; > } > > @@ -162,7 +163,8 @@ test_hugetlb_read(int fd, size_t len, size_t wr_chunk_size) > munmap(filemap, len); > done: > if (ftruncate(fd, 0) < 0) { > - perror(PREFIX ERROR_PREFIX "ftruncate back to 0 failed"); > + ksft_print_msg(PREFIX ERROR_PREFIX "ftruncate back to 0 failed : %s\n", > + strerror(errno)); > status = TEST_FAILED; > } > > @@ -179,14 +181,15 @@ test_hugetlb_read_hwpoison(int fd, size_t len, size_t wr_chunk_size, > const unsigned long pagesize = getpagesize(); > > if (ftruncate(fd, len) < 0) { > - perror(PREFIX ERROR_PREFIX "ftruncate failed"); > + ksft_print_msg(PREFIX ERROR_PREFIX "ftruncate failed: %s\n", strerror(errno)); > return status; > } > > filemap = mmap(NULL, len, PROT_READ | PROT_WRITE, > MAP_SHARED | MAP_POPULATE, fd, 0); > if (filemap == MAP_FAILED) { > - perror(PREFIX ERROR_PREFIX "mmap for primary mapping failed"); > + ksft_print_msg(PREFIX ERROR_PREFIX "mmap for primary mapping failed: %s\n", > + strerror(errno)); > goto done; > } > > @@ -201,7 +204,7 @@ test_hugetlb_read_hwpoison(int fd, size_t len, size_t wr_chunk_size, > */ > hwp_addr = filemap + len / 2 + pagesize; > if (madvise(hwp_addr, pagesize, MADV_HWPOISON) < 0) { > - perror(PREFIX ERROR_PREFIX "MADV_HWPOISON failed"); > + ksft_print_msg(PREFIX ERROR_PREFIX "MADV_HWPOISON failed: %s\n", strerror(errno)); > goto unmap; > } > > @@ -228,7 +231,8 @@ test_hugetlb_read_hwpoison(int fd, size_t len, size_t wr_chunk_size, > munmap(filemap, len); > done: > if (ftruncate(fd, 0) < 0) { > - perror(PREFIX ERROR_PREFIX "ftruncate back to 0 failed"); > + ksft_print_msg(PREFIX ERROR_PREFIX "ftruncate back to 0 failed: %s\n", > + strerror(errno)); > status = TEST_FAILED; > } > > @@ -240,27 +244,32 @@ static int create_hugetlbfs_file(struct statfs *file_stat) > int fd; > > fd = memfd_create("hugetlb_tmp", MFD_HUGETLB); > - if (fd < 0) { > - perror(PREFIX ERROR_PREFIX "could not open hugetlbfs file"); > - return -1; > - } > + if (fd < 0) > + ksft_exit_fail_msg(PREFIX ERROR_PREFIX "could not open hugetlbfs file: %s\n", > + strerror(errno)); > > memset(file_stat, 0, sizeof(*file_stat)); > + > if (fstatfs(fd, file_stat)) { > - perror(PREFIX ERROR_PREFIX "fstatfs failed"); > - goto close; > + close(fd); > + ksft_exit_fail_msg(PREFIX ERROR_PREFIX "fstatfs failed: %s\n", strerror(errno)); > } > if (file_stat->f_type != HUGETLBFS_MAGIC) { > - printf(PREFIX ERROR_PREFIX "not hugetlbfs file\n"); > - goto close; > + close(fd); > + ksft_exit_fail_msg(PREFIX ERROR_PREFIX "not hugetlbfs file\n"); > } > > return fd; > -close: > - close(fd); > - return -1; > } > > +#define KSFT_PRINT_MSG(status, fmt, ...) \ > + do { \ > + if (status == TEST_SKIPPED) \ > + ksft_test_result_skip(fmt, __VA_ARGS__); \ > + else \ > + ksft_test_result(status == TEST_PASSED, fmt, __VA_ARGS__); \ > + } while (0) > + > int main(void) > { > int fd; > @@ -273,50 +282,37 @@ int main(void) > }; > size_t i; > > + ksft_print_header(); > + ksft_set_plan(12); > + > for (i = 0; i < ARRAY_SIZE(wr_chunk_sizes); ++i) { > - printf("Write/read chunk size=0x%lx\n", > - wr_chunk_sizes[i]); > + ksft_print_msg("Write/read chunk size=0x%lx\n", > + wr_chunk_sizes[i]); > > fd = create_hugetlbfs_file(&file_stat); > - if (fd < 0) > - goto create_failure; > - printf(PREFIX "HugeTLB read regression test...\n"); > + ksft_print_msg(PREFIX "HugeTLB read regression test...\n"); > status = test_hugetlb_read(fd, file_stat.f_bsize, > wr_chunk_sizes[i]); > - printf(PREFIX "HugeTLB read regression test...%s\n", > - status_to_str(status)); > + KSFT_PRINT_MSG(status, PREFIX "HugeTLB read regression test...%s\n", > + status_to_str(status)); > close(fd); > - if (status == TEST_FAILED) > - return -1; > > fd = create_hugetlbfs_file(&file_stat); > - if (fd < 0) > - goto create_failure; > - printf(PREFIX "HugeTLB read HWPOISON test...\n"); > + ksft_print_msg(PREFIX "HugeTLB read HWPOISON test...\n"); > status = test_hugetlb_read_hwpoison(fd, file_stat.f_bsize, > wr_chunk_sizes[i], false); > - printf(PREFIX "HugeTLB read HWPOISON test...%s\n", > - status_to_str(status)); > + KSFT_PRINT_MSG(status, PREFIX "HugeTLB read HWPOISON test...%s\n", > + status_to_str(status)); > close(fd); > - if (status == TEST_FAILED) > - return -1; > > fd = create_hugetlbfs_file(&file_stat); > - if (fd < 0) > - goto create_failure; > - printf(PREFIX "HugeTLB seek then read HWPOISON test...\n"); > + ksft_print_msg(PREFIX "HugeTLB seek then read HWPOISON test...\n"); > status = test_hugetlb_read_hwpoison(fd, file_stat.f_bsize, > wr_chunk_sizes[i], true); > - printf(PREFIX "HugeTLB seek then read HWPOISON test...%s\n", > - status_to_str(status)); > + KSFT_PRINT_MSG(status, PREFIX "HugeTLB seek then read HWPOISON test...%s\n", > + status_to_str(status)); > close(fd); > - if (status == TEST_FAILED) > - return -1; > } > > - return 0; > - > -create_failure: > - printf(ERROR_PREFIX "Abort test: failed to create hugetlbfs file\n"); > - return -1; > + ksft_finished(); > } > -- > 2.42.0 > >
On 1/13/24 6:08 AM, Jiaqi Yan wrote: > On Thu, Jan 11, 2024 at 11:21 PM Muhammad Usama Anjum > <usama.anjum@collabora.com> wrote: >> >> Conform the layout, informational and status messages to TAP. No >> functional change is intended other than the layout of output messages. >> >> Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com> >> --- >> Tested this by reverting the patch a08c7193e4f18dc8508f2d07d0de2c5b94cb39a3 >> ("mm/filemap: remove hugetlb special casing in filemap.c") as it has >> broken the test. The bug report can be found at [1]. >> >> Tested with proposed fix as well [2]. >> >> [1] https://lore.kernel.org/all/079335ab-190f-41f7-b832-6ffe7528fd8b@collabora.com >> [2] https://lore.kernel.org/all/a20e7bdb-7344-306d-e8f5-5ee69af7d5ea@oracle.com >> --- >> .../selftests/mm/hugetlb-read-hwpoison.c | 116 +++++++++--------- >> 1 file changed, 56 insertions(+), 60 deletions(-) >> >> diff --git a/tools/testing/selftests/mm/hugetlb-read-hwpoison.c b/tools/testing/selftests/mm/hugetlb-read-hwpoison.c >> index ba6cc6f9cabc..193ad7275df5 100644 >> --- a/tools/testing/selftests/mm/hugetlb-read-hwpoison.c >> +++ b/tools/testing/selftests/mm/hugetlb-read-hwpoison.c >> @@ -58,8 +58,8 @@ static bool verify_chunk(char *buf, size_t len, char val) >> >> for (i = 0; i < len; ++i) { >> if (buf[i] != val) { >> - printf(PREFIX ERROR_PREFIX "check fail: buf[%lu] = %u != %u\n", >> - i, buf[i], val); >> + ksft_print_msg(PREFIX ERROR_PREFIX "check fail: buf[%lu] = %u != %u\n", >> + i, buf[i], val); >> return false; >> } >> } >> @@ -75,9 +75,9 @@ static bool seek_read_hugepage_filemap(int fd, size_t len, size_t wr_chunk_size, >> ssize_t total_ret_count = 0; >> char val = offset / wr_chunk_size + offset % wr_chunk_size; >> >> - printf(PREFIX PREFIX "init val=%u with offset=0x%lx\n", val, offset); >> - printf(PREFIX PREFIX "expect to read 0x%lx bytes of data in total\n", >> - expected); >> + ksft_print_msg(PREFIX PREFIX "init val=%u with offset=0x%lx\n", val, offset); >> + ksft_print_msg(PREFIX PREFIX "expect to read 0x%lx bytes of data in total\n", >> + expected); >> if (lseek(fd, offset, SEEK_SET) < 0) { >> perror(PREFIX ERROR_PREFIX "seek failed"); >> return false; >> @@ -86,7 +86,7 @@ static bool seek_read_hugepage_filemap(int fd, size_t len, size_t wr_chunk_size, >> while (offset + total_ret_count < len) { >> ret_count = read(fd, buf, wr_chunk_size); >> if (ret_count == 0) { >> - printf(PREFIX PREFIX "read reach end of the file\n"); >> + ksft_print_msg(PREFIX PREFIX "read reach end of the file\n"); >> break; >> } else if (ret_count < 0) { >> perror(PREFIX ERROR_PREFIX "read failed"); >> @@ -98,8 +98,8 @@ static bool seek_read_hugepage_filemap(int fd, size_t len, size_t wr_chunk_size, >> >> total_ret_count += ret_count; >> } >> - printf(PREFIX PREFIX "actually read 0x%lx bytes of data in total\n", >> - total_ret_count); >> + ksft_print_msg(PREFIX PREFIX "actually read 0x%lx bytes of data in total\n", >> + total_ret_count); >> >> return total_ret_count == expected; >> } >> @@ -112,15 +112,15 @@ static bool read_hugepage_filemap(int fd, size_t len, >> ssize_t total_ret_count = 0; >> char val = 0; >> >> - printf(PREFIX PREFIX "expect to read 0x%lx bytes of data in total\n", >> - expected); >> + ksft_print_msg(PREFIX PREFIX "expect to read 0x%lx bytes of data in total\n", >> + expected); >> while (total_ret_count < len) { >> ret_count = read(fd, buf, wr_chunk_size); >> if (ret_count == 0) { >> - printf(PREFIX PREFIX "read reach end of the file\n"); >> + ksft_print_msg(PREFIX PREFIX "read reach end of the file\n"); >> break; >> } else if (ret_count < 0) { >> - perror(PREFIX ERROR_PREFIX "read failed"); >> + ksft_print_msg(PREFIX ERROR_PREFIX "read failed"); > > Should we also include strerror(errno) in log msg, like you did below? Looks like I missed it. I'll post a v3. > > Actually, would ksft_perror be a better choice for perror()s? It may or may not be. There aren't ksft_*_perror macros for printing the logs at this time. I prefer using just whatever macros are available. They aren't generic enough. Maybe mm tests use perror whenever error occurs, but other tests don't. > >> >> break; >> } >> ++val; >> @@ -129,8 +129,8 @@ static bool read_hugepage_filemap(int fd, size_t len, >> >> total_ret_count += ret_count; >> } >> - printf(PREFIX PREFIX "actually read 0x%lx bytes of data in total\n", >> - total_ret_count); >> + ksft_print_msg(PREFIX PREFIX "actually read 0x%lx bytes of data in total\n", >> + total_ret_count); >> >> return total_ret_count == expected; >> } >> @@ -142,14 +142,15 @@ test_hugetlb_read(int fd, size_t len, size_t wr_chunk_size) >> char *filemap = NULL; >> >> if (ftruncate(fd, len) < 0) { >> - perror(PREFIX ERROR_PREFIX "ftruncate failed"); >> + ksft_print_msg(PREFIX ERROR_PREFIX "ftruncate failed: %s\n", strerror(errno)); >> return status; >> } >> >> filemap = mmap(NULL, len, PROT_READ | PROT_WRITE, >> MAP_SHARED | MAP_POPULATE, fd, 0); >> if (filemap == MAP_FAILED) { >> - perror(PREFIX ERROR_PREFIX "mmap for primary mapping failed"); >> + ksft_print_msg(PREFIX ERROR_PREFIX "mmap for primary mapping failed: %s\n", >> + strerror(errno)); >> goto done; >> } >> >> @@ -162,7 +163,8 @@ test_hugetlb_read(int fd, size_t len, size_t wr_chunk_size) >> munmap(filemap, len); >> done: >> if (ftruncate(fd, 0) < 0) { >> - perror(PREFIX ERROR_PREFIX "ftruncate back to 0 failed"); >> + ksft_print_msg(PREFIX ERROR_PREFIX "ftruncate back to 0 failed : %s\n", >> + strerror(errno)); >> status = TEST_FAILED; >> } >> >> @@ -179,14 +181,15 @@ test_hugetlb_read_hwpoison(int fd, size_t len, size_t wr_chunk_size, >> const unsigned long pagesize = getpagesize(); >> >> if (ftruncate(fd, len) < 0) { >> - perror(PREFIX ERROR_PREFIX "ftruncate failed"); >> + ksft_print_msg(PREFIX ERROR_PREFIX "ftruncate failed: %s\n", strerror(errno)); >> return status; >> } >> >> filemap = mmap(NULL, len, PROT_READ | PROT_WRITE, >> MAP_SHARED | MAP_POPULATE, fd, 0); >> if (filemap == MAP_FAILED) { >> - perror(PREFIX ERROR_PREFIX "mmap for primary mapping failed"); >> + ksft_print_msg(PREFIX ERROR_PREFIX "mmap for primary mapping failed: %s\n", >> + strerror(errno)); >> goto done; >> } >> >> @@ -201,7 +204,7 @@ test_hugetlb_read_hwpoison(int fd, size_t len, size_t wr_chunk_size, >> */ >> hwp_addr = filemap + len / 2 + pagesize; >> if (madvise(hwp_addr, pagesize, MADV_HWPOISON) < 0) { >> - perror(PREFIX ERROR_PREFIX "MADV_HWPOISON failed"); >> + ksft_print_msg(PREFIX ERROR_PREFIX "MADV_HWPOISON failed: %s\n", strerror(errno)); >> goto unmap; >> } >> >> @@ -228,7 +231,8 @@ test_hugetlb_read_hwpoison(int fd, size_t len, size_t wr_chunk_size, >> munmap(filemap, len); >> done: >> if (ftruncate(fd, 0) < 0) { >> - perror(PREFIX ERROR_PREFIX "ftruncate back to 0 failed"); >> + ksft_print_msg(PREFIX ERROR_PREFIX "ftruncate back to 0 failed: %s\n", >> + strerror(errno)); >> status = TEST_FAILED; >> } >> >> @@ -240,27 +244,32 @@ static int create_hugetlbfs_file(struct statfs *file_stat) >> int fd; >> >> fd = memfd_create("hugetlb_tmp", MFD_HUGETLB); >> - if (fd < 0) { >> - perror(PREFIX ERROR_PREFIX "could not open hugetlbfs file"); >> - return -1; >> - } >> + if (fd < 0) >> + ksft_exit_fail_msg(PREFIX ERROR_PREFIX "could not open hugetlbfs file: %s\n", >> + strerror(errno)); >> >> memset(file_stat, 0, sizeof(*file_stat)); >> + >> if (fstatfs(fd, file_stat)) { >> - perror(PREFIX ERROR_PREFIX "fstatfs failed"); >> - goto close; >> + close(fd); >> + ksft_exit_fail_msg(PREFIX ERROR_PREFIX "fstatfs failed: %s\n", strerror(errno)); >> } >> if (file_stat->f_type != HUGETLBFS_MAGIC) { >> - printf(PREFIX ERROR_PREFIX "not hugetlbfs file\n"); >> - goto close; >> + close(fd); >> + ksft_exit_fail_msg(PREFIX ERROR_PREFIX "not hugetlbfs file\n"); >> } >> >> return fd; >> -close: >> - close(fd); >> - return -1; >> } >> >> +#define KSFT_PRINT_MSG(status, fmt, ...) \ >> + do { \ >> + if (status == TEST_SKIPPED) \ >> + ksft_test_result_skip(fmt, __VA_ARGS__); \ >> + else \ >> + ksft_test_result(status == TEST_PASSED, fmt, __VA_ARGS__); \ >> + } while (0) >> + >> int main(void) >> { >> int fd; >> @@ -273,50 +282,37 @@ int main(void) >> }; >> size_t i; >> >> + ksft_print_header(); >> + ksft_set_plan(12); >> + >> for (i = 0; i < ARRAY_SIZE(wr_chunk_sizes); ++i) { >> - printf("Write/read chunk size=0x%lx\n", >> - wr_chunk_sizes[i]); >> + ksft_print_msg("Write/read chunk size=0x%lx\n", >> + wr_chunk_sizes[i]); >> >> fd = create_hugetlbfs_file(&file_stat); >> - if (fd < 0) >> - goto create_failure; >> - printf(PREFIX "HugeTLB read regression test...\n"); >> + ksft_print_msg(PREFIX "HugeTLB read regression test...\n"); >> status = test_hugetlb_read(fd, file_stat.f_bsize, >> wr_chunk_sizes[i]); >> - printf(PREFIX "HugeTLB read regression test...%s\n", >> - status_to_str(status)); >> + KSFT_PRINT_MSG(status, PREFIX "HugeTLB read regression test...%s\n", >> + status_to_str(status)); >> close(fd); >> - if (status == TEST_FAILED) >> - return -1; >> >> fd = create_hugetlbfs_file(&file_stat); >> - if (fd < 0) >> - goto create_failure; >> - printf(PREFIX "HugeTLB read HWPOISON test...\n"); >> + ksft_print_msg(PREFIX "HugeTLB read HWPOISON test...\n"); >> status = test_hugetlb_read_hwpoison(fd, file_stat.f_bsize, >> wr_chunk_sizes[i], false); >> - printf(PREFIX "HugeTLB read HWPOISON test...%s\n", >> - status_to_str(status)); >> + KSFT_PRINT_MSG(status, PREFIX "HugeTLB read HWPOISON test..%s\n", >> + status_to_str(status)); >> close(fd); >> - if (status == TEST_FAILED) >> - return -1; >> >> fd = create_hugetlbfs_file(&file_stat); >> - if (fd < 0) >> - goto create_failure; >> - printf(PREFIX "HugeTLB seek then read HWPOISON test...\n"); >> + ksft_print_msg(PREFIX "HugeTLB seek then read HWPOISON test...\n"); >> status = test_hugetlb_read_hwpoison(fd, file_stat.f_bsize, >> wr_chunk_sizes[i], true); >> - printf(PREFIX "HugeTLB seek then read HWPOISON test...%s\n", >> - status_to_str(status)); >> + KSFT_PRINT_MSG(status, PREFIX "HugeTLB seek then read HWPOISON test...%s\n", >> + status_to_str(status)); >> close(fd); >> - if (status == TEST_FAILED) >> - return -1; >> } >> >> - return 0; >> - >> -create_failure: >> - printf(ERROR_PREFIX "Abort test: failed to create hugetlbfs file\n"); >> - return -1; >> + ksft_finished(); >> } >> -- >> 2.42.0 >> >> >
On Sun, Jan 14, 2024 at 10:32 PM Muhammad Usama Anjum <usama.anjum@collabora.com> wrote: > > On 1/13/24 6:08 AM, Jiaqi Yan wrote: > > On Thu, Jan 11, 2024 at 11:21 PM Muhammad Usama Anjum > > <usama.anjum@collabora.com> wrote: > >> > >> Conform the layout, informational and status messages to TAP. No > >> functional change is intended other than the layout of output messages. > >> > >> Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com> > >> --- > >> Tested this by reverting the patch a08c7193e4f18dc8508f2d07d0de2c5b94cb39a3 > >> ("mm/filemap: remove hugetlb special casing in filemap.c") as it has > >> broken the test. The bug report can be found at [1]. > >> > >> Tested with proposed fix as well [2]. > >> > >> [1] https://lore.kernel.org/all/079335ab-190f-41f7-b832-6ffe7528fd8b@collabora.com > >> [2] https://lore.kernel.org/all/a20e7bdb-7344-306d-e8f5-5ee69af7d5ea@oracle.com > >> --- > >> .../selftests/mm/hugetlb-read-hwpoison.c | 116 +++++++++--------- > >> 1 file changed, 56 insertions(+), 60 deletions(-) > >> > >> diff --git a/tools/testing/selftests/mm/hugetlb-read-hwpoison.c b/tools/testing/selftests/mm/hugetlb-read-hwpoison.c > >> index ba6cc6f9cabc..193ad7275df5 100644 > >> --- a/tools/testing/selftests/mm/hugetlb-read-hwpoison.c > >> +++ b/tools/testing/selftests/mm/hugetlb-read-hwpoison.c > >> @@ -58,8 +58,8 @@ static bool verify_chunk(char *buf, size_t len, char val) > >> > >> for (i = 0; i < len; ++i) { > >> if (buf[i] != val) { > >> - printf(PREFIX ERROR_PREFIX "check fail: buf[%lu] = %u != %u\n", > >> - i, buf[i], val); > >> + ksft_print_msg(PREFIX ERROR_PREFIX "check fail: buf[%lu] = %u != %u\n", > >> + i, buf[i], val); > >> return false; > >> } > >> } > >> @@ -75,9 +75,9 @@ static bool seek_read_hugepage_filemap(int fd, size_t len, size_t wr_chunk_size, > >> ssize_t total_ret_count = 0; > >> char val = offset / wr_chunk_size + offset % wr_chunk_size; > >> > >> - printf(PREFIX PREFIX "init val=%u with offset=0x%lx\n", val, offset); > >> - printf(PREFIX PREFIX "expect to read 0x%lx bytes of data in total\n", > >> - expected); > >> + ksft_print_msg(PREFIX PREFIX "init val=%u with offset=0x%lx\n", val, offset); > >> + ksft_print_msg(PREFIX PREFIX "expect to read 0x%lx bytes of data in total\n", > >> + expected); > >> if (lseek(fd, offset, SEEK_SET) < 0) { > >> perror(PREFIX ERROR_PREFIX "seek failed"); > >> return false; > >> @@ -86,7 +86,7 @@ static bool seek_read_hugepage_filemap(int fd, size_t len, size_t wr_chunk_size, > >> while (offset + total_ret_count < len) { > >> ret_count = read(fd, buf, wr_chunk_size); > >> if (ret_count == 0) { > >> - printf(PREFIX PREFIX "read reach end of the file\n"); > >> + ksft_print_msg(PREFIX PREFIX "read reach end of the file\n"); > >> break; > >> } else if (ret_count < 0) { > >> perror(PREFIX ERROR_PREFIX "read failed"); > >> @@ -98,8 +98,8 @@ static bool seek_read_hugepage_filemap(int fd, size_t len, size_t wr_chunk_size, > >> > >> total_ret_count += ret_count; > >> } > >> - printf(PREFIX PREFIX "actually read 0x%lx bytes of data in total\n", > >> - total_ret_count); > >> + ksft_print_msg(PREFIX PREFIX "actually read 0x%lx bytes of data in total\n", > >> + total_ret_count); > >> > >> return total_ret_count == expected; > >> } > >> @@ -112,15 +112,15 @@ static bool read_hugepage_filemap(int fd, size_t len, > >> ssize_t total_ret_count = 0; > >> char val = 0; > >> > >> - printf(PREFIX PREFIX "expect to read 0x%lx bytes of data in total\n", > >> - expected); > >> + ksft_print_msg(PREFIX PREFIX "expect to read 0x%lx bytes of data in total\n", > >> + expected); > >> while (total_ret_count < len) { > >> ret_count = read(fd, buf, wr_chunk_size); > >> if (ret_count == 0) { > >> - printf(PREFIX PREFIX "read reach end of the file\n"); > >> + ksft_print_msg(PREFIX PREFIX "read reach end of the file\n"); > >> break; > >> } else if (ret_count < 0) { > >> - perror(PREFIX ERROR_PREFIX "read failed"); > >> + ksft_print_msg(PREFIX ERROR_PREFIX "read failed"); > > > > Should we also include strerror(errno) in log msg, like you did below? > Looks like I missed it. I'll post a v3. > > > > Actually, would ksft_perror be a better choice for perror()s? > It may or may not be. There aren't ksft_*_perror macros for printing the > logs at this time. I prefer using just whatever macros are available. They > aren't generic enough. Maybe mm tests use perror whenever error occurs, but > other tests don't. can't you use this ksft_perror available today? https://github.com/torvalds/linux/blob/master/tools/testing/selftests/kselftest.h#L161C25-L172 > > > > >> > >> break; > >> } > >> ++val; > >> @@ -129,8 +129,8 @@ static bool read_hugepage_filemap(int fd, size_t len, > >> > >> total_ret_count += ret_count; > >> } > >> - printf(PREFIX PREFIX "actually read 0x%lx bytes of data in total\n", > >> - total_ret_count); > >> + ksft_print_msg(PREFIX PREFIX "actually read 0x%lx bytes of data in total\n", > >> + total_ret_count); > >> > >> return total_ret_count == expected; > >> } > >> @@ -142,14 +142,15 @@ test_hugetlb_read(int fd, size_t len, size_t wr_chunk_size) > >> char *filemap = NULL; > >> > >> if (ftruncate(fd, len) < 0) { > >> - perror(PREFIX ERROR_PREFIX "ftruncate failed"); > >> + ksft_print_msg(PREFIX ERROR_PREFIX "ftruncate failed: %s\n", strerror(errno)); > >> return status; > >> } > >> > >> filemap = mmap(NULL, len, PROT_READ | PROT_WRITE, > >> MAP_SHARED | MAP_POPULATE, fd, 0); > >> if (filemap == MAP_FAILED) { > >> - perror(PREFIX ERROR_PREFIX "mmap for primary mapping failed"); > >> + ksft_print_msg(PREFIX ERROR_PREFIX "mmap for primary mapping failed: %s\n", > >> + strerror(errno)); > >> goto done; > >> } > >> > >> @@ -162,7 +163,8 @@ test_hugetlb_read(int fd, size_t len, size_t wr_chunk_size) > >> munmap(filemap, len); > >> done: > >> if (ftruncate(fd, 0) < 0) { > >> - perror(PREFIX ERROR_PREFIX "ftruncate back to 0 failed"); > >> + ksft_print_msg(PREFIX ERROR_PREFIX "ftruncate back to 0 failed : %s\n", > >> + strerror(errno)); > >> status = TEST_FAILED; > >> } > >> > >> @@ -179,14 +181,15 @@ test_hugetlb_read_hwpoison(int fd, size_t len, size_t wr_chunk_size, > >> const unsigned long pagesize = getpagesize(); > >> > >> if (ftruncate(fd, len) < 0) { > >> - perror(PREFIX ERROR_PREFIX "ftruncate failed"); > >> + ksft_print_msg(PREFIX ERROR_PREFIX "ftruncate failed: %s\n", strerror(errno)); > >> return status; > >> } > >> > >> filemap = mmap(NULL, len, PROT_READ | PROT_WRITE, > >> MAP_SHARED | MAP_POPULATE, fd, 0); > >> if (filemap == MAP_FAILED) { > >> - perror(PREFIX ERROR_PREFIX "mmap for primary mapping failed"); > >> + ksft_print_msg(PREFIX ERROR_PREFIX "mmap for primary mapping failed: %s\n", > >> + strerror(errno)); > >> goto done; > >> } > >> > >> @@ -201,7 +204,7 @@ test_hugetlb_read_hwpoison(int fd, size_t len, size_t wr_chunk_size, > >> */ > >> hwp_addr = filemap + len / 2 + pagesize; > >> if (madvise(hwp_addr, pagesize, MADV_HWPOISON) < 0) { > >> - perror(PREFIX ERROR_PREFIX "MADV_HWPOISON failed"); > >> + ksft_print_msg(PREFIX ERROR_PREFIX "MADV_HWPOISON failed: %s\n", strerror(errno)); > >> goto unmap; > >> } > >> > >> @@ -228,7 +231,8 @@ test_hugetlb_read_hwpoison(int fd, size_t len, size_t wr_chunk_size, > >> munmap(filemap, len); > >> done: > >> if (ftruncate(fd, 0) < 0) { > >> - perror(PREFIX ERROR_PREFIX "ftruncate back to 0 failed"); > >> + ksft_print_msg(PREFIX ERROR_PREFIX "ftruncate back to 0 failed: %s\n", > >> + strerror(errno)); > >> status = TEST_FAILED; > >> } > >> > >> @@ -240,27 +244,32 @@ static int create_hugetlbfs_file(struct statfs *file_stat) > >> int fd; > >> > >> fd = memfd_create("hugetlb_tmp", MFD_HUGETLB); > >> - if (fd < 0) { > >> - perror(PREFIX ERROR_PREFIX "could not open hugetlbfs file"); > >> - return -1; > >> - } > >> + if (fd < 0) > >> + ksft_exit_fail_msg(PREFIX ERROR_PREFIX "could not open hugetlbfs file: %s\n", > >> + strerror(errno)); > >> > >> memset(file_stat, 0, sizeof(*file_stat)); > >> + > >> if (fstatfs(fd, file_stat)) { > >> - perror(PREFIX ERROR_PREFIX "fstatfs failed"); > >> - goto close; > >> + close(fd); > >> + ksft_exit_fail_msg(PREFIX ERROR_PREFIX "fstatfs failed: %s\n", strerror(errno)); > >> } > >> if (file_stat->f_type != HUGETLBFS_MAGIC) { > >> - printf(PREFIX ERROR_PREFIX "not hugetlbfs file\n"); > >> - goto close; > >> + close(fd); > >> + ksft_exit_fail_msg(PREFIX ERROR_PREFIX "not hugetlbfs file\n"); > >> } > >> > >> return fd; > >> -close: > >> - close(fd); > >> - return -1; > >> } > >> > >> +#define KSFT_PRINT_MSG(status, fmt, ...) \ > >> + do { \ > >> + if (status == TEST_SKIPPED) \ > >> + ksft_test_result_skip(fmt, __VA_ARGS__); \ > >> + else \ > >> + ksft_test_result(status == TEST_PASSED, fmt, __VA_ARGS__); \ > >> + } while (0) > >> + > >> int main(void) > >> { > >> int fd; > >> @@ -273,50 +282,37 @@ int main(void) > >> }; > >> size_t i; > >> > >> + ksft_print_header(); > >> + ksft_set_plan(12); > >> + > >> for (i = 0; i < ARRAY_SIZE(wr_chunk_sizes); ++i) { > >> - printf("Write/read chunk size=0x%lx\n", > >> - wr_chunk_sizes[i]); > >> + ksft_print_msg("Write/read chunk size=0x%lx\n", > >> + wr_chunk_sizes[i]); > >> > >> fd = create_hugetlbfs_file(&file_stat); > >> - if (fd < 0) > >> - goto create_failure; > >> - printf(PREFIX "HugeTLB read regression test...\n"); > >> + ksft_print_msg(PREFIX "HugeTLB read regression test...\n"); > >> status = test_hugetlb_read(fd, file_stat.f_bsize, > >> wr_chunk_sizes[i]); > >> - printf(PREFIX "HugeTLB read regression test...%s\n", > >> - status_to_str(status)); > >> + KSFT_PRINT_MSG(status, PREFIX "HugeTLB read regression test...%s\n", > >> + status_to_str(status)); > >> close(fd); > >> - if (status == TEST_FAILED) > >> - return -1; > >> > >> fd = create_hugetlbfs_file(&file_stat); > >> - if (fd < 0) > >> - goto create_failure; > >> - printf(PREFIX "HugeTLB read HWPOISON test...\n"); > >> + ksft_print_msg(PREFIX "HugeTLB read HWPOISON test...\n"); > >> status = test_hugetlb_read_hwpoison(fd, file_stat.f_bsize, > >> wr_chunk_sizes[i], false); > >> - printf(PREFIX "HugeTLB read HWPOISON test...%s\n", > >> - status_to_str(status)); > >> + KSFT_PRINT_MSG(status, PREFIX "HugeTLB read HWPOISON test..%s\n", > >> + status_to_str(status)); > >> close(fd); > >> - if (status == TEST_FAILED) > >> - return -1; > >> > >> fd = create_hugetlbfs_file(&file_stat); > >> - if (fd < 0) > >> - goto create_failure; > >> - printf(PREFIX "HugeTLB seek then read HWPOISON test...\n"); > >> + ksft_print_msg(PREFIX "HugeTLB seek then read HWPOISON test...\n"); > >> status = test_hugetlb_read_hwpoison(fd, file_stat.f_bsize, > >> wr_chunk_sizes[i], true); > >> - printf(PREFIX "HugeTLB seek then read HWPOISON test...%s\n", > >> - status_to_str(status)); > >> + KSFT_PRINT_MSG(status, PREFIX "HugeTLB seek then read HWPOISON test...%s\n", > >> + status_to_str(status)); > >> close(fd); > >> - if (status == TEST_FAILED) > >> - return -1; > >> } > >> > >> - return 0; > >> - > >> -create_failure: > >> - printf(ERROR_PREFIX "Abort test: failed to create hugetlbfs file\n"); > >> - return -1; > >> + ksft_finished(); > >> } > >> -- > >> 2.42.0 > >> > >> > > > > -- > BR, > Muhammad Usama Anjum
On 1/15/24 11:52 AM, Jiaqi Yan wrote: > On Sun, Jan 14, 2024 at 10:32 PM Muhammad Usama Anjum > <usama.anjum@collabora.com> wrote: >> >> On 1/13/24 6:08 AM, Jiaqi Yan wrote: >>> On Thu, Jan 11, 2024 at 11:21 PM Muhammad Usama Anjum >>> <usama.anjum@collabora.com> wrote: >>>> >>>> Conform the layout, informational and status messages to TAP. No >>>> functional change is intended other than the layout of output messages. >>>> >>>> Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com> >>>> --- >>>> Tested this by reverting the patch a08c7193e4f18dc8508f2d07d0de2c5b94cb39a3 >>>> ("mm/filemap: remove hugetlb special casing in filemap.c") as it has >>>> broken the test. The bug report can be found at [1]. >>>> >>>> Tested with proposed fix as well [2]. >>>> >>>> [1] https://lore.kernel.org/all/079335ab-190f-41f7-b832-6ffe7528fd8b@collabora.com >>>> [2] https://lore.kernel.org/all/a20e7bdb-7344-306d-e8f5-5ee69af7d5ea@oracle.com >>>> --- >>>> .../selftests/mm/hugetlb-read-hwpoison.c | 116 +++++++++--------- >>>> 1 file changed, 56 insertions(+), 60 deletions(-) >>>> >>>> diff --git a/tools/testing/selftests/mm/hugetlb-read-hwpoison.c b/tools/testing/selftests/mm/hugetlb-read-hwpoison.c >>>> index ba6cc6f9cabc..193ad7275df5 100644 >>>> --- a/tools/testing/selftests/mm/hugetlb-read-hwpoison.c >>>> +++ b/tools/testing/selftests/mm/hugetlb-read-hwpoison.c >>>> @@ -58,8 +58,8 @@ static bool verify_chunk(char *buf, size_t len, char val) >>>> >>>> for (i = 0; i < len; ++i) { >>>> if (buf[i] != val) { >>>> - printf(PREFIX ERROR_PREFIX "check fail: buf[%lu] = %u != %u\n", >>>> - i, buf[i], val); >>>> + ksft_print_msg(PREFIX ERROR_PREFIX "check fail: buf[%lu] = %u != %u\n", >>>> + i, buf[i], val); >>>> return false; >>>> } >>>> } >>>> @@ -75,9 +75,9 @@ static bool seek_read_hugepage_filemap(int fd, size_t len, size_t wr_chunk_size, >>>> ssize_t total_ret_count = 0; >>>> char val = offset / wr_chunk_size + offset % wr_chunk_size; >>>> >>>> - printf(PREFIX PREFIX "init val=%u with offset=0x%lx\n", val, offset); >>>> - printf(PREFIX PREFIX "expect to read 0x%lx bytes of data in total\n", >>>> - expected); >>>> + ksft_print_msg(PREFIX PREFIX "init val=%u with offset=0x%lx\n", val, offset); >>>> + ksft_print_msg(PREFIX PREFIX "expect to read 0x%lx bytes of data in total\n", >>>> + expected); >>>> if (lseek(fd, offset, SEEK_SET) < 0) { >>>> perror(PREFIX ERROR_PREFIX "seek failed"); >>>> return false; >>>> @@ -86,7 +86,7 @@ static bool seek_read_hugepage_filemap(int fd, size_t len, size_t wr_chunk_size, >>>> while (offset + total_ret_count < len) { >>>> ret_count = read(fd, buf, wr_chunk_size); >>>> if (ret_count == 0) { >>>> - printf(PREFIX PREFIX "read reach end of the file\n"); >>>> + ksft_print_msg(PREFIX PREFIX "read reach end of the file\n"); >>>> break; >>>> } else if (ret_count < 0) { >>>> perror(PREFIX ERROR_PREFIX "read failed"); >>>> @@ -98,8 +98,8 @@ static bool seek_read_hugepage_filemap(int fd, size_t len, size_t wr_chunk_size, >>>> >>>> total_ret_count += ret_count; >>>> } >>>> - printf(PREFIX PREFIX "actually read 0x%lx bytes of data in total\n", >>>> - total_ret_count); >>>> + ksft_print_msg(PREFIX PREFIX "actually read 0x%lx bytes of data in total\n", >>>> + total_ret_count); >>>> >>>> return total_ret_count == expected; >>>> } >>>> @@ -112,15 +112,15 @@ static bool read_hugepage_filemap(int fd, size_t len, >>>> ssize_t total_ret_count = 0; >>>> char val = 0; >>>> >>>> - printf(PREFIX PREFIX "expect to read 0x%lx bytes of data in total\n", >>>> - expected); >>>> + ksft_print_msg(PREFIX PREFIX "expect to read 0x%lx bytes of data in total\n", >>>> + expected); >>>> while (total_ret_count < len) { >>>> ret_count = read(fd, buf, wr_chunk_size); >>>> if (ret_count == 0) { >>>> - printf(PREFIX PREFIX "read reach end of the file\n"); >>>> + ksft_print_msg(PREFIX PREFIX "read reach end of the file\n"); >>>> break; >>>> } else if (ret_count < 0) { >>>> - perror(PREFIX ERROR_PREFIX "read failed"); >>>> + ksft_print_msg(PREFIX ERROR_PREFIX "read failed"); >>> >>> Should we also include strerror(errno) in log msg, like you did below? >> Looks like I missed it. I'll post a v3. >>> >>> Actually, would ksft_perror be a better choice for perror()s? >> It may or may not be. There aren't ksft_*_perror macros for printing the >> logs at this time. I prefer using just whatever macros are available. They >> aren't generic enough. Maybe mm tests use perror whenever error occurs, but >> other tests don't. > > can't you use this ksft_perror available today? > https://github.com/torvalds/linux/blob/master/tools/testing/selftests/kselftest.h#L161C25-L172 Sorry, I wan't able to find it. It has missing signature in header comment as well. I'll just update this current test. I'll use ksft_perror in future patches. Thanks for mentining it. > >> >>> >>>> >>>> break; >>>> } >>>> ++val; >>>> @@ -129,8 +129,8 @@ static bool read_hugepage_filemap(int fd, size_t len, >>>> >>>> total_ret_count += ret_count; >>>> } >>>> - printf(PREFIX PREFIX "actually read 0x%lx bytes of data in total\n", >>>> - total_ret_count); >>>> + ksft_print_msg(PREFIX PREFIX "actually read 0x%lx bytes of data in total\n", >>>> + total_ret_count); >>>> >>>> return total_ret_count == expected; >>>> } >>>> @@ -142,14 +142,15 @@ test_hugetlb_read(int fd, size_t len, size_t wr_chunk_size) >>>> char *filemap = NULL; >>>> >>>> if (ftruncate(fd, len) < 0) { >>>> - perror(PREFIX ERROR_PREFIX "ftruncate failed"); >>>> + ksft_print_msg(PREFIX ERROR_PREFIX "ftruncate failed: %s\n", strerror(errno)); >>>> return status; >>>> } >>>> >>>> filemap = mmap(NULL, len, PROT_READ | PROT_WRITE, >>>> MAP_SHARED | MAP_POPULATE, fd, 0); >>>> if (filemap == MAP_FAILED) { >>>> - perror(PREFIX ERROR_PREFIX "mmap for primary mapping failed"); >>>> + ksft_print_msg(PREFIX ERROR_PREFIX "mmap for primary mapping failed: %s\n", >>>> + strerror(errno)); >>>> goto done; >>>> } >>>> >>>> @@ -162,7 +163,8 @@ test_hugetlb_read(int fd, size_t len, size_t wr_chunk_size) >>>> munmap(filemap, len); >>>> done: >>>> if (ftruncate(fd, 0) < 0) { >>>> - perror(PREFIX ERROR_PREFIX "ftruncate back to 0 failed"); >>>> + ksft_print_msg(PREFIX ERROR_PREFIX "ftruncate back to 0 failed : %s\n", >>>> + strerror(errno)); >>>> status = TEST_FAILED; >>>> } >>>> >>>> @@ -179,14 +181,15 @@ test_hugetlb_read_hwpoison(int fd, size_t len, size_t wr_chunk_size, >>>> const unsigned long pagesize = getpagesize(); >>>> >>>> if (ftruncate(fd, len) < 0) { >>>> - perror(PREFIX ERROR_PREFIX "ftruncate failed"); >>>> + ksft_print_msg(PREFIX ERROR_PREFIX "ftruncate failed: %s\n", strerror(errno)); >>>> return status; >>>> } >>>> >>>> filemap = mmap(NULL, len, PROT_READ | PROT_WRITE, >>>> MAP_SHARED | MAP_POPULATE, fd, 0); >>>> if (filemap == MAP_FAILED) { >>>> - perror(PREFIX ERROR_PREFIX "mmap for primary mapping failed"); >>>> + ksft_print_msg(PREFIX ERROR_PREFIX "mmap for primary mapping failed: %s\n", >>>> + strerror(errno)); >>>> goto done; >>>> } >>>> >>>> @@ -201,7 +204,7 @@ test_hugetlb_read_hwpoison(int fd, size_t len, size_t wr_chunk_size, >>>> */ >>>> hwp_addr = filemap + len / 2 + pagesize; >>>> if (madvise(hwp_addr, pagesize, MADV_HWPOISON) < 0) { >>>> - perror(PREFIX ERROR_PREFIX "MADV_HWPOISON failed"); >>>> + ksft_print_msg(PREFIX ERROR_PREFIX "MADV_HWPOISON failed: %s\n", strerror(errno)); >>>> goto unmap; >>>> } >>>> >>>> @@ -228,7 +231,8 @@ test_hugetlb_read_hwpoison(int fd, size_t len, size_t wr_chunk_size, >>>> munmap(filemap, len); >>>> done: >>>> if (ftruncate(fd, 0) < 0) { >>>> - perror(PREFIX ERROR_PREFIX "ftruncate back to 0 failed"); >>>> + ksft_print_msg(PREFIX ERROR_PREFIX "ftruncate back to 0 failed: %s\n", >>>> + strerror(errno)); >>>> status = TEST_FAILED; >>>> } >>>> >>>> @@ -240,27 +244,32 @@ static int create_hugetlbfs_file(struct statfs *file_stat) >>>> int fd; >>>> >>>> fd = memfd_create("hugetlb_tmp", MFD_HUGETLB); >>>> - if (fd < 0) { >>>> - perror(PREFIX ERROR_PREFIX "could not open hugetlbfs file"); >>>> - return -1; >>>> - } >>>> + if (fd < 0) >>>> + ksft_exit_fail_msg(PREFIX ERROR_PREFIX "could not open hugetlbfs file: %s\n", >>>> + strerror(errno)); >>>> >>>> memset(file_stat, 0, sizeof(*file_stat)); >>>> + >>>> if (fstatfs(fd, file_stat)) { >>>> - perror(PREFIX ERROR_PREFIX "fstatfs failed"); >>>> - goto close; >>>> + close(fd); >>>> + ksft_exit_fail_msg(PREFIX ERROR_PREFIX "fstatfs failed: %s\n", strerror(errno)); >>>> } >>>> if (file_stat->f_type != HUGETLBFS_MAGIC) { >>>> - printf(PREFIX ERROR_PREFIX "not hugetlbfs file\n"); >>>> - goto close; >>>> + close(fd); >>>> + ksft_exit_fail_msg(PREFIX ERROR_PREFIX "not hugetlbfs file\n"); >>>> } >>>> >>>> return fd; >>>> -close: >>>> - close(fd); >>>> - return -1; >>>> } >>>> >>>> +#define KSFT_PRINT_MSG(status, fmt, ...) \ >>>> + do { \ >>>> + if (status == TEST_SKIPPED) \ >>>> + ksft_test_result_skip(fmt, __VA_ARGS__); \ >>>> + else \ >>>> + ksft_test_result(status == TEST_PASSED, fmt, __VA_ARGS__); \ >>>> + } while (0) >>>> + >>>> int main(void) >>>> { >>>> int fd; >>>> @@ -273,50 +282,37 @@ int main(void) >>>> }; >>>> size_t i; >>>> >>>> + ksft_print_header(); >>>> + ksft_set_plan(12); >>>> + >>>> for (i = 0; i < ARRAY_SIZE(wr_chunk_sizes); ++i) { >>>> - printf("Write/read chunk size=0x%lx\n", >>>> - wr_chunk_sizes[i]); >>>> + ksft_print_msg("Write/read chunk size=0x%lx\n", >>>> + wr_chunk_sizes[i]); >>>> >>>> fd = create_hugetlbfs_file(&file_stat); >>>> - if (fd < 0) >>>> - goto create_failure; >>>> - printf(PREFIX "HugeTLB read regression test...\n"); >>>> + ksft_print_msg(PREFIX "HugeTLB read regression test...\n"); >>>> status = test_hugetlb_read(fd, file_stat.f_bsize, >>>> wr_chunk_sizes[i]); >>>> - printf(PREFIX "HugeTLB read regression test...%s\n", >>>> - status_to_str(status)); >>>> + KSFT_PRINT_MSG(status, PREFIX "HugeTLB read regression test...%s\n", >>>> + status_to_str(status)); >>>> close(fd); >>>> - if (status == TEST_FAILED) >>>> - return -1; >>>> >>>> fd = create_hugetlbfs_file(&file_stat); >>>> - if (fd < 0) >>>> - goto create_failure; >>>> - printf(PREFIX "HugeTLB read HWPOISON test...\n"); >>>> + ksft_print_msg(PREFIX "HugeTLB read HWPOISON test...\n"); >>>> status = test_hugetlb_read_hwpoison(fd, file_stat.f_bsize, >>>> wr_chunk_sizes[i], false); >>>> - printf(PREFIX "HugeTLB read HWPOISON test...%s\n", >>>> - status_to_str(status)); >>>> + KSFT_PRINT_MSG(status, PREFIX "HugeTLB read HWPOISON test..%s\n", >>>> + status_to_str(status)); >>>> close(fd); >>>> - if (status == TEST_FAILED) >>>> - return -1; >>>> >>>> fd = create_hugetlbfs_file(&file_stat); >>>> - if (fd < 0) >>>> - goto create_failure; >>>> - printf(PREFIX "HugeTLB seek then read HWPOISON test...\n"); >>>> + ksft_print_msg(PREFIX "HugeTLB seek then read HWPOISON test...\n"); >>>> status = test_hugetlb_read_hwpoison(fd, file_stat.f_bsize, >>>> wr_chunk_sizes[i], true); >>>> - printf(PREFIX "HugeTLB seek then read HWPOISON test...%s\n", >>>> - status_to_str(status)); >>>> + KSFT_PRINT_MSG(status, PREFIX "HugeTLB seek then read HWPOISON test...%s\n", >>>> + status_to_str(status)); >>>> close(fd); >>>> - if (status == TEST_FAILED) >>>> - return -1; >>>> } >>>> >>>> - return 0; >>>> - >>>> -create_failure: >>>> - printf(ERROR_PREFIX "Abort test: failed to create hugetlbfs file\n"); >>>> - return -1; >>>> + ksft_finished(); >>>> } >>>> -- >>>> 2.42.0 >>>> >>>> >>> >> >> -- >> BR, >> Muhammad Usama Anjum >
diff --git a/tools/testing/selftests/mm/hugetlb-read-hwpoison.c b/tools/testing/selftests/mm/hugetlb-read-hwpoison.c index ba6cc6f9cabc..193ad7275df5 100644 --- a/tools/testing/selftests/mm/hugetlb-read-hwpoison.c +++ b/tools/testing/selftests/mm/hugetlb-read-hwpoison.c @@ -58,8 +58,8 @@ static bool verify_chunk(char *buf, size_t len, char val) for (i = 0; i < len; ++i) { if (buf[i] != val) { - printf(PREFIX ERROR_PREFIX "check fail: buf[%lu] = %u != %u\n", - i, buf[i], val); + ksft_print_msg(PREFIX ERROR_PREFIX "check fail: buf[%lu] = %u != %u\n", + i, buf[i], val); return false; } } @@ -75,9 +75,9 @@ static bool seek_read_hugepage_filemap(int fd, size_t len, size_t wr_chunk_size, ssize_t total_ret_count = 0; char val = offset / wr_chunk_size + offset % wr_chunk_size; - printf(PREFIX PREFIX "init val=%u with offset=0x%lx\n", val, offset); - printf(PREFIX PREFIX "expect to read 0x%lx bytes of data in total\n", - expected); + ksft_print_msg(PREFIX PREFIX "init val=%u with offset=0x%lx\n", val, offset); + ksft_print_msg(PREFIX PREFIX "expect to read 0x%lx bytes of data in total\n", + expected); if (lseek(fd, offset, SEEK_SET) < 0) { perror(PREFIX ERROR_PREFIX "seek failed"); return false; @@ -86,7 +86,7 @@ static bool seek_read_hugepage_filemap(int fd, size_t len, size_t wr_chunk_size, while (offset + total_ret_count < len) { ret_count = read(fd, buf, wr_chunk_size); if (ret_count == 0) { - printf(PREFIX PREFIX "read reach end of the file\n"); + ksft_print_msg(PREFIX PREFIX "read reach end of the file\n"); break; } else if (ret_count < 0) { perror(PREFIX ERROR_PREFIX "read failed"); @@ -98,8 +98,8 @@ static bool seek_read_hugepage_filemap(int fd, size_t len, size_t wr_chunk_size, total_ret_count += ret_count; } - printf(PREFIX PREFIX "actually read 0x%lx bytes of data in total\n", - total_ret_count); + ksft_print_msg(PREFIX PREFIX "actually read 0x%lx bytes of data in total\n", + total_ret_count); return total_ret_count == expected; } @@ -112,15 +112,15 @@ static bool read_hugepage_filemap(int fd, size_t len, ssize_t total_ret_count = 0; char val = 0; - printf(PREFIX PREFIX "expect to read 0x%lx bytes of data in total\n", - expected); + ksft_print_msg(PREFIX PREFIX "expect to read 0x%lx bytes of data in total\n", + expected); while (total_ret_count < len) { ret_count = read(fd, buf, wr_chunk_size); if (ret_count == 0) { - printf(PREFIX PREFIX "read reach end of the file\n"); + ksft_print_msg(PREFIX PREFIX "read reach end of the file\n"); break; } else if (ret_count < 0) { - perror(PREFIX ERROR_PREFIX "read failed"); + ksft_print_msg(PREFIX ERROR_PREFIX "read failed"); break; } ++val; @@ -129,8 +129,8 @@ static bool read_hugepage_filemap(int fd, size_t len, total_ret_count += ret_count; } - printf(PREFIX PREFIX "actually read 0x%lx bytes of data in total\n", - total_ret_count); + ksft_print_msg(PREFIX PREFIX "actually read 0x%lx bytes of data in total\n", + total_ret_count); return total_ret_count == expected; } @@ -142,14 +142,15 @@ test_hugetlb_read(int fd, size_t len, size_t wr_chunk_size) char *filemap = NULL; if (ftruncate(fd, len) < 0) { - perror(PREFIX ERROR_PREFIX "ftruncate failed"); + ksft_print_msg(PREFIX ERROR_PREFIX "ftruncate failed: %s\n", strerror(errno)); return status; } filemap = mmap(NULL, len, PROT_READ | PROT_WRITE, MAP_SHARED | MAP_POPULATE, fd, 0); if (filemap == MAP_FAILED) { - perror(PREFIX ERROR_PREFIX "mmap for primary mapping failed"); + ksft_print_msg(PREFIX ERROR_PREFIX "mmap for primary mapping failed: %s\n", + strerror(errno)); goto done; } @@ -162,7 +163,8 @@ test_hugetlb_read(int fd, size_t len, size_t wr_chunk_size) munmap(filemap, len); done: if (ftruncate(fd, 0) < 0) { - perror(PREFIX ERROR_PREFIX "ftruncate back to 0 failed"); + ksft_print_msg(PREFIX ERROR_PREFIX "ftruncate back to 0 failed : %s\n", + strerror(errno)); status = TEST_FAILED; } @@ -179,14 +181,15 @@ test_hugetlb_read_hwpoison(int fd, size_t len, size_t wr_chunk_size, const unsigned long pagesize = getpagesize(); if (ftruncate(fd, len) < 0) { - perror(PREFIX ERROR_PREFIX "ftruncate failed"); + ksft_print_msg(PREFIX ERROR_PREFIX "ftruncate failed: %s\n", strerror(errno)); return status; } filemap = mmap(NULL, len, PROT_READ | PROT_WRITE, MAP_SHARED | MAP_POPULATE, fd, 0); if (filemap == MAP_FAILED) { - perror(PREFIX ERROR_PREFIX "mmap for primary mapping failed"); + ksft_print_msg(PREFIX ERROR_PREFIX "mmap for primary mapping failed: %s\n", + strerror(errno)); goto done; } @@ -201,7 +204,7 @@ test_hugetlb_read_hwpoison(int fd, size_t len, size_t wr_chunk_size, */ hwp_addr = filemap + len / 2 + pagesize; if (madvise(hwp_addr, pagesize, MADV_HWPOISON) < 0) { - perror(PREFIX ERROR_PREFIX "MADV_HWPOISON failed"); + ksft_print_msg(PREFIX ERROR_PREFIX "MADV_HWPOISON failed: %s\n", strerror(errno)); goto unmap; } @@ -228,7 +231,8 @@ test_hugetlb_read_hwpoison(int fd, size_t len, size_t wr_chunk_size, munmap(filemap, len); done: if (ftruncate(fd, 0) < 0) { - perror(PREFIX ERROR_PREFIX "ftruncate back to 0 failed"); + ksft_print_msg(PREFIX ERROR_PREFIX "ftruncate back to 0 failed: %s\n", + strerror(errno)); status = TEST_FAILED; } @@ -240,27 +244,32 @@ static int create_hugetlbfs_file(struct statfs *file_stat) int fd; fd = memfd_create("hugetlb_tmp", MFD_HUGETLB); - if (fd < 0) { - perror(PREFIX ERROR_PREFIX "could not open hugetlbfs file"); - return -1; - } + if (fd < 0) + ksft_exit_fail_msg(PREFIX ERROR_PREFIX "could not open hugetlbfs file: %s\n", + strerror(errno)); memset(file_stat, 0, sizeof(*file_stat)); + if (fstatfs(fd, file_stat)) { - perror(PREFIX ERROR_PREFIX "fstatfs failed"); - goto close; + close(fd); + ksft_exit_fail_msg(PREFIX ERROR_PREFIX "fstatfs failed: %s\n", strerror(errno)); } if (file_stat->f_type != HUGETLBFS_MAGIC) { - printf(PREFIX ERROR_PREFIX "not hugetlbfs file\n"); - goto close; + close(fd); + ksft_exit_fail_msg(PREFIX ERROR_PREFIX "not hugetlbfs file\n"); } return fd; -close: - close(fd); - return -1; } +#define KSFT_PRINT_MSG(status, fmt, ...) \ + do { \ + if (status == TEST_SKIPPED) \ + ksft_test_result_skip(fmt, __VA_ARGS__); \ + else \ + ksft_test_result(status == TEST_PASSED, fmt, __VA_ARGS__); \ + } while (0) + int main(void) { int fd; @@ -273,50 +282,37 @@ int main(void) }; size_t i; + ksft_print_header(); + ksft_set_plan(12); + for (i = 0; i < ARRAY_SIZE(wr_chunk_sizes); ++i) { - printf("Write/read chunk size=0x%lx\n", - wr_chunk_sizes[i]); + ksft_print_msg("Write/read chunk size=0x%lx\n", + wr_chunk_sizes[i]); fd = create_hugetlbfs_file(&file_stat); - if (fd < 0) - goto create_failure; - printf(PREFIX "HugeTLB read regression test...\n"); + ksft_print_msg(PREFIX "HugeTLB read regression test...\n"); status = test_hugetlb_read(fd, file_stat.f_bsize, wr_chunk_sizes[i]); - printf(PREFIX "HugeTLB read regression test...%s\n", - status_to_str(status)); + KSFT_PRINT_MSG(status, PREFIX "HugeTLB read regression test...%s\n", + status_to_str(status)); close(fd); - if (status == TEST_FAILED) - return -1; fd = create_hugetlbfs_file(&file_stat); - if (fd < 0) - goto create_failure; - printf(PREFIX "HugeTLB read HWPOISON test...\n"); + ksft_print_msg(PREFIX "HugeTLB read HWPOISON test...\n"); status = test_hugetlb_read_hwpoison(fd, file_stat.f_bsize, wr_chunk_sizes[i], false); - printf(PREFIX "HugeTLB read HWPOISON test...%s\n", - status_to_str(status)); + KSFT_PRINT_MSG(status, PREFIX "HugeTLB read HWPOISON test...%s\n", + status_to_str(status)); close(fd); - if (status == TEST_FAILED) - return -1; fd = create_hugetlbfs_file(&file_stat); - if (fd < 0) - goto create_failure; - printf(PREFIX "HugeTLB seek then read HWPOISON test...\n"); + ksft_print_msg(PREFIX "HugeTLB seek then read HWPOISON test...\n"); status = test_hugetlb_read_hwpoison(fd, file_stat.f_bsize, wr_chunk_sizes[i], true); - printf(PREFIX "HugeTLB seek then read HWPOISON test...%s\n", - status_to_str(status)); + KSFT_PRINT_MSG(status, PREFIX "HugeTLB seek then read HWPOISON test...%s\n", + status_to_str(status)); close(fd); - if (status == TEST_FAILED) - return -1; } - return 0; - -create_failure: - printf(ERROR_PREFIX "Abort test: failed to create hugetlbfs file\n"); - return -1; + ksft_finished(); }
Conform the layout, informational and status messages to TAP. No functional change is intended other than the layout of output messages. Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com> --- Tested this by reverting the patch a08c7193e4f18dc8508f2d07d0de2c5b94cb39a3 ("mm/filemap: remove hugetlb special casing in filemap.c") as it has broken the test. The bug report can be found at [1]. Tested with proposed fix as well [2]. [1] https://lore.kernel.org/all/079335ab-190f-41f7-b832-6ffe7528fd8b@collabora.com [2] https://lore.kernel.org/all/a20e7bdb-7344-306d-e8f5-5ee69af7d5ea@oracle.com --- .../selftests/mm/hugetlb-read-hwpoison.c | 116 +++++++++--------- 1 file changed, 56 insertions(+), 60 deletions(-)