On Mon, 7 Jul 2025 20:55:56 +0530 Suresh K C <[email protected]> wrote:
> From: Suresh K C <[email protected]> > > 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)
