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 wrote: > On Tue, Jun 24, 2025 at 9:58 PM Suresh Chandrappa > 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, > wrote: > >> > >> From: Suresh K C > >> > >> 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 > >> --- > >> .../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 > >> >