| Message ID | 20240115073247.1280266-5-usama.anjum@collabora.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v3,1/7] selftests/mm: hugepage-shm: conform test to TAP format output | expand |
On Sun, Jan 14, 2024 at 11:33 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> > --- > Changes in v3: > - Use ksft_perror as short hand instead of missing strerror(errno) at > one place Minor thing: I think we should preserve previous changelogs, right? > > 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, 54 insertions(+), 62 deletions(-) > > diff --git a/tools/testing/selftests/mm/hugetlb-read-hwpoison.c b/tools/testing/selftests/mm/hugetlb-read-hwpoison.c > index ba6cc6f9cabc..23b41b88c6af 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,21 +75,21 @@ 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"); > + ksft_perror(PREFIX ERROR_PREFIX "seek failed"); > return false; > } > > 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"); > + ksft_perror(PREFIX ERROR_PREFIX "read failed"); > break; > } > ++val; > @@ -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_perror(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,14 @@ 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_perror(PREFIX ERROR_PREFIX "ftruncate failed"); > 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_perror(PREFIX ERROR_PREFIX "mmap for primary mapping failed"); > goto done; > } > > @@ -162,7 +162,7 @@ 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_perror(PREFIX ERROR_PREFIX "ftruncate back to 0 failed"); > status = TEST_FAILED; > } > > @@ -179,14 +179,14 @@ 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_perror(PREFIX ERROR_PREFIX "ftruncate failed"); > 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_perror(PREFIX ERROR_PREFIX "mmap for primary mapping failed"); > goto done; > } > > @@ -201,7 +201,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_perror(PREFIX ERROR_PREFIX "MADV_HWPOISON failed"); > goto unmap; > } > > @@ -228,7 +228,7 @@ 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_perror(PREFIX ERROR_PREFIX "ftruncate back to 0 failed"); > status = TEST_FAILED; > } > > @@ -240,27 +240,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 +278,37 @@ int main(void) > }; > size_t i; > > + ksft_print_header(); > + ksft_set_plan(12); Minor: can this number be calculated, or at least defined as a macro with documents? That would make it easier for reading. > + > 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 > > This version looks good to me. Maybe someone else need to take another look, just add mine: Reviewed-by: Jiaqi Yan <jiaqiyan@google.com>
On 1/16/24 10:07 PM, Jiaqi Yan wrote: > On Sun, Jan 14, 2024 at 11:33 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> >> --- >> Changes in v3: >> - Use ksft_perror as short hand instead of missing strerror(errno) at >> one place > > Minor thing: I think we should preserve previous changelogs, right? Definately. There was no changes in v2 for this patch. Hence there isn't any changelog before this series. > >> >> 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, 54 insertions(+), 62 deletions(-) >> >> diff --git a/tools/testing/selftests/mm/hugetlb-read-hwpoison.c b/tools/testing/selftests/mm/hugetlb-read-hwpoison.c >> index ba6cc6f9cabc..23b41b88c6af 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,21 +75,21 @@ 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"); >> + ksft_perror(PREFIX ERROR_PREFIX "seek failed"); >> return false; >> } >> >> 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"); >> + ksft_perror(PREFIX ERROR_PREFIX "read failed"); >> break; >> } >> ++val; >> @@ -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_perror(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,14 @@ 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_perror(PREFIX ERROR_PREFIX "ftruncate failed"); >> 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_perror(PREFIX ERROR_PREFIX "mmap for primary mapping failed"); >> goto done; >> } >> >> @@ -162,7 +162,7 @@ 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_perror(PREFIX ERROR_PREFIX "ftruncate back to 0 failed"); >> status = TEST_FAILED; >> } >> >> @@ -179,14 +179,14 @@ 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_perror(PREFIX ERROR_PREFIX "ftruncate failed"); >> 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_perror(PREFIX ERROR_PREFIX "mmap for primary mapping failed"); >> goto done; >> } >> >> @@ -201,7 +201,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_perror(PREFIX ERROR_PREFIX "MADV_HWPOISON failed"); >> goto unmap; >> } >> >> @@ -228,7 +228,7 @@ 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_perror(PREFIX ERROR_PREFIX "ftruncate back to 0 failed"); >> status = TEST_FAILED; >> } >> >> @@ -240,27 +240,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 +278,37 @@ int main(void) >> }; >> size_t i; >> >> + ksft_print_header(); >> + ksft_set_plan(12); > > Minor: can this number be calculated, or at least defined as a macro > with documents? That would make it easier for reading. The number can be calculated to some extent via macro. As all the tests don't define a macro for this, I'll prepare one patch for all the tests later on. > >> + >> 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 >> >> > > This version looks good to me. Maybe someone else need to take another > look, just add mine: > > Reviewed-by: Jiaqi Yan <jiaqiyan@google.com> Thanks >
diff --git a/tools/testing/selftests/mm/hugetlb-read-hwpoison.c b/tools/testing/selftests/mm/hugetlb-read-hwpoison.c index ba6cc6f9cabc..23b41b88c6af 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,21 +75,21 @@ 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"); + ksft_perror(PREFIX ERROR_PREFIX "seek failed"); return false; } 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"); + ksft_perror(PREFIX ERROR_PREFIX "read failed"); break; } ++val; @@ -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_perror(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,14 @@ 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_perror(PREFIX ERROR_PREFIX "ftruncate failed"); 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_perror(PREFIX ERROR_PREFIX "mmap for primary mapping failed"); goto done; } @@ -162,7 +162,7 @@ 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_perror(PREFIX ERROR_PREFIX "ftruncate back to 0 failed"); status = TEST_FAILED; } @@ -179,14 +179,14 @@ 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_perror(PREFIX ERROR_PREFIX "ftruncate failed"); 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_perror(PREFIX ERROR_PREFIX "mmap for primary mapping failed"); goto done; } @@ -201,7 +201,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_perror(PREFIX ERROR_PREFIX "MADV_HWPOISON failed"); goto unmap; } @@ -228,7 +228,7 @@ 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_perror(PREFIX ERROR_PREFIX "ftruncate back to 0 failed"); status = TEST_FAILED; } @@ -240,27 +240,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 +278,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> --- Changes in v3: - Use ksft_perror as short hand instead of missing strerror(errno) at one place 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, 54 insertions(+), 62 deletions(-)