linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 1/2] selftests: cachestat: add tests for mmap
@ 2025-07-01 18:49 Suresh K C
  0 siblings, 0 replies; 5+ messages in thread
From: Suresh K C @ 2025-07-01 18:49 UTC (permalink / raw)
  To: nphamcs, hannes, joshua.hahnjy, shuah
  Cc: linux-mm, linux-kselftest, linux-kernel, 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


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH v2 1/2] selftests: cachestat: add tests for mmap
@ 2025-07-07 15:25 Suresh K C
  2025-07-07 15:25 ` [PATCH v2 2/2] selftest: improve mmap test clarity Suresh K C
  2025-07-07 21:26 ` [PATCH v2 1/2] selftests: cachestat: add tests for mmap Joshua Hahn
  0 siblings, 2 replies; 5+ messages in thread
From: Suresh K C @ 2025-07-07 15:25 UTC (permalink / raw)
  To: nphamcs, hannes, joshua.hahnjy, shuah
  Cc: linux-mm, linux-kselftest, linux-kernel, 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


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH v2 2/2] selftest: improve mmap test clarity
  2025-07-07 15:25 [PATCH v2 1/2] selftests: cachestat: add tests for mmap Suresh K C
@ 2025-07-07 15:25 ` Suresh K C
  2025-07-07 21:26 ` [PATCH v2 1/2] selftests: cachestat: add tests for mmap Joshua Hahn
  1 sibling, 0 replies; 5+ messages in thread
From: Suresh K C @ 2025-07-07 15:25 UTC (permalink / raw)
  To: nphamcs, hannes, joshua.hahnjy, shuah
  Cc: linux-mm, linux-kselftest, linux-kernel, 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


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v2 1/2] selftests: cachestat: add tests for mmap
  2025-07-07 15:25 [PATCH v2 1/2] selftests: cachestat: add tests for mmap Suresh K C
  2025-07-07 15:25 ` [PATCH v2 2/2] selftest: improve mmap test clarity Suresh K C
@ 2025-07-07 21:26 ` Joshua Hahn
  1 sibling, 0 replies; 5+ messages in thread
From: Joshua Hahn @ 2025-07-07 21:26 UTC (permalink / raw)
  To: Suresh K C
  Cc: nphamcs, hannes, joshua.hahnjy, shuah, linux-mm, linux-kselftest,
	linux-kernel

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)

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2 1/2] selftests: cachestat: add tests for mmap
       [not found] <CAAMO5phQQ8wMdOfuW40No4kpK5Rn2o4_414F8cgUrQ5NBsCcng@mail.gmail.com>
@ 2025-07-09 17:03 ` Joshua Hahn
  0 siblings, 0 replies; 5+ messages in thread
From: Joshua Hahn @ 2025-07-09 17:03 UTC (permalink / raw)
  To: Suresh Chandrappa
  Cc: nphamcs, hannes, shuah, linux-mm, linux-kselftest, linux-kernel

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)

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2025-07-09 17:04 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-07 15:25 [PATCH v2 1/2] selftests: cachestat: add tests for mmap Suresh K C
2025-07-07 15:25 ` [PATCH v2 2/2] selftest: improve mmap test clarity Suresh K C
2025-07-07 21:26 ` [PATCH v2 1/2] selftests: cachestat: add tests for mmap Joshua Hahn
     [not found] <CAAMO5phQQ8wMdOfuW40No4kpK5Rn2o4_414F8cgUrQ5NBsCcng@mail.gmail.com>
2025-07-09 17:03 ` Joshua Hahn
  -- strict thread matches above, loose matches on Subject: below --
2025-07-01 18:49 Suresh K C

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).