[PATCH v2 1/2] selftests: cachestat: add tests for mmap

[PATCH v2 1/2] selftests: cachestat: add tests for mmap

[Suresh K C]

From: Suresh K C <suresh.k.chandrappa@gmail.com>

Add a test case to verify cachestat behavior with memory-mapped files
using mmap(). This ensures that pages accessed via mmap are correctly
accounted for in the page cache.

Tested on x86_64 with default kernel config

Signed-off-by: Suresh K C <suresh.k.chandrappa@gmail.com>
---
 .../selftests/cachestat/test_cachestat.c      | 39 ++++++++++++++++---
 1 file changed, 33 insertions(+), 6 deletions(-)

diff --git a/tools/testing/selftests/cachestat/test_cachestat.c b/tools/testing/selftests/cachestat/test_cachestat.c
index 632ab44737ec..b6452978dae0 100644
--- a/tools/testing/selftests/cachestat/test_cachestat.c
+++ b/tools/testing/selftests/cachestat/test_cachestat.c
@@ -33,6 +33,11 @@ void print_cachestat(struct cachestat *cs)
 	cs->nr_evicted, cs->nr_recently_evicted);
 }
 
+enum file_type {
+	FILE_MMAP,
+	FILE_SHMEM
+};
+
 bool write_exactly(int fd, size_t filesize)
 {
 	int random_fd = open("/dev/urandom", O_RDONLY);
@@ -202,7 +207,7 @@ static int test_cachestat(const char *filename, bool write_random, bool create,
 	return ret;
 }
 
-bool test_cachestat_shmem(void)
+bool run_cachestat_test(enum file_type type)
 {
 	size_t PS = sysconf(_SC_PAGESIZE);
 	size_t filesize = PS * 512 * 2; /* 2 2MB huge pages */
@@ -212,27 +217,43 @@ bool test_cachestat_shmem(void)
 	char *filename = "tmpshmcstat";
 	struct cachestat cs;
 	bool ret = true;
+	int fd;
 	unsigned long num_pages = compute_len / PS;
-	int fd = shm_open(filename, O_CREAT | O_RDWR, 0600);
+	if (type == FILE_SHMEM)
+		fd = shm_open(filename, O_CREAT | O_RDWR, 0600);
+	else
+		fd = open(filename, O_RDWR | O_CREAT | O_TRUNC, 0666);
 
 	if (fd < 0) {
-		ksft_print_msg("Unable to create shmem file.\n");
+		ksft_print_msg("Unable to create file.\n");
 		ret = false;
 		goto out;
 	}
 
 	if (ftruncate(fd, filesize)) {
-		ksft_print_msg("Unable to truncate shmem file.\n");
+		ksft_print_msg("Unable to truncate file.\n");
 		ret = false;
 		goto close_fd;
 	}
 
 	if (!write_exactly(fd, filesize)) {
-		ksft_print_msg("Unable to write to shmem file.\n");
+		ksft_print_msg("Unable to write to file.\n");
 		ret = false;
 		goto close_fd;
 	}
 
+	if (type == FILE_MMAP){
+		char *map = mmap(NULL, filesize, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0);
+		if (map == MAP_FAILED) {
+			ksft_print_msg("mmap failed.\n");
+			ret = false;
+			goto close_fd;
+		}
+		for (int i = 0; i < filesize; i++) {
+			map[i] = 'A';
+		}
+		map[filesize - 1] = 'X';
+	}
 	syscall_ret = syscall(__NR_cachestat, fd, &cs_range, &cs, 0);
 
 	if (syscall_ret) {
@@ -308,12 +329,18 @@ int main(void)
 		break;
 	}
 
-	if (test_cachestat_shmem())
+	if (run_cachestat_test(FILE_SHMEM))
 		ksft_test_result_pass("cachestat works with a shmem file\n");
 	else {
 		ksft_test_result_fail("cachestat fails with a shmem file\n");
 		ret = 1;
 	}
 
+	if (run_cachestat_test(FILE_MMAP))
+		ksft_test_result_pass("cachestat works with a mmap file\n");
+	else {
+		ksft_test_result_fail("cachestat fails with a mmap file\n");
+		ret = 1;
+	}
 	return ret;
 }
-- 
2.43.0

[PATCH v2 1/2] selftests: cachestat: add tests for mmap

[Suresh K C]

From: Suresh K C <suresh.k.chandrappa@gmail.com>

Add a test case to verify cachestat behavior with memory-mapped files
using mmap(). This ensures that pages accessed via mmap are correctly
accounted for in the page cache.

Tested on x86_64 with default kernel config

Signed-off-by: Suresh K C <suresh.k.chandrappa@gmail.com>
---
 .../selftests/cachestat/test_cachestat.c      | 39 ++++++++++++++++---
 1 file changed, 33 insertions(+), 6 deletions(-)

diff --git a/tools/testing/selftests/cachestat/test_cachestat.c b/tools/testing/selftests/cachestat/test_cachestat.c
index 632ab44737ec..b6452978dae0 100644
--- a/tools/testing/selftests/cachestat/test_cachestat.c
+++ b/tools/testing/selftests/cachestat/test_cachestat.c
@@ -33,6 +33,11 @@ void print_cachestat(struct cachestat *cs)
 	cs->nr_evicted, cs->nr_recently_evicted);
 }
 
+enum file_type {
+	FILE_MMAP,
+	FILE_SHMEM
+};
+
 bool write_exactly(int fd, size_t filesize)
 {
 	int random_fd = open("/dev/urandom", O_RDONLY);
@@ -202,7 +207,7 @@ static int test_cachestat(const char *filename, bool write_random, bool create,
 	return ret;
 }
 
-bool test_cachestat_shmem(void)
+bool run_cachestat_test(enum file_type type)
 {
 	size_t PS = sysconf(_SC_PAGESIZE);
 	size_t filesize = PS * 512 * 2; /* 2 2MB huge pages */
@@ -212,27 +217,43 @@ bool test_cachestat_shmem(void)
 	char *filename = "tmpshmcstat";
 	struct cachestat cs;
 	bool ret = true;
+	int fd;
 	unsigned long num_pages = compute_len / PS;
-	int fd = shm_open(filename, O_CREAT | O_RDWR, 0600);
+	if (type == FILE_SHMEM)
+		fd = shm_open(filename, O_CREAT | O_RDWR, 0600);
+	else
+		fd = open(filename, O_RDWR | O_CREAT | O_TRUNC, 0666);
 
 	if (fd < 0) {
-		ksft_print_msg("Unable to create shmem file.\n");
+		ksft_print_msg("Unable to create file.\n");
 		ret = false;
 		goto out;
 	}
 
 	if (ftruncate(fd, filesize)) {
-		ksft_print_msg("Unable to truncate shmem file.\n");
+		ksft_print_msg("Unable to truncate file.\n");
 		ret = false;
 		goto close_fd;
 	}
 
 	if (!write_exactly(fd, filesize)) {
-		ksft_print_msg("Unable to write to shmem file.\n");
+		ksft_print_msg("Unable to write to file.\n");
 		ret = false;
 		goto close_fd;
 	}
 
+	if (type == FILE_MMAP){
+		char *map = mmap(NULL, filesize, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0);
+		if (map == MAP_FAILED) {
+			ksft_print_msg("mmap failed.\n");
+			ret = false;
+			goto close_fd;
+		}
+		for (int i = 0; i < filesize; i++) {
+			map[i] = 'A';
+		}
+		map[filesize - 1] = 'X';
+	}
 	syscall_ret = syscall(__NR_cachestat, fd, &cs_range, &cs, 0);
 
 	if (syscall_ret) {
@@ -308,12 +329,18 @@ int main(void)
 		break;
 	}
 
-	if (test_cachestat_shmem())
+	if (run_cachestat_test(FILE_SHMEM))
 		ksft_test_result_pass("cachestat works with a shmem file\n");
 	else {
 		ksft_test_result_fail("cachestat fails with a shmem file\n");
 		ret = 1;
 	}
 
+	if (run_cachestat_test(FILE_MMAP))
+		ksft_test_result_pass("cachestat works with a mmap file\n");
+	else {
+		ksft_test_result_fail("cachestat fails with a mmap file\n");
+		ret = 1;
+	}
 	return ret;
 }
-- 
2.43.0

[PATCH v2 2/2] selftest: improve mmap test clarity

[Suresh K C]

From: Suresh K C <suresh.k.chandrappa@gmail.com>

This patch refactors the mmap test logic to remove redundancy and improve
error reporting. It also removes leftover test code that is no longer needed.

Changes since v1:
- Refactored mmap logic into a switch statement as suggested
- Removed the last-character difference, which was only used for testing
- Added clearer error messages to indicate whether shmem or mmap failed
- Combined patches into a series for better context
- Addressed feedback on patch origin and versioning

Signed-off-by: Suresh K C <suresh.k.chandrappa@gmail.com>
---
 .../selftests/cachestat/test_cachestat.c      | 52 +++++++++++++------
 1 file changed, 35 insertions(+), 17 deletions(-)

diff --git a/tools/testing/selftests/cachestat/test_cachestat.c b/tools/testing/selftests/cachestat/test_cachestat.c
index b6452978dae0..0549b7224ba1 100644
--- a/tools/testing/selftests/cachestat/test_cachestat.c
+++ b/tools/testing/selftests/cachestat/test_cachestat.c
@@ -206,6 +206,17 @@ static int test_cachestat(const char *filename, bool write_random, bool create,
 out:
 	return ret;
 }
+const char* file_type_str(enum file_type type) {
+	switch (type) {
+		case FILE_SHMEM:
+			return "shmem";
+		case FILE_MMAP:
+			return "mmap";
+		default:
+			return "unknown";
+	}
+}
+
 
 bool run_cachestat_test(enum file_type type)
 {
@@ -225,34 +236,41 @@ bool run_cachestat_test(enum file_type type)
 		fd = open(filename, O_RDWR | O_CREAT | O_TRUNC, 0666);
 
 	if (fd < 0) {
-		ksft_print_msg("Unable to create file.\n");
+		ksft_print_msg("Unable to create %s file.\n",file_type_str(type));
 		ret = false;
 		goto out;
 	}
 
 	if (ftruncate(fd, filesize)) {
-		ksft_print_msg("Unable to truncate file.\n");
-		ret = false;
-		goto close_fd;
-	}
-
-	if (!write_exactly(fd, filesize)) {
-		ksft_print_msg("Unable to write to file.\n");
+		ksft_print_msg("Unable to truncate %s file.\n",file_type_str(type));
 		ret = false;
 		goto close_fd;
 	}
 
-	if (type == FILE_MMAP){
-		char *map = mmap(NULL, filesize, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0);
-		if (map == MAP_FAILED) {
-			ksft_print_msg("mmap failed.\n");
+	switch (type){
+		case FILE_SHMEM:
+			if (!write_exactly(fd, filesize)) {
+				ksft_print_msg("Unable to write to file.\n");
+				ret = false;
+				goto close_fd;
+			}
+			break;
+		case FILE_MMAP:
+			char *map = mmap(NULL, filesize, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0);
+			if (map == MAP_FAILED) {
+				ksft_print_msg("mmap failed.\n");
+				ret = false;
+				goto close_fd;
+			}
+			for (int i = 0; i < filesize; i++) {
+				map[i] = 'A';
+			}
+			break;
+		default:
+			ksft_print_msg("Unsupported file type.\n");
 			ret = false;
 			goto close_fd;
-		}
-		for (int i = 0; i < filesize; i++) {
-			map[i] = 'A';
-		}
-		map[filesize - 1] = 'X';
+			break;
 	}
 	syscall_ret = syscall(__NR_cachestat, fd, &cs_range, &cs, 0);
 
-- 
2.43.0

Re: [PATCH v2 1/2] selftests: cachestat: add tests for mmap

[Joshua Hahn]

On Mon,  7 Jul 2025 20:55:56 +0530 Suresh K C <suresh.k.chandrappa@gmail.com> wrote:

> From: Suresh K C <suresh.k.chandrappa@gmail.com>
> 
> Add a test case to verify cachestat behavior with memory-mapped files
> using mmap(). This ensures that pages accessed via mmap are correctly
> accounted for in the page cache.
> 
> Tested on x86_64 with default kernel config

Hey Suresh,

Thanks for the second version with the updates, sorry that I missed the
first time you sent this patch.

[...snip...]

>  	if (fd < 0) {
> -		ksft_print_msg("Unable to create shmem file.\n");
> +		ksft_print_msg("Unable to create file.\n");

NIT: I saw that you change this in the second part of the patch. However, why
not just include it in this patch? I feel that it would be good practice
to keep the kerenl in a "correct" state, even in between patches belonging
to the same series. If someone were to just apply this patch but not the
next (however unlikely that is), then they will not see the description of
what file type they failed to create. Just my 2c, no need to change this if
you don't think this is important.

[...snip...]

> +	if (type == FILE_MMAP){
> +		char *map = mmap(NULL, filesize, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0);
> +		if (map == MAP_FAILED) {
> +			ksft_print_msg("mmap failed.\n");
> +			ret = false;
> +			goto close_fd;
> +		}
> +		for (int i = 0; i < filesize; i++) {
> +			map[i] = 'A';
> +		}
> +		map[filesize - 1] = 'X';

NIT: Likewise, I don't know if there is a good reason to include this, only to
remove it in the second patch. Perhaps it would be best to just remove it
in this patch, so you don't have to delete it later?

Please let me know what you think. Have a great day!
Joshua

Sent using hkml (https://github.com/sjp38/hackermail)

Re: [PATCH v2 1/2] selftests: cachestat: add tests for mmap

[Suresh Chandrappa]

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

Hi Joshua,

Thanks for the feedback! In the first patch, both shmem and mmap operations
are present, but I hadn’t introduced any logic to distinguish between them
yet. That distinction is added in the second patch through a new API.

> +	if (type == FILE_MMAP){
> +		char *map = mmap(NULL, filesize, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0);
> +		if (map == MAP_FAILED) {
> +			ksft_print_msg("mmap failed.\n");
> +			ret = false;
> +			goto close_fd;
> +		}
> +		for (int i = 0; i < filesize; i++) {
> +			map[i] = 'A';
> +		}
> +		map[filesize - 1] = 'X';

I initially included the first version of the patch mainly to gather early
feedback and ensure all review comments were addressed. Based on the input,
I plan to consolidate the changes into a single patch that reflects the
final version.

Thanks,

Suresh K C

On Tue, Jul 8, 2025 at 2:56 AM Joshua Hahn <joshua.hahnjy@gmail.com> wrote:

> On Mon,  7 Jul 2025 20:55:56 +0530 Suresh K C <
> suresh.k.chandrappa@gmail.com> wrote:
>
> > From: Suresh K C <suresh.k.chandrappa@gmail.com>
> >
> > Add a test case to verify cachestat behavior with memory-mapped files
> > using mmap(). This ensures that pages accessed via mmap are correctly
> > accounted for in the page cache.
> >
> > Tested on x86_64 with default kernel config
>
> Hey Suresh,
>
> Thanks for the second version with the updates, sorry that I missed the
> first time you sent this patch.
>
> [...snip...]
>
> >       if (fd < 0) {
> > -             ksft_print_msg("Unable to create shmem file.\n");
> > +             ksft_print_msg("Unable to create file.\n");
>
> NIT: I saw that you change this in the second part of the patch. However,
> why
> not just include it in this patch? I feel that it would be good practice
> to keep the kerenl in a "correct" state, even in between patches belonging
> to the same series. If someone were to just apply this patch but not the
> next (however unlikely that is), then they will not see the description of
> what file type they failed to create. Just my 2c, no need to change this if
> you don't think this is important.
>
> [...snip...]
>
> > +     if (type == FILE_MMAP){
> > +             char *map = mmap(NULL, filesize, PROT_READ | PROT_WRITE,
> MAP_SHARED, fd, 0);
> > +             if (map == MAP_FAILED) {
> > +                     ksft_print_msg("mmap failed.\n");
> > +                     ret = false;
> > +                     goto close_fd;
> > +             }
> > +             for (int i = 0; i < filesize; i++) {
> > +                     map[i] = 'A';
> > +             }
> > +             map[filesize - 1] = 'X';
>
> NIT: Likewise, I don't know if there is a good reason to include this,
> only to
> remove it in the second patch. Perhaps it would be best to just remove it
> in this patch, so you don't have to delete it later?
>
> Please let me know what you think. Have a great day!
> Joshua
>
> Sent using hkml (https://github.com/sjp38/hackermail)
>

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

Re: [PATCH v2 1/2] selftests: cachestat: add tests for mmap

[Joshua Hahn]

On Tue, 8 Jul 2025 23:13:01 +0530 Suresh Chandrappa <suresh.k.chandrappa@gmail.com> wrote:

> Hi Joshua,
> 
> Thanks for the feedback! In the first patch, both shmem and mmap operations
> are present, but I hadn’t introduced any logic to distinguish between them
> yet. That distinction is added in the second patch through a new API.

Hi Suresh,

Yes, this makes sense to me. I think what I was getting at was that we could
still make a conditional statement like

if (type == FILE_SHMEM)
	ksft_print_msg("Unable to create shmem file.\n")'
else if (type == FILE_MMAP)
	ksft_print_msg("Unable to create mmap file.\n");

(or use a switch statement)

...

And just refactor it in patch 2, as opposed to changing the behavior.
But this is mostly a nit. If you are planning to merge both patches in one
patch in the next version, then all of these comments shouldn't matter : -)

Looking forward to the next version, have a great day!
Joshua

Sent using hkml (https://github.com/sjp38/hackermail)

Re: [PATCH v2 1/2] selftests: cachestat: add tests for mmap

[Suresh Chandrappa]

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

Hi Joshua

Thanks for the feedback! I’ll go ahead and create a new patch that merges
the two, as suggested. Appreciate your input, looking forward to sharing
the next version soon.

Thanks
Suresh K C

On Wed, Jul 9, 2025 at 10:34 PM Joshua Hahn <joshua.hahnjy@gmail.com> wrote:

> On Tue, 8 Jul 2025 23:13:01 +0530 Suresh Chandrappa <
> suresh.k.chandrappa@gmail.com> wrote:
>
> > Hi Joshua,
> >
> > Thanks for the feedback! In the first patch, both shmem and mmap
> operations
> > are present, but I hadn’t introduced any logic to distinguish between
> them
> > yet. That distinction is added in the second patch through a new API.
>
> Hi Suresh,
>
> Yes, this makes sense to me. I think what I was getting at was that we
> could
> still make a conditional statement like
>
> if (type == FILE_SHMEM)
>         ksft_print_msg("Unable to create shmem file.\n")'
> else if (type == FILE_MMAP)
>         ksft_print_msg("Unable to create mmap file.\n");
>
> (or use a switch statement)
>
> ...
>
> And just refactor it in patch 2, as opposed to changing the behavior.
> But this is mostly a nit. If you are planning to merge both patches in one
> patch in the next version, then all of these comments shouldn't matter : -)
>
> Looking forward to the next version, have a great day!
> Joshua
>
> Sent using hkml (https://github.com/sjp38/hackermail)
>

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