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

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

[Vineet Agarwal]

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;
+
+		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);
-- 
2.54.0

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

[David Hildenbrand (Arm)]

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

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

[Wei Yang]

On Tue, May 12, 2026 at 01:19:24PM +0530, 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;
>+
>+		written = write(*fd, buf, sizeof(buf));
>+		if (written != sizeof(buf)) {
>+			ksft_perror("write testfile");
>+			close(*fd);
>+			goto err_out_unlink;

Maybe we can "goto err_out_close" and remove the close() here?

Apart from this, I found on error writing to /proc/sys/vm/drop_caches in below,
it just goto err_out_unlink, which left fd open.

How about fix that at the same time.

Generally, the change look good.

>+		}
>+	}
> 	close(*fd);
> 	sync();
> 	*fd = open("/proc/sys/vm/drop_caches", O_WRONLY);
>-- 
>2.54.0
>

-- 
Wei Yang
Help you, Help me