* [PATCH v2] selftests: cachestat: Refactor test to remove duplication
@ 2025-06-11 18:09 Suresh K C
2025-06-25 4:58 ` Suresh Chandrappa
0 siblings, 1 reply; 5+ messages in thread
From: Suresh K C @ 2025-06-11 18:09 UTC (permalink / raw)
To: nphamcs, hannes, shuah
Cc: linux-mm, linux-kselftest, linux-kernel, Suresh K C
From: Suresh K C <suresh.k.chandrappa@gmail.com>
Refactored the mmap and shmem test logic into a common function
to reduce code duplication and improve maintainability
Changes in v2:
Refactored mmap and shmem tests into a common function
Renamed test function to run_cachestat_test()
Removed test for /proc/cpuinfo as a general /proc test case already exists
Signed-off-by: Suresh K C <suresh.k.chandrappa@gmail.com>
---
.../selftests/cachestat/test_cachestat.c | 97 ++++++-------------
1 file changed, 30 insertions(+), 67 deletions(-)
diff --git a/tools/testing/selftests/cachestat/test_cachestat.c b/tools/testing/selftests/cachestat/test_cachestat.c
index 81e7f6dd2279..7c2f64175943 100644
--- a/tools/testing/selftests/cachestat/test_cachestat.c
+++ b/tools/testing/selftests/cachestat/test_cachestat.c
@@ -22,7 +22,7 @@
static const char * const dev_files[] = {
"/dev/zero", "/dev/null", "/dev/urandom",
- "/proc/version","/proc/cpuinfo","/proc"
+ "/proc/version","/proc"
};
void print_cachestat(struct cachestat *cs)
@@ -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,66 +207,8 @@ static int test_cachestat(const char *filename, bool write_random, bool create,
return ret;
}
-bool test_cachestat_mmap(void){
-
- size_t PS = sysconf(_SC_PAGESIZE);
- size_t filesize = PS * 512 * 2;;
- int syscall_ret;
- size_t compute_len = PS * 512;
- struct cachestat_range cs_range = { PS, compute_len };
- char *filename = "tmpshmcstat";
- unsigned long num_pages = compute_len / PS;
- struct cachestat cs;
- bool ret = true;
- int fd = open(filename, O_RDWR | O_CREAT | O_TRUNC, 0666);
- if (fd < 0) {
- ksft_print_msg("Unable to create mmap file.\n");
- ret = false;
- goto out;
- }
- if (ftruncate(fd, filesize)) {
- ksft_print_msg("Unable to truncate mmap file.\n");
- ret = false;
- goto close_fd;
- }
- if (!write_exactly(fd, filesize)) {
- ksft_print_msg("Unable to write to mmap file.\n");
- ret = false;
- goto close_fd;
- }
- 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) {
- ksft_print_msg("Cachestat returned non-zero.\n");
- ret = false;
- } else {
- print_cachestat(&cs);
- if (cs.nr_cache + cs.nr_evicted != num_pages) {
- ksft_print_msg("Total number of cached and evicted pages is off.\n");
- ret = false;
- }
- }
-
-close_fd:
- close(fd);
- unlink(filename);
-out:
- 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 */
@@ -271,27 +218,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) {
@@ -333,7 +296,7 @@ int main(void)
ret = 1;
}
- for (int i = 0; i < 6; i++) {
+ for (int i = 0; i < 5; i++) {
const char *dev_filename = dev_files[i];
if (test_cachestat(dev_filename, false, false, false,
@@ -367,14 +330,14 @@ 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 (test_cachestat_mmap())
+ 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");
--
2.43.0
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v2] selftests: cachestat: Refactor test to remove duplication 2025-06-11 18:09 [PATCH v2] selftests: cachestat: Refactor test to remove duplication Suresh K C @ 2025-06-25 4:58 ` Suresh Chandrappa 2025-06-27 16:41 ` Nhat Pham 0 siblings, 1 reply; 5+ messages in thread From: Suresh Chandrappa @ 2025-06-25 4:58 UTC (permalink / raw) To: nphamcs, hannes, shuah; +Cc: linux-mm, linux-kselftest, linux-kernel [-- Attachment #1: Type: text/plain, Size: 6705 bytes --] Hi @nphamcs Can you please check the modified change Thanks Suresh K C On Wed, 11 Jun 2025, 23:39 Suresh K C, <suresh.k.chandrappa@gmail.com> wrote: > From: Suresh K C <suresh.k.chandrappa@gmail.com> > > Refactored the mmap and shmem test logic into a common function > to reduce code duplication and improve maintainability > > Changes in v2: > Refactored mmap and shmem tests into a common function > Renamed test function to run_cachestat_test() > Removed test for /proc/cpuinfo as a general /proc test case already > exists > > Signed-off-by: Suresh K C <suresh.k.chandrappa@gmail.com> > --- > .../selftests/cachestat/test_cachestat.c | 97 ++++++------------- > 1 file changed, 30 insertions(+), 67 deletions(-) > > diff --git a/tools/testing/selftests/cachestat/test_cachestat.c > b/tools/testing/selftests/cachestat/test_cachestat.c > index 81e7f6dd2279..7c2f64175943 100644 > --- a/tools/testing/selftests/cachestat/test_cachestat.c > +++ b/tools/testing/selftests/cachestat/test_cachestat.c > @@ -22,7 +22,7 @@ > > static const char * const dev_files[] = { > "/dev/zero", "/dev/null", "/dev/urandom", > - "/proc/version","/proc/cpuinfo","/proc" > + "/proc/version","/proc" > }; > > void print_cachestat(struct cachestat *cs) > @@ -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,66 +207,8 @@ static int test_cachestat(const char *filename, bool > write_random, bool create, > return ret; > } > > -bool test_cachestat_mmap(void){ > - > - size_t PS = sysconf(_SC_PAGESIZE); > - size_t filesize = PS * 512 * 2;; > - int syscall_ret; > - size_t compute_len = PS * 512; > - struct cachestat_range cs_range = { PS, compute_len }; > - char *filename = "tmpshmcstat"; > - unsigned long num_pages = compute_len / PS; > - struct cachestat cs; > - bool ret = true; > - int fd = open(filename, O_RDWR | O_CREAT | O_TRUNC, 0666); > - if (fd < 0) { > - ksft_print_msg("Unable to create mmap file.\n"); > - ret = false; > - goto out; > - } > - if (ftruncate(fd, filesize)) { > - ksft_print_msg("Unable to truncate mmap file.\n"); > - ret = false; > - goto close_fd; > - } > - if (!write_exactly(fd, filesize)) { > - ksft_print_msg("Unable to write to mmap file.\n"); > - ret = false; > - goto close_fd; > - } > - 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) { > - ksft_print_msg("Cachestat returned non-zero.\n"); > - ret = false; > - } else { > - print_cachestat(&cs); > - if (cs.nr_cache + cs.nr_evicted != num_pages) { > - ksft_print_msg("Total number of cached and evicted > pages is off.\n"); > - ret = false; > - } > - } > - > -close_fd: > - close(fd); > - unlink(filename); > -out: > - 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 */ > @@ -271,27 +218,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) { > @@ -333,7 +296,7 @@ int main(void) > ret = 1; > } > > - for (int i = 0; i < 6; i++) { > + for (int i = 0; i < 5; i++) { > const char *dev_filename = dev_files[i]; > > if (test_cachestat(dev_filename, false, false, false, > @@ -367,14 +330,14 @@ 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 (test_cachestat_mmap()) > + 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"); > -- > 2.43.0 > > [-- Attachment #2: Type: text/html, Size: 8875 bytes --] ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] selftests: cachestat: Refactor test to remove duplication 2025-06-25 4:58 ` Suresh Chandrappa @ 2025-06-27 16:41 ` Nhat Pham 2025-06-30 17:37 ` Suresh Chandrappa 0 siblings, 1 reply; 5+ messages in thread From: Nhat Pham @ 2025-06-27 16:41 UTC (permalink / raw) To: Suresh Chandrappa; +Cc: hannes, shuah, linux-mm, linux-kselftest, linux-kernel On Tue, Jun 24, 2025 at 9:58 PM Suresh Chandrappa <suresh.k.chandrappa@gmail.com> wrote: > > Hi @nphamcs > Can you please check the modified change Is this supposed to be on top of the earlier patch you sent out? In that case, you should send both together as a patch series. > > Thanks > Suresh K C > > > On Wed, 11 Jun 2025, 23:39 Suresh K C, <suresh.k.chandrappa@gmail.com> wrote: >> >> From: Suresh K C <suresh.k.chandrappa@gmail.com> >> >> Refactored the mmap and shmem test logic into a common function >> to reduce code duplication and improve maintainability >> >> Changes in v2: >> Refactored mmap and shmem tests into a common function >> Renamed test function to run_cachestat_test() >> Removed test for /proc/cpuinfo as a general /proc test case already exists >> >> Signed-off-by: Suresh K C <suresh.k.chandrappa@gmail.com> >> --- >> .../selftests/cachestat/test_cachestat.c | 97 ++++++------------- >> 1 file changed, 30 insertions(+), 67 deletions(-) >> >> diff --git a/tools/testing/selftests/cachestat/test_cachestat.c b/tools/testing/selftests/cachestat/test_cachestat.c >> index 81e7f6dd2279..7c2f64175943 100644 >> --- a/tools/testing/selftests/cachestat/test_cachestat.c >> +++ b/tools/testing/selftests/cachestat/test_cachestat.c >> @@ -22,7 +22,7 @@ >> >> static const char * const dev_files[] = { >> "/dev/zero", "/dev/null", "/dev/urandom", >> - "/proc/version","/proc/cpuinfo","/proc" >> + "/proc/version","/proc" So you removed one file that you added in an earlier patch, right? Then why bother adding it in the first place...? Can you either: 1. Send the two patch together as a series. In the first patch, do not add /proc/cpuinfo. or 2. Squash them into a single patch. I'll let you decide if this is worth it. >> }; >> >> void print_cachestat(struct cachestat *cs) >> @@ -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,66 +207,8 @@ static int test_cachestat(const char *filename, bool write_random, bool create, >> return ret; >> } >> >> -bool test_cachestat_mmap(void){ >> - >> - size_t PS = sysconf(_SC_PAGESIZE); >> - size_t filesize = PS * 512 * 2;; >> - int syscall_ret; >> - size_t compute_len = PS * 512; >> - struct cachestat_range cs_range = { PS, compute_len }; >> - char *filename = "tmpshmcstat"; >> - unsigned long num_pages = compute_len / PS; >> - struct cachestat cs; >> - bool ret = true; >> - int fd = open(filename, O_RDWR | O_CREAT | O_TRUNC, 0666); >> - if (fd < 0) { >> - ksft_print_msg("Unable to create mmap file.\n"); >> - ret = false; >> - goto out; >> - } >> - if (ftruncate(fd, filesize)) { >> - ksft_print_msg("Unable to truncate mmap file.\n"); >> - ret = false; >> - goto close_fd; >> - } >> - if (!write_exactly(fd, filesize)) { >> - ksft_print_msg("Unable to write to mmap file.\n"); >> - ret = false; >> - goto close_fd; >> - } >> - 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) { >> - ksft_print_msg("Cachestat returned non-zero.\n"); >> - ret = false; >> - } else { >> - print_cachestat(&cs); >> - if (cs.nr_cache + cs.nr_evicted != num_pages) { >> - ksft_print_msg("Total number of cached and evicted pages is off.\n"); >> - ret = false; >> - } >> - } >> - >> -close_fd: >> - close(fd); >> - unlink(filename); >> -out: >> - return ret; >> -} >> >> -bool test_cachestat_shmem(void) >> +bool run_cachestat_test(enum file_type type) Can you just call this function test_cachestat(enum file_type type) ? >> { >> size_t PS = sysconf(_SC_PAGESIZE); >> size_t filesize = PS * 512 * 2; /* 2 2MB huge pages */ >> @@ -271,27 +218,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) { >> @@ -333,7 +296,7 @@ int main(void) >> ret = 1; >> } >> >> - for (int i = 0; i < 6; i++) { >> + for (int i = 0; i < 5; i++) { >> const char *dev_filename = dev_files[i]; >> >> if (test_cachestat(dev_filename, false, false, false, >> @@ -367,14 +330,14 @@ 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 (test_cachestat_mmap()) >> + 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"); >> -- >> 2.43.0 >> ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] selftests: cachestat: Refactor test to remove duplication 2025-06-27 16:41 ` Nhat Pham @ 2025-06-30 17:37 ` Suresh Chandrappa 2025-06-30 17:47 ` Suresh Chandrappa 0 siblings, 1 reply; 5+ messages in thread From: Suresh Chandrappa @ 2025-06-30 17:37 UTC (permalink / raw) To: Nhat Pham; +Cc: hannes, shuah, linux-mm [-- Attachment #1: Type: text/plain, Size: 8341 bytes --] Hi Nhat, Sure i will squash them to a single patch and will share it. Thanks Suresh K C On Fri, Jun 27, 2025 at 10:11 PM Nhat Pham <nphamcs@gmail.com> wrote: > On Tue, Jun 24, 2025 at 9:58 PM Suresh Chandrappa > <suresh.k.chandrappa@gmail.com> wrote: > > > > Hi @nphamcs > > Can you please check the modified change > > Is this supposed to be on top of the earlier patch you sent out? In > that case, you should send both together as a patch series. > > > > > Thanks > > Suresh K C > > > > > > On Wed, 11 Jun 2025, 23:39 Suresh K C, <suresh.k.chandrappa@gmail.com> > wrote: > >> > >> From: Suresh K C <suresh.k.chandrappa@gmail.com> > >> > >> Refactored the mmap and shmem test logic into a common function > >> to reduce code duplication and improve maintainability > >> > >> Changes in v2: > >> Refactored mmap and shmem tests into a common function > >> Renamed test function to run_cachestat_test() > >> Removed test for /proc/cpuinfo as a general /proc test case already > exists > >> > >> Signed-off-by: Suresh K C <suresh.k.chandrappa@gmail.com> > >> --- > >> .../selftests/cachestat/test_cachestat.c | 97 ++++++------------- > >> 1 file changed, 30 insertions(+), 67 deletions(-) > >> > >> diff --git a/tools/testing/selftests/cachestat/test_cachestat.c > b/tools/testing/selftests/cachestat/test_cachestat.c > >> index 81e7f6dd2279..7c2f64175943 100644 > >> --- a/tools/testing/selftests/cachestat/test_cachestat.c > >> +++ b/tools/testing/selftests/cachestat/test_cachestat.c > >> @@ -22,7 +22,7 @@ > >> > >> static const char * const dev_files[] = { > >> "/dev/zero", "/dev/null", "/dev/urandom", > >> - "/proc/version","/proc/cpuinfo","/proc" > >> + "/proc/version","/proc" > > So you removed one file that you added in an earlier patch, right? > Then why bother adding it in the first place...? > > Can you either: > > 1. Send the two patch together as a series. In the first patch, do not > add /proc/cpuinfo. > > or > > 2. Squash them into a single patch. I'll let you decide if this is worth > it. > > > >> }; > >> > >> void print_cachestat(struct cachestat *cs) > >> @@ -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,66 +207,8 @@ static int test_cachestat(const char *filename, > bool write_random, bool create, > >> return ret; > >> } > >> > >> -bool test_cachestat_mmap(void){ > >> - > >> - size_t PS = sysconf(_SC_PAGESIZE); > >> - size_t filesize = PS * 512 * 2;; > >> - int syscall_ret; > >> - size_t compute_len = PS * 512; > >> - struct cachestat_range cs_range = { PS, compute_len }; > >> - char *filename = "tmpshmcstat"; > >> - unsigned long num_pages = compute_len / PS; > >> - struct cachestat cs; > >> - bool ret = true; > >> - int fd = open(filename, O_RDWR | O_CREAT | O_TRUNC, 0666); > >> - if (fd < 0) { > >> - ksft_print_msg("Unable to create mmap file.\n"); > >> - ret = false; > >> - goto out; > >> - } > >> - if (ftruncate(fd, filesize)) { > >> - ksft_print_msg("Unable to truncate mmap file.\n"); > >> - ret = false; > >> - goto close_fd; > >> - } > >> - if (!write_exactly(fd, filesize)) { > >> - ksft_print_msg("Unable to write to mmap file.\n"); > >> - ret = false; > >> - goto close_fd; > >> - } > >> - 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) { > >> - ksft_print_msg("Cachestat returned non-zero.\n"); > >> - ret = false; > >> - } else { > >> - print_cachestat(&cs); > >> - if (cs.nr_cache + cs.nr_evicted != num_pages) { > >> - ksft_print_msg("Total number of cached and > evicted pages is off.\n"); > >> - ret = false; > >> - } > >> - } > >> - > >> -close_fd: > >> - close(fd); > >> - unlink(filename); > >> -out: > >> - return ret; > >> -} > >> > >> -bool test_cachestat_shmem(void) > >> +bool run_cachestat_test(enum file_type type) > > Can you just call this function test_cachestat(enum file_type type) ? > > >> { > >> size_t PS = sysconf(_SC_PAGESIZE); > >> size_t filesize = PS * 512 * 2; /* 2 2MB huge pages */ > >> @@ -271,27 +218,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) { > >> @@ -333,7 +296,7 @@ int main(void) > >> ret = 1; > >> } > >> > >> - for (int i = 0; i < 6; i++) { > >> + for (int i = 0; i < 5; i++) { > >> const char *dev_filename = dev_files[i]; > >> > >> if (test_cachestat(dev_filename, false, false, false, > >> @@ -367,14 +330,14 @@ 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 (test_cachestat_mmap()) > >> + 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"); > >> -- > >> 2.43.0 > >> > [-- Attachment #2: Type: text/html, Size: 11659 bytes --] ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] selftests: cachestat: Refactor test to remove duplication 2025-06-30 17:37 ` Suresh Chandrappa @ 2025-06-30 17:47 ` Suresh Chandrappa 0 siblings, 0 replies; 5+ messages in thread From: Suresh Chandrappa @ 2025-06-30 17:47 UTC (permalink / raw) To: Nhat Pham; +Cc: hannes, shuah, linux-mm [-- Attachment #1: Type: text/plain, Size: 9045 bytes --] Hi Nhat, >> -bool test_cachestat_shmem(void) >> +bool run_cachestat_test(enum file_type type) Can you just call this function test_cachestat(enum file_type type) ? We already have a function named *test_cachestat* that was introduced earlier in the codebase, so to avoid confusion or naming conflicts, I’ve named this one run_cachestat_test On Mon, Jun 30, 2025 at 11:07 PM Suresh Chandrappa < suresh.k.chandrappa@gmail.com> wrote: > Hi Nhat, > Sure i will squash them to a single patch and will share it. > > Thanks > Suresh K C > > On Fri, Jun 27, 2025 at 10:11 PM Nhat Pham <nphamcs@gmail.com> wrote: > >> On Tue, Jun 24, 2025 at 9:58 PM Suresh Chandrappa >> <suresh.k.chandrappa@gmail.com> wrote: >> > >> > Hi @nphamcs >> > Can you please check the modified change >> >> Is this supposed to be on top of the earlier patch you sent out? In >> that case, you should send both together as a patch series. >> >> > >> > Thanks >> > Suresh K C >> > >> > >> > On Wed, 11 Jun 2025, 23:39 Suresh K C, <suresh.k.chandrappa@gmail.com> >> wrote: >> >> >> >> From: Suresh K C <suresh.k.chandrappa@gmail.com> >> >> >> >> Refactored the mmap and shmem test logic into a common function >> >> to reduce code duplication and improve maintainability >> >> >> >> Changes in v2: >> >> Refactored mmap and shmem tests into a common function >> >> Renamed test function to run_cachestat_test() >> >> Removed test for /proc/cpuinfo as a general /proc test case >> already exists >> >> >> >> Signed-off-by: Suresh K C <suresh.k.chandrappa@gmail.com> >> >> --- >> >> .../selftests/cachestat/test_cachestat.c | 97 ++++++------------- >> >> 1 file changed, 30 insertions(+), 67 deletions(-) >> >> >> >> diff --git a/tools/testing/selftests/cachestat/test_cachestat.c >> b/tools/testing/selftests/cachestat/test_cachestat.c >> >> index 81e7f6dd2279..7c2f64175943 100644 >> >> --- a/tools/testing/selftests/cachestat/test_cachestat.c >> >> +++ b/tools/testing/selftests/cachestat/test_cachestat.c >> >> @@ -22,7 +22,7 @@ >> >> >> >> static const char * const dev_files[] = { >> >> "/dev/zero", "/dev/null", "/dev/urandom", >> >> - "/proc/version","/proc/cpuinfo","/proc" >> >> + "/proc/version","/proc" >> >> So you removed one file that you added in an earlier patch, right? >> Then why bother adding it in the first place...? >> >> Can you either: >> >> 1. Send the two patch together as a series. In the first patch, do not >> add /proc/cpuinfo. >> >> or >> >> 2. Squash them into a single patch. I'll let you decide if this is worth >> it. >> >> >> >> }; >> >> >> >> void print_cachestat(struct cachestat *cs) >> >> @@ -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,66 +207,8 @@ static int test_cachestat(const char *filename, >> bool write_random, bool create, >> >> return ret; >> >> } >> >> >> >> -bool test_cachestat_mmap(void){ >> >> - >> >> - size_t PS = sysconf(_SC_PAGESIZE); >> >> - size_t filesize = PS * 512 * 2;; >> >> - int syscall_ret; >> >> - size_t compute_len = PS * 512; >> >> - struct cachestat_range cs_range = { PS, compute_len }; >> >> - char *filename = "tmpshmcstat"; >> >> - unsigned long num_pages = compute_len / PS; >> >> - struct cachestat cs; >> >> - bool ret = true; >> >> - int fd = open(filename, O_RDWR | O_CREAT | O_TRUNC, 0666); >> >> - if (fd < 0) { >> >> - ksft_print_msg("Unable to create mmap file.\n"); >> >> - ret = false; >> >> - goto out; >> >> - } >> >> - if (ftruncate(fd, filesize)) { >> >> - ksft_print_msg("Unable to truncate mmap file.\n"); >> >> - ret = false; >> >> - goto close_fd; >> >> - } >> >> - if (!write_exactly(fd, filesize)) { >> >> - ksft_print_msg("Unable to write to mmap file.\n"); >> >> - ret = false; >> >> - goto close_fd; >> >> - } >> >> - 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) { >> >> - ksft_print_msg("Cachestat returned non-zero.\n"); >> >> - ret = false; >> >> - } else { >> >> - print_cachestat(&cs); >> >> - if (cs.nr_cache + cs.nr_evicted != num_pages) { >> >> - ksft_print_msg("Total number of cached and >> evicted pages is off.\n"); >> >> - ret = false; >> >> - } >> >> - } >> >> - >> >> -close_fd: >> >> - close(fd); >> >> - unlink(filename); >> >> -out: >> >> - return ret; >> >> -} >> >> >> >> -bool test_cachestat_shmem(void) >> >> +bool run_cachestat_test(enum file_type type) >> >> Can you just call this function test_cachestat(enum file_type type) ? >> >> >> { >> >> size_t PS = sysconf(_SC_PAGESIZE); >> >> size_t filesize = PS * 512 * 2; /* 2 2MB huge pages */ >> >> @@ -271,27 +218,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) { >> >> @@ -333,7 +296,7 @@ int main(void) >> >> ret = 1; >> >> } >> >> >> >> - for (int i = 0; i < 6; i++) { >> >> + for (int i = 0; i < 5; i++) { >> >> const char *dev_filename = dev_files[i]; >> >> >> >> if (test_cachestat(dev_filename, false, false, false, >> >> @@ -367,14 +330,14 @@ 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 (test_cachestat_mmap()) >> >> + 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"); >> >> -- >> >> 2.43.0 >> >> >> > [-- Attachment #2: Type: text/html, Size: 12646 bytes --] ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2025-06-30 17:47 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2025-06-11 18:09 [PATCH v2] selftests: cachestat: Refactor test to remove duplication Suresh K C 2025-06-25 4:58 ` Suresh Chandrappa 2025-06-27 16:41 ` Nhat Pham 2025-06-30 17:37 ` Suresh Chandrappa 2025-06-30 17:47 ` Suresh Chandrappa
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox