Re: [PATCH 11/53] selftests/mm: hugetlb-read-hwpoison: use kselftest framework

[Donet Tom <donettom@linux.ibm.com>]

On 4/6/26 7:46 PM, Mike Rapoport wrote:
> From: "Mike Rapoport (Microsoft)" <rppt@kernel.org>
>
> Convert hugetlb-read-hwpoison test to use kselftest framework for
> reporting and tracking successful and failing runs.
>
> Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
> ---
>   .../selftests/mm/hugetlb-read-hwpoison.c      | 118 +++++++++---------
>   1 file changed, 57 insertions(+), 61 deletions(-)
>
> diff --git a/tools/testing/selftests/mm/hugetlb-read-hwpoison.c b/tools/testing/selftests/mm/hugetlb-read-hwpoison.c
> index 6bbf15f78061..01cd5465e0f8 100644
> --- a/tools/testing/selftests/mm/hugetlb-read-hwpoison.c
> +++ b/tools/testing/selftests/mm/hugetlb-read-hwpoison.c
> @@ -13,9 +13,7 @@
>   #include <signal.h>
>   
>   #include "kselftest.h"
> -
> -#define PREFIX " ... "
> -#define ERROR_PREFIX " !!! "
> +#include "vm_util.h"
>   
>   #define MAX_WRITE_READ_CHUNK_SIZE (getpagesize() * 16)
>   #define MAX(a, b) (((a) > (b)) ? (a) : (b))
> @@ -26,17 +24,22 @@ enum test_status {
>   	TEST_SKIPPED = 2,
>   };
>   
> -static char *status_to_str(enum test_status status)
> +static void report_status(enum test_status status, const char *test_name,
> +			  size_t chunk_size)
>   {
>   	switch (status) {
>   	case TEST_PASSED:
> -		return "TEST_PASSED";
> +		ksft_test_result_pass("%s chunk_size=0x%lx\n",
> +				      test_name, chunk_size);
> +		break;
>   	case TEST_FAILED:
> -		return "TEST_FAILED";
> +		ksft_test_result_fail("%s chunk_size=0x%lx\n",
> +				      test_name, chunk_size);
> +		break;
>   	case TEST_SKIPPED:
> -		return "TEST_SKIPPED";
> -	default:
> -		return "TEST_???";
> +		ksft_test_result_skip("%s chunk_size=0x%lx\n",
> +				      test_name, chunk_size);
> +		break;
>   	}
>   }
>   
> @@ -59,8 +62,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("check fail: buf[%lu] = %u != %u\n",
> +				       i, buf[i], val);
>   			return false;
>   		}
>   	}
> @@ -76,21 +79,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("init val=%u with offset=0x%lx\n", val, offset);
> +	ksft_print_msg("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("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("read reach end of the file\n");
>   			break;
>   		} else if (ret_count < 0) {
> -			perror(PREFIX ERROR_PREFIX "read failed");
> +			ksft_perror("read failed");
>   			break;
>   		}
>   		++val;
> @@ -99,8 +102,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("actually read 0x%lx bytes of data in total\n",
> +		       total_ret_count);
>   
>   	return total_ret_count == expected;
>   }
> @@ -113,15 +116,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("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("read reach end of the file\n");
>   			break;
>   		} else if (ret_count < 0) {
> -			perror(PREFIX ERROR_PREFIX "read failed");
> +			ksft_perror("read failed");
>   			break;
>   		}
>   		++val;
> @@ -130,8 +133,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("actually read 0x%lx bytes of data in total\n",
> +		       total_ret_count);
>   
>   	return total_ret_count == expected;
>   }
> @@ -143,14 +146,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("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("mmap for primary mapping failed");
>   		goto done;
>   	}
>   
> @@ -163,7 +166,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("ftruncate back to 0 failed");
>   		status = TEST_FAILED;
>   	}
>   
> @@ -180,14 +183,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("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("mmap for primary mapping failed");
>   		goto done;
>   	}
>   
> @@ -202,7 +205,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("MADV_HWPOISON failed");
>   		goto unmap;
>   	}
>   
> @@ -229,7 +232,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("ftruncate back to 0 failed");
>   		status = TEST_FAILED;
>   	}
>   
> @@ -242,17 +245,17 @@ static int create_hugetlbfs_file(struct statfs *file_stat)
>   
>   	fd = memfd_create("hugetlb_tmp", MFD_HUGETLB);
>   	if (fd < 0) {
> -		perror(PREFIX ERROR_PREFIX "could not open hugetlbfs file");
> +		ksft_perror("could not open hugetlbfs file");
>   		return -1;
>   	}
>   
>   	memset(file_stat, 0, sizeof(*file_stat));
>   	if (fstatfs(fd, file_stat)) {
> -		perror(PREFIX ERROR_PREFIX "fstatfs failed");
> +		ksft_perror("fstatfs failed");
>   		goto close;
>   	}
>   	if (file_stat->f_type != HUGETLBFS_MAGIC) {
> -		printf(PREFIX ERROR_PREFIX "not hugetlbfs file\n");
> +		ksft_print_msg("not hugetlbfs file\n");
>   		goto close;
>   	}
>   
> @@ -264,7 +267,7 @@ static int create_hugetlbfs_file(struct statfs *file_stat)
>   
>   static void sigbus_handler(int sig)
>   {
> -	printf(PREFIX "received SIGBUS\n");
> +	ksft_print_msg("received SIGBUS\n");
>   }
>   
>   int main(void)
> @@ -279,51 +282,44 @@ int main(void)
>   	};
>   	size_t i;
>   
> +	ksft_print_header();
> +	ksft_set_plan(ARRAY_SIZE(wr_chunk_sizes) * 3);
> +
>   	signal(SIGBUS, sigbus_handler);
>   	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_exit_fail_msg("Failed to create hugetlbfs file\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));
>   		close(fd);
> -		if (status == TEST_FAILED)
> -			return -1;
> +		report_status(status, "HugeTLB read regression",
> +			      wr_chunk_sizes[i]);
>   
>   		fd = create_hugetlbfs_file(&file_stat);
>   		if (fd < 0)
> -			goto create_failure;
> -		printf(PREFIX "HugeTLB read HWPOISON test...\n");
> +			ksft_exit_fail_msg("Failed to create hugetlbfs file\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));
>   		close(fd);
> -		if (status == TEST_FAILED)
> -			return -1;
> +		report_status(status, "HugeTLB read HWPOISON",
> +			      wr_chunk_sizes[i]);
>   
>   		fd = create_hugetlbfs_file(&file_stat);
>   		if (fd < 0)
> -			goto create_failure;
> -		printf(PREFIX "HugeTLB seek then read HWPOISON test...\n");
> +			ksft_exit_fail_msg("Failed to create hugetlbfs file\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));
>   		close(fd);
> -		if (status == TEST_FAILED)
> -			return -1;
> +		report_status(status, "HugeTLB seek then read HWPOISON",
> +			      wr_chunk_sizes[i]);
>   	}
>   
> -	return 0;
> -
> -create_failure:
> -	printf(ERROR_PREFIX "Abort test: failed to create hugetlbfs file\n");
> -	return -1;
> +	ksft_finished();
>   }


LGTM

Reviewed by: Donet Tom <donettom@linux.ibm.com>

-Donet