Re: [PATCH] selftests/mm: check file initialization writes in split_huge_page_test

["David Hildenbrand (Arm)" <david@kernel.org>]

On 5/12/26 09:49, Vineet Agarwal wrote:
> create_pagecache_thp_and_fd() fills the backing file for the
> pagecache THP tests using repeated write() calls, but the return
> value is never checked.
> 
> If a write fails or completes only partially, the test may continue
> with an incompletely initialized file and produce misleading results.
> 
> Check the result of write() and fail the test if the expected number
> of bytes was not written.
> 
> Signed-off-by: Vineet Agarwal <agarwal.vineet2006@gmail.com>
> ---
>  tools/testing/selftests/mm/split_huge_page_test.c | 13 ++++++++++---
>  1 file changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/testing/selftests/mm/split_huge_page_test.c b/tools/testing/selftests/mm/split_huge_page_test.c
> index 500d07c4938b..eab69b0f59a0 100644
> --- a/tools/testing/selftests/mm/split_huge_page_test.c
> +++ b/tools/testing/selftests/mm/split_huge_page_test.c
> @@ -609,9 +609,16 @@ static int create_pagecache_thp_and_fd(const char *testfile, size_t fd_size,
>  	assert(fd_size % sizeof(buf) == 0);
>  	for (i = 0; i < sizeof(buf); i++)
>  		buf[i] = (unsigned char)i;
> -	for (i = 0; i < fd_size; i += sizeof(buf))
> -		write(*fd, buf, sizeof(buf));
> -
> +	for (i = 0; i < fd_size; i += sizeof(buf)) {
> +		ssize_t written;

Do you really need the temporary variable?

> +
> +		written = write(*fd, buf, sizeof(buf));
> +		if (written != sizeof(buf)) {
> +			ksft_perror("write testfile");
> +			close(*fd);
> +			goto err_out_unlink;
> +		}
> +	}
>  	close(*fd);
>  	sync();
>  	*fd = open("/proc/sys/vm/drop_caches", O_WRONLY);

Apart from that LGTM

Acked-by: David Hildenbrand (Arm) <david@kernel.org>

-- 
Cheers,

David