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

[Andrew Morton <akpm@linux-foundation.org>]

On Fri, 15 May 2026 09:22:11 +0000 Wei Yang <richard.weiyang@gmail.com> wrote:

> 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?

AI review suggested the same:
	https://sashiko.dev/#/patchset/20260512074924.27721-1-agarwal.vineet2006%40gmail.com

And it complained about the handling of short writes in the added code.

> 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.

Not understanding - cab you please describe more completely?

> How about fix that at the same time.
> 
> Generally, the change look good.
>