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; }
On Mon, 30 Jun 2025 23:38:03 +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
Hi Suresh,
Thank you for writing this patch! I'll let Nhat or Johannes comment more on the patch, but just had a few thoughts. Before going into the code, I wanted to note that it would be helpful in the future to note where this patch comes from. I saw there were a few iterations of this before, so it would help reviewers track what changed between the versions and what the motivation for new versions are.
[...snip...]
ksft_print_msg("Unable to create shmem file.\n");
ksft_print_msg("Unable to create file.\n");
Maybe we don't want to lose information about this -- it would be helpful to see why the test failed. It doesn't seem like there are any other indicators that would let users know if it was shmem or mmap that failed, so users would basically be guessing as to which of these two test failed. (And the same feedback aplies to the next two print statements)
} if (ftruncate(fd, filesize)) {
ksft_print_msg("Unable to truncate shmem file.\n");
ksft_print_msg("Unable to truncate file.\n");if (!write_exactly(fd, filesize)) {
ksft_print_msg("Unable to write to shmem file.\n");
ksft_print_msg("Unable to write to file.\n");
I'm curious if we need this part down below. It seems like we already call write_exactly above, which should fill the file descriptor with random things up to filesize. Maybe it makes more sense to have these two options (write_exactly vs. directly modifying the contents) in a switch statement? It seems a bit redundant to do both for FILE_MMAP.
- 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'm also curious what the point of having the last character be different here. It doesn't seem like there is any validation code to check the contents of the file, so it seems a bit redundant to me as well.
Have a great day! Joshua
Sent using hkml (https://github.com/sjp38/hackermail)
On Mon, Jun 30, 2025 at 11:08 AM 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
Signed-off-by: Suresh K C suresh.k.chandrappa@gmail.com
FWIW, the tests passed :)
.../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);elsefd = 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';}
Joshua is right. We're already doing the write_exactly() for both cases? Let's write_exactly() (i.e using the file descriptor-based write syscall) only for FILE_SHMEM.
And why do we need the 'X' at the end?
linux-kselftest-mirror@lists.linaro.org
-
Joshua Hahn -
Nhat Pham -
Suresh K C