Re: [PATCH v2 5/7] selftests/mm: hugetlb-read-hwpoison: conform test to TAP format output

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Jan 11, 2024 at 11:21 PM Muhammad Usama Anjum
<usama.anjum@xxxxxxxxxxxxx> 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@xxxxxxxxxxxxx>
> ---
> 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@xxxxxxxxxxxxx
> [2] https://lore.kernel.org/all/a20e7bdb-7344-306d-e8f5-5ee69af7d5ea@xxxxxxxxxx
> ---
>  .../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
>
>





[Index of Archives]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Share Photos]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux