On Mon, May 18, 2026 at 04:35:53PM -0700, Andrew Morton wrote:
>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?
>
I mean do this below.
diff --git a/tools/testing/selftests/mm/split_huge_page_test.c
b/tools/testing/selftests/mm/split_huge_page_test.c
index 460fa1f606fd..f30402ece608 100644
--- a/tools/testing/selftests/mm/split_huge_page_test.c
+++ b/tools/testing/selftests/mm/split_huge_page_test.c
@@ -469,7 +469,7 @@ static int create_pagecache_thp_and_fd(const char
*testfile, size_t fd_size,
}
if (write(*fd, "3", 1) != 1) {
ksft_perror("write to drop_caches");
- goto err_out_unlink;
+ goto err_out_close;
}
close(*fd);
On error writing this fd, /proc/sys/vm/drop_caches, we forget to close it.
>> How about fix that at the same time.
>>
>> Generally, the change look good.
>>
--
Wei Yang
Help you, Help me
