Message-ID: <CAKEwX=M6WOoE+A=6DVdAjR6J2tyULLGAqiZ6325Fn7R=Tn=igQ@mail.gmail.com>
Date: Thu, 10 Jul 2025 07:35:53 -0700
From: Nhat Pham <nphamcs@...il.com>
To: Suresh K C <suresh.k.chandrappa@...il.com>
Cc: hannes@...xchg.org, joshua.hahnjy@...il.com, shuah@...nel.org, 
	linux-mm@...ck.org, linux-kselftest@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3] selftests: cachestat: add tests for mmap Refactor and
 enhance mmap test for cachestat validation

On Wed, Jul 9, 2025 at 10:47 AM Suresh K C
<suresh.k.chandrappa@...il.com> wrote:
>
> From: Suresh K C <suresh.k.chandrappa@...il.com>
>
> This patch merges the previous two patches into a single,
> cohesive test case that verifies cachestat behavior with memory-mapped files using mmap().
> It also refactors the test logic to reduce redundancy, improve error reporting, and clarify failure messages for both shmem and mmap file types.
>
> Changes since v2:
>
> - Merged the two patches into one as suggested
> - Improved error messages for better clarity
>
> Tested on x86_64 with default kernel config.
>
> Signed-off-by: Suresh K C <suresh.k.chandrappa@...il.com>
> ---
>  .../selftests/cachestat/test_cachestat.c      | 66 +++++++++++++++----
>  1 file changed, 55 insertions(+), 11 deletions(-)
>
> diff --git a/tools/testing/selftests/cachestat/test_cachestat.c b/tools/testing/selftests/cachestat/test_cachestat.c
> index 632ab44737ec..18188d7c639e 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);
> @@ -201,8 +206,19 @@ 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 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 +228,49 @@ 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 %s file.\n",file_type_str(type));
>                 ret = false;
>                 goto out;
>         }
>
>         if (ftruncate(fd, filesize)) {
> -               ksft_print_msg("Unable to truncate shmem file.\n");
> +               ksft_print_msg("Unable to truncate %s file.\n",file_type_str(type));
>                 ret = false;
>                 goto close_fd;
>         }
> -
> -       if (!write_exactly(fd, filesize)) {
> -               ksft_print_msg("Unable to write to shmem file.\n");
> -               ret = false;
> -               goto close_fd;
> +       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;
> +                       break;
>         }
> -
>         syscall_ret = syscall(__NR_cachestat, fd, &cs_range, &cs, 0);
>
>         if (syscall_ret) {
> @@ -308,12 +346,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
>

Code LGTM, and the test passed on my system (x86-64 too though). FWIW:

Tested-by: Nhat Pham <nphamcs@...il.com>
Acked-by: Nhat Pham <nphamcs@...il.com>

Thanks for the test contribution, Suresh! And thanks for the review
help, Joshua :)

Powered by blists - more mailing lists