On Fri, 15 May 2026 09:22:11 +0000 Wei Yang <[email protected]> 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 <[email protected]>
> >---
> > 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.
> 


Reply via email to