[PATCH] selftests/mm: khugepaged: handle partial write in file_setup_area()

[PATCH] selftests/mm: khugepaged: handle partial write in file_setup_area()

[Vineet Agarwal]

file_setup_area() writes initialized test data into the backing
file used for collapse testing, but it ignores the return value
from write().

If write() fails or completes only partially, the test continues
with incomplete file contents, which can lead to incorrect test
results and make failures harder to diagnose.

Handle partial writes by retrying until all data is written and
abort the test if write() fails.

Signed-off-by: Vineet Agarwal <agarwal.vineet2006@gmail.com>
---
 tools/testing/selftests/mm/khugepaged.c | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/mm/khugepaged.c b/tools/testing/selftests/mm/khugepaged.c
index 3fe7ef04ac62..c13f06758750 100644
--- a/tools/testing/selftests/mm/khugepaged.c
+++ b/tools/testing/selftests/mm/khugepaged.c
@@ -369,7 +369,9 @@ static void *file_setup_area(int nr_hpages)
 	int fd;
 	void *p;
 	unsigned long size;
-
+	size_t remaining;
+	ssize_t ret;
+	char *buf;
 	unlink(finfo.path);  /* Cleanup from previous failed tests */
 	printf("Creating %s for collapse%s...", finfo.path,
 	       finfo.type == VMA_SHMEM ? " (tmpfs)" : "");
@@ -383,7 +385,19 @@ static void *file_setup_area(int nr_hpages)
 	size = nr_hpages * hpage_pmd_size;
 	p = alloc_mapping(nr_hpages);
 	fill_memory(p, 0, size);
-	write(fd, p, size);
+	remaining = size;
+	buf = p;
+
+	while (remaining > 0) {
+		ret = write(fd, buf, remaining);
+		if (ret <= 0) {
+			close(fd);
+			munmap(p, size);
+			ksft_exit_fail_msg("write() failed while preparing test file\n");
+		}
+		buf += ret;
+		remaining -= ret;
+	}
 	close(fd);
 	munmap(p, size);
 	success("OK");
-- 
2.54.0

Re: [PATCH] selftests/mm: khugepaged: handle partial write in file_setup_area()

[David Hildenbrand (Arm)]

On 4/28/26 08:23, Vineet Agarwal wrote:
> file_setup_area() writes initialized test data into the backing
> file used for collapse testing, but it ignores the return value
> from write().
> 
> If write() fails or completes only partially, the test continues
> with incomplete file contents, which can lead to incorrect test
> results and make failures harder to diagnose.
> 
> Handle partial writes by retrying until all data is written and
> abort the test if write() fails.
> 
> Signed-off-by: Vineet Agarwal <agarwal.vineet2006@gmail.com>
> ---
>  tools/testing/selftests/mm/khugepaged.c | 18 ++++++++++++++++--
>  1 file changed, 16 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/testing/selftests/mm/khugepaged.c b/tools/testing/selftests/mm/khugepaged.c
> index 3fe7ef04ac62..c13f06758750 100644
> --- a/tools/testing/selftests/mm/khugepaged.c
> +++ b/tools/testing/selftests/mm/khugepaged.c
> @@ -369,7 +369,9 @@ static void *file_setup_area(int nr_hpages)
>  	int fd;
>  	void *p;
>  	unsigned long size;
> -
> +	size_t remaining;
> +	ssize_t ret;
> +	char *buf;
>  	unlink(finfo.path);  /* Cleanup from previous failed tests */
>  	printf("Creating %s for collapse%s...", finfo.path,
>  	       finfo.type == VMA_SHMEM ? " (tmpfs)" : "");
> @@ -383,7 +385,19 @@ static void *file_setup_area(int nr_hpages)
>  	size = nr_hpages * hpage_pmd_size;
>  	p = alloc_mapping(nr_hpages);
>  	fill_memory(p, 0, size);
> -	write(fd, p, size);
> +	remaining = size;
> +	buf = p;
> +
> +	while (remaining > 0) {
> +		ret = write(fd, buf, remaining);
> +		if (ret <= 0) {
> +			close(fd);
> +			munmap(p, size);
> +			ksft_exit_fail_msg("write() failed while preparing test file\n");
> +		}
> +		buf += ret;
> +		remaining -= ret;
> +	}

Why are we even using write() instead of just mapping the fd and writing to that
area?

diff --git a/tools/testing/selftests/mm/khugepaged.c
b/tools/testing/selftests/mm/khugepaged.c
index 3fe7ef04ac62..a77f753ccf38 100644
--- a/tools/testing/selftests/mm/khugepaged.c
+++ b/tools/testing/selftests/mm/khugepaged.c
@@ -381,9 +381,10 @@ static void *file_setup_area(int nr_hpages)
        }

        size = nr_hpages * hpage_pmd_size;
-       p = alloc_mapping(nr_hpages);
+       p = mmap(BASE_ADDR, size, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0);
+       if (p != BASE_ADDR)
+               ksft_exit_fail_msg("mmap() failed while preparing test file\n");
        fill_memory(p, 0, size);
-       write(fd, p, size);
        close(fd);
        munmap(p, size);
        success("OK");


-- 
Cheers,

David

Re: [PATCH] selftests/mm: khugepaged: handle partial write in file_setup_area()

[Vineet Agarwal]

[-- Attachment #1: Type: text/plain, Size: 3448 bytes --]

Thanks for pointing that out.

I’ll take a closer look at whether mapping the file descriptor directly and
initializing the mapped area would be a cleaner approach here instead of
relying on write().

If mmap-based initialization works for this test setup, that would indeed
avoid the partial write handling entirely. I’ll investigate and send a v2
accordingly.

Thanks,
Vineet


On Tue, 28 Apr 2026 at 13:34, David Hildenbrand (Arm) <david@kernel.org>
wrote:

> On 4/28/26 08:23, Vineet Agarwal wrote:
> > file_setup_area() writes initialized test data into the backing
> > file used for collapse testing, but it ignores the return value
> > from write().
> >
> > If write() fails or completes only partially, the test continues
> > with incomplete file contents, which can lead to incorrect test
> > results and make failures harder to diagnose.
> >
> > Handle partial writes by retrying until all data is written and
> > abort the test if write() fails.
> >
> > Signed-off-by: Vineet Agarwal <agarwal.vineet2006@gmail.com>
> > ---
> >  tools/testing/selftests/mm/khugepaged.c | 18 ++++++++++++++++--
> >  1 file changed, 16 insertions(+), 2 deletions(-)
> >
> > diff --git a/tools/testing/selftests/mm/khugepaged.c
> b/tools/testing/selftests/mm/khugepaged.c
> > index 3fe7ef04ac62..c13f06758750 100644
> > --- a/tools/testing/selftests/mm/khugepaged.c
> > +++ b/tools/testing/selftests/mm/khugepaged.c
> > @@ -369,7 +369,9 @@ static void *file_setup_area(int nr_hpages)
> >       int fd;
> >       void *p;
> >       unsigned long size;
> > -
> > +     size_t remaining;
> > +     ssize_t ret;
> > +     char *buf;
> >       unlink(finfo.path);  /* Cleanup from previous failed tests */
> >       printf("Creating %s for collapse%s...", finfo.path,
> >              finfo.type == VMA_SHMEM ? " (tmpfs)" : "");
> > @@ -383,7 +385,19 @@ static void *file_setup_area(int nr_hpages)
> >       size = nr_hpages * hpage_pmd_size;
> >       p = alloc_mapping(nr_hpages);
> >       fill_memory(p, 0, size);
> > -     write(fd, p, size);
> > +     remaining = size;
> > +     buf = p;
> > +
> > +     while (remaining > 0) {
> > +             ret = write(fd, buf, remaining);
> > +             if (ret <= 0) {
> > +                     close(fd);
> > +                     munmap(p, size);
> > +                     ksft_exit_fail_msg("write() failed while preparing
> test file\n");
> > +             }
> > +             buf += ret;
> > +             remaining -= ret;
> > +     }
>
> Why are we even using write() instead of just mapping the fd and writing
> to that
> area?
>
> diff --git a/tools/testing/selftests/mm/khugepaged.c
> b/tools/testing/selftests/mm/khugepaged.c
> index 3fe7ef04ac62..a77f753ccf38 100644
> --- a/tools/testing/selftests/mm/khugepaged.c
> +++ b/tools/testing/selftests/mm/khugepaged.c
> @@ -381,9 +381,10 @@ static void *file_setup_area(int nr_hpages)
>         }
>
>         size = nr_hpages * hpage_pmd_size;
> -       p = alloc_mapping(nr_hpages);
> +       p = mmap(BASE_ADDR, size, PROT_READ | PROT_WRITE, MAP_SHARED, fd,
> 0);
> +       if (p != BASE_ADDR)
> +               ksft_exit_fail_msg("mmap() failed while preparing test
> file\n");
>         fill_memory(p, 0, size);
> -       write(fd, p, size);
>         close(fd);
>         munmap(p, size);
>         success("OK");
>
>
> --
> Cheers,
>
> David
>

[-- Attachment #2: Type: text/html, Size: 4543 bytes --]

[PATCH v2] selftests/mm: khugepaged: initialize file contents via mmap

[Vineet Agarwal]

file_setup_area() currently allocates anonymous memory, fills it,
and writes it into the backing file used for collapse testing.

Instead of copying data through write(), map the file directly with
MAP_SHARED after resizing it with ftruncate(), initialize the mapped
area in place, and sync it with msync().

This simplifies the setup path and avoids the need for explicit
partial write handling.

Signed-off-by: Vineet Agarwal <agarwal.vineet2006@gmail.com>
---
 tools/testing/selftests/mm/khugepaged.c | 25 +++++++++++++++++++++----
 1 file changed, 21 insertions(+), 4 deletions(-)

diff --git a/tools/testing/selftests/mm/khugepaged.c b/tools/testing/selftests/mm/khugepaged.c
index 3fe7ef04ac62..f6e171e1da4f 100644
--- a/tools/testing/selftests/mm/khugepaged.c
+++ b/tools/testing/selftests/mm/khugepaged.c
@@ -369,7 +369,6 @@ static void *file_setup_area(int nr_hpages)
 	int fd;
 	void *p;
 	unsigned long size;
-
 	unlink(finfo.path);  /* Cleanup from previous failed tests */
 	printf("Creating %s for collapse%s...", finfo.path,
 	       finfo.type == VMA_SHMEM ? " (tmpfs)" : "");
@@ -381,11 +380,29 @@ static void *file_setup_area(int nr_hpages)
 	}
 
 	size = nr_hpages * hpage_pmd_size;
-	p = alloc_mapping(nr_hpages);
+	if (ftruncate(fd, size)) {
+		perror("ftruncate()");
+		close(fd);
+		exit(EXIT_FAILURE);
+	}
+	p = mmap(BASE_ADDR, size, PROT_READ | PROT_WRITE,
+		MAP_SHARED, fd, 0);
+	if (p == MAP_FAILED || p != BASE_ADDR) {
+		perror("mmap()");
+		close(fd);
+		exit(EXIT_FAILURE);
+	}
 	fill_memory(p, 0, size);
-	write(fd, p, size);
-	close(fd);
+
+	if (msync(p, size, MS_SYNC)) {
+		perror("msync()");
+		munmap(p, size);
+		close(fd);
+		exit(EXIT_FAILURE);
+	}
+
 	munmap(p, size);
+	close(fd);
 	success("OK");
 
 	printf("Opening %s read only for collapse...", finfo.path);
-- 
2.54.0

Re: [PATCH v2] selftests/mm: khugepaged: initialize file contents via mmap

[David Hildenbrand (Arm)]

On 4/28/26 11:43, Vineet Agarwal wrote:
> file_setup_area() currently allocates anonymous memory, fills it,
> and writes it into the backing file used for collapse testing.
> 
> Instead of copying data through write(), map the file directly with
> MAP_SHARED after resizing it with ftruncate(), initialize the mapped
> area in place, and sync it with msync().
> 
> This simplifies the setup path and avoids the need for explicit
> partial write handling.
> 
> Signed-off-by: Vineet Agarwal <agarwal.vineet2006@gmail.com>
> ---
>  tools/testing/selftests/mm/khugepaged.c | 25 +++++++++++++++++++++----
>  1 file changed, 21 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/testing/selftests/mm/khugepaged.c b/tools/testing/selftests/mm/khugepaged.c
> index 3fe7ef04ac62..f6e171e1da4f 100644
> --- a/tools/testing/selftests/mm/khugepaged.c
> +++ b/tools/testing/selftests/mm/khugepaged.c
> @@ -369,7 +369,6 @@ static void *file_setup_area(int nr_hpages)
>  	int fd;
>  	void *p;
>  	unsigned long size;
> -
>  	unlink(finfo.path);  /* Cleanup from previous failed tests */
>  	printf("Creating %s for collapse%s...", finfo.path,
>  	       finfo.type == VMA_SHMEM ? " (tmpfs)" : "");
> @@ -381,11 +380,29 @@ static void *file_setup_area(int nr_hpages)
>  	}
>  
>  	size = nr_hpages * hpage_pmd_size;
> -	p = alloc_mapping(nr_hpages);
> +	if (ftruncate(fd, size)) {
> +		perror("ftruncate()");
> +		close(fd);
> +		exit(EXIT_FAILURE);
> +	}

Why is that required? We write all pages.

> +	p = mmap(BASE_ADDR, size, PROT_READ | PROT_WRITE,
> +		MAP_SHARED, fd, 0);
> +	if (p == MAP_FAILED || p != BASE_ADDR) {
> +		perror("mmap()");
> +		close(fd);
> +		exit(EXIT_FAILURE);
> +	}
>  	fill_memory(p, 0, size);
> -	write(fd, p, size);
> -	close(fd);
> +
> +	if (msync(p, size, MS_SYNC)) {
> +		perror("msync()");
> +		munmap(p, size);
> +		close(fd);
> +		exit(EXIT_FAILURE);

Why is that required?

-- 
Cheers,

David

Re: [PATCH v2] selftests/mm: khugepaged: initialize file contents via mmap

[Vineet Agarwal]

[-- Attachment #1: Type: text/plain, Size: 2823 bytes --]

ftruncate() is needed before mmap(), since the file is created with
O_TRUNC and initially has size 0. Without resizing it first, mapping
and accessing the full range can go beyond EOF.

You are right about msync() though — since all pages are written
through the shared mapping and the test later drops page cache
explicitly, an explicit msync() is unnecessary here.

I'll remove msync() and send a v3.

Thanks,
Vineet




On Tue, 28 Apr 2026 at 15:16, David Hildenbrand (Arm) <david@kernel.org>
wrote:

> On 4/28/26 11:43, Vineet Agarwal wrote:
> > file_setup_area() currently allocates anonymous memory, fills it,
> > and writes it into the backing file used for collapse testing.
> >
> > Instead of copying data through write(), map the file directly with
> > MAP_SHARED after resizing it with ftruncate(), initialize the mapped
> > area in place, and sync it with msync().
> >
> > This simplifies the setup path and avoids the need for explicit
> > partial write handling.
> >
> > Signed-off-by: Vineet Agarwal <agarwal.vineet2006@gmail.com>
> > ---
> >  tools/testing/selftests/mm/khugepaged.c | 25 +++++++++++++++++++++----
> >  1 file changed, 21 insertions(+), 4 deletions(-)
> >
> > diff --git a/tools/testing/selftests/mm/khugepaged.c
> b/tools/testing/selftests/mm/khugepaged.c
> > index 3fe7ef04ac62..f6e171e1da4f 100644
> > --- a/tools/testing/selftests/mm/khugepaged.c
> > +++ b/tools/testing/selftests/mm/khugepaged.c
> > @@ -369,7 +369,6 @@ static void *file_setup_area(int nr_hpages)
> >       int fd;
> >       void *p;
> >       unsigned long size;
> > -
> >       unlink(finfo.path);  /* Cleanup from previous failed tests */
> >       printf("Creating %s for collapse%s...", finfo.path,
> >              finfo.type == VMA_SHMEM ? " (tmpfs)" : "");
> > @@ -381,11 +380,29 @@ static void *file_setup_area(int nr_hpages)
> >       }
> >
> >       size = nr_hpages * hpage_pmd_size;
> > -     p = alloc_mapping(nr_hpages);
> > +     if (ftruncate(fd, size)) {
> > +             perror("ftruncate()");
> > +             close(fd);
> > +             exit(EXIT_FAILURE);
> > +     }
>
> Why is that required? We write all pages.
>
> > +     p = mmap(BASE_ADDR, size, PROT_READ | PROT_WRITE,
> > +             MAP_SHARED, fd, 0);
> > +     if (p == MAP_FAILED || p != BASE_ADDR) {
> > +             perror("mmap()");
> > +             close(fd);
> > +             exit(EXIT_FAILURE);
> > +     }
> >       fill_memory(p, 0, size);
> > -     write(fd, p, size);
> > -     close(fd);
> > +
> > +     if (msync(p, size, MS_SYNC)) {
> > +             perror("msync()");
> > +             munmap(p, size);
> > +             close(fd);
> > +             exit(EXIT_FAILURE);
>
> Why is that required?
>
> --
> Cheers,
>
> David
>

[-- Attachment #2: Type: text/html, Size: 3941 bytes --]

[PATCH v3] selftests/mm: khugepaged: initialize file contents via mmap

[Vineet Agarwal]

file_setup_area() currently allocates anonymous memory, fills it,
and writes it into the backing file used for collapse testing.

Instead of copying data through write(), resize the file with
ftruncate(), map it directly with MAP_SHARED, and initialize the
mapped area in place.

This simplifies the setup path and avoids the need for explicit
partial write handling.

Signed-off-by: Vineet Agarwal <agarwal.vineet2006@gmail.com>
---
 tools/testing/selftests/mm/khugepaged.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/tools/testing/selftests/mm/khugepaged.c b/tools/testing/selftests/mm/khugepaged.c
index 3fe7ef04ac62..57ca4224bd5a 100644
--- a/tools/testing/selftests/mm/khugepaged.c
+++ b/tools/testing/selftests/mm/khugepaged.c
@@ -369,7 +369,6 @@ static void *file_setup_area(int nr_hpages)
 	int fd;
 	void *p;
 	unsigned long size;
-
 	unlink(finfo.path);  /* Cleanup from previous failed tests */
 	printf("Creating %s for collapse%s...", finfo.path,
 	       finfo.type == VMA_SHMEM ? " (tmpfs)" : "");
@@ -381,11 +380,21 @@ static void *file_setup_area(int nr_hpages)
 	}
 
 	size = nr_hpages * hpage_pmd_size;
-	p = alloc_mapping(nr_hpages);
+	if (ftruncate(fd, size)) {
+		perror("ftruncate()");
+		close(fd);
+		exit(EXIT_FAILURE);
+	}
+	p = mmap(BASE_ADDR, size, PROT_READ | PROT_WRITE,
+		MAP_SHARED, fd, 0);
+	if (p == MAP_FAILED || p != BASE_ADDR) {
+		perror("mmap()");
+		close(fd);
+		exit(EXIT_FAILURE);
+	}
 	fill_memory(p, 0, size);
-	write(fd, p, size);
-	close(fd);
 	munmap(p, size);
+	close(fd);
 	success("OK");
 
 	printf("Opening %s read only for collapse...", finfo.path);
-- 
2.54.0

Re: [PATCH v2] selftests/mm: khugepaged: initialize file contents via mmap

[David Hildenbrand (Arm)]

On 4/28/26 11:52, Vineet Agarwal wrote:
> ftruncate() is needed before mmap(), since the file is created with
> O_TRUNC and initially has size 0. Without resizing it first, mapping
> and accessing the full range can go beyond EOF.

Ah, that makes sense, worth spelling it out. Indeed, we otherwise get

$ sudo ./khugepaged madvise:file .
Save THP and khugepaged settings... OK
Allocate huge page on fault... OK
Split huge PMD on MADV_DONTNEED... OK

Run test: collapse_full (madvise:file)
Creating ./collapse_test_file for collapse...Bus error

-- 
Cheers,

David

Re: [PATCH v3] selftests/mm: khugepaged: initialize file contents via mmap

[David Hildenbrand (Arm)]

On 4/28/26 12:00, Vineet Agarwal wrote:
> file_setup_area() currently allocates anonymous memory, fills it,
> and writes it into the backing file used for collapse testing.
> 
> Instead of copying data through write(), resize the file with
> ftruncate(), map it directly with MAP_SHARED, and initialize the
> mapped area in place.
> 
> This simplifies the setup path and avoids the need for explicit
> partial write handling.


Please

a) Wait a bit longer before you resend.

b) Don't send as reply to earlier revisions.

> 
> Signed-off-by: Vineet Agarwal <agarwal.vineet2006@gmail.com>
> ---
>  tools/testing/selftests/mm/khugepaged.c | 17 +++++++++++++----
>  1 file changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/testing/selftests/mm/khugepaged.c b/tools/testing/selftests/mm/khugepaged.c
> index 3fe7ef04ac62..57ca4224bd5a 100644
> --- a/tools/testing/selftests/mm/khugepaged.c
> +++ b/tools/testing/selftests/mm/khugepaged.c
> @@ -369,7 +369,6 @@ static void *file_setup_area(int nr_hpages)
>  	int fd;
>  	void *p;
>  	unsigned long size;
> -

Unrelated change.

>  	unlink(finfo.path);  /* Cleanup from previous failed tests */
>  	printf("Creating %s for collapse%s...", finfo.path,
>  	       finfo.type == VMA_SHMEM ? " (tmpfs)" : "");
> @@ -381,11 +380,21 @@ static void *file_setup_area(int nr_hpages)
>  	}
>  
>  	size = nr_hpages * hpage_pmd_size;
> -	p = alloc_mapping(nr_hpages);
> +	if (ftruncate(fd, size)) {
> +		perror("ftruncate()");
> +		close(fd);
> +		exit(EXIT_FAILURE);
> +	}
> +	p = mmap(BASE_ADDR, size, PROT_READ | PROT_WRITE,
> +		MAP_SHARED, fd, 0);
> +	if (p == MAP_FAILED || p != BASE_ADDR) {
> +		perror("mmap()");
> +		close(fd);
> +		exit(EXIT_FAILURE);
> +	}
>  	fill_memory(p, 0, size);
> -	write(fd, p, size);
> -	close(fd);
>  	munmap(p, size);
> +	close(fd);

That change is not strictly required. We can munmap() after close().

Apart from that LGTM.

-- 
Cheers,

David