* Re: [PATCH] selftests/mm: khugepaged: handle partial write in file_setup_area()
2026-04-28 8:04 ` David Hildenbrand (Arm)
@ 2026-04-28 9:15 ` Vineet Agarwal
2026-04-28 9:43 ` [PATCH v2] selftests/mm: khugepaged: initialize file contents via mmap Vineet Agarwal
2026-04-28 10:00 ` [PATCH v3] " Vineet Agarwal
2 siblings, 0 replies; 9+ messages in thread
From: Vineet Agarwal @ 2026-04-28 9:15 UTC (permalink / raw)
To: David Hildenbrand (Arm)
Cc: akpm, ljs, shuah, linux-mm, linux-kselftest, linux-kernel
[-- 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 --]
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH v2] selftests/mm: khugepaged: initialize file contents via mmap
2026-04-28 8:04 ` David Hildenbrand (Arm)
2026-04-28 9:15 ` Vineet Agarwal
@ 2026-04-28 9:43 ` Vineet Agarwal
2026-04-28 9:46 ` David Hildenbrand (Arm)
2026-04-28 10:00 ` [PATCH v3] " Vineet Agarwal
2 siblings, 1 reply; 9+ messages in thread
From: Vineet Agarwal @ 2026-04-28 9:43 UTC (permalink / raw)
To: akpm, david, ljs, shuah
Cc: linux-mm, linux-kselftest, linux-kernel, 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
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH v2] selftests/mm: khugepaged: initialize file contents via mmap
2026-04-28 9:43 ` [PATCH v2] selftests/mm: khugepaged: initialize file contents via mmap Vineet Agarwal
@ 2026-04-28 9:46 ` David Hildenbrand (Arm)
2026-04-28 9:52 ` Vineet Agarwal
0 siblings, 1 reply; 9+ messages in thread
From: David Hildenbrand (Arm) @ 2026-04-28 9:46 UTC (permalink / raw)
To: Vineet Agarwal, akpm, ljs, shuah; +Cc: linux-mm, linux-kselftest, linux-kernel
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
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH v2] selftests/mm: khugepaged: initialize file contents via mmap
2026-04-28 9:46 ` David Hildenbrand (Arm)
@ 2026-04-28 9:52 ` Vineet Agarwal
2026-04-28 10:06 ` David Hildenbrand (Arm)
0 siblings, 1 reply; 9+ messages in thread
From: Vineet Agarwal @ 2026-04-28 9:52 UTC (permalink / raw)
To: David Hildenbrand (Arm)
Cc: akpm, ljs, shuah, linux-mm, linux-kselftest, linux-kernel
[-- 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 --]
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH v2] selftests/mm: khugepaged: initialize file contents via mmap
2026-04-28 9:52 ` Vineet Agarwal
@ 2026-04-28 10:06 ` David Hildenbrand (Arm)
0 siblings, 0 replies; 9+ messages in thread
From: David Hildenbrand (Arm) @ 2026-04-28 10:06 UTC (permalink / raw)
To: Vineet Agarwal; +Cc: akpm, ljs, shuah, linux-mm, linux-kselftest, linux-kernel
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v3] selftests/mm: khugepaged: initialize file contents via mmap
2026-04-28 8:04 ` David Hildenbrand (Arm)
2026-04-28 9:15 ` Vineet Agarwal
2026-04-28 9:43 ` [PATCH v2] selftests/mm: khugepaged: initialize file contents via mmap Vineet Agarwal
@ 2026-04-28 10:00 ` Vineet Agarwal
2026-04-28 10:07 ` David Hildenbrand (Arm)
2 siblings, 1 reply; 9+ messages in thread
From: Vineet Agarwal @ 2026-04-28 10:00 UTC (permalink / raw)
To: akpm, david, ljs, shuah
Cc: linux-mm, linux-kselftest, linux-kernel, 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
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH v3] selftests/mm: khugepaged: initialize file contents via mmap
2026-04-28 10:00 ` [PATCH v3] " Vineet Agarwal
@ 2026-04-28 10:07 ` David Hildenbrand (Arm)
0 siblings, 0 replies; 9+ messages in thread
From: David Hildenbrand (Arm) @ 2026-04-28 10:07 UTC (permalink / raw)
To: Vineet Agarwal, akpm, ljs, shuah; +Cc: linux-mm, linux-kselftest, linux-kernel
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
^ permalink raw reply [flat|nested] 9+ messages in thread