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

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

Posted by Vineet Agarwal 1 month, 2 weeks ago

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()

Posted by David Hildenbrand (Arm) 1 month, 2 weeks ago

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

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

Posted by Vineet Agarwal 1 month, 2 weeks ago

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 v3] selftests/mm: khugepaged: initialize file contents via mmap

Posted by David Hildenbrand (Arm) 1 month, 2 weeks ago

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

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

Posted by Vineet Agarwal 1 month, 2 weeks ago

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

Posted by David Hildenbrand (Arm) 1 month, 2 weeks ago

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