Re: [LTP] [PATCH 02/13] Hugetlb: Migrating libhugetlbfs mmap-cow

[Li Wang <liwang@redhat.com>]

On Sun, Dec 25, 2022 at 11:42 PM Tarun Sahu <tsahu@linux.ibm.com> wrote:

> Migrating the libhugetlbfs/testcases/mmap-cow.c test
>
> Test Description: Tests copy-on-write semantics of large pages where a
> number of threads map the same file with the MAP_PRIVATE flag. The threads
> then write into their copy of the mapping and recheck the contents to
> ensure they were not corrupted by the other threads.
>
> Signed-off-by: Tarun Sahu <tsahu@linux.ibm.com>
> ---
>  runtest/hugetlb                               |   1 +
>  testcases/kernel/mem/.gitignore               |   1 +
>  .../kernel/mem/hugetlb/hugemmap/hugemmap21.c  | 131 ++++++++++++++++++
>  3 files changed, 133 insertions(+)
>  create mode 100644 testcases/kernel/mem/hugetlb/hugemmap/hugemmap21.c
>
> diff --git a/runtest/hugetlb b/runtest/hugetlb
> index 2dffa8421..71b4d57e4 100644
> --- a/runtest/hugetlb
> +++ b/runtest/hugetlb
> @@ -22,6 +22,7 @@ hugemmap17 hugemmap17
>  hugemmap18 hugemmap18
>  hugemmap19 hugemmap19
>  hugemmap20 hugemmap20
> +hugemmap21 hugemmap21
>  hugemmap05_1 hugemmap05 -m
>  hugemmap05_2 hugemmap05 -s
>  hugemmap05_3 hugemmap05 -s -m
> diff --git a/testcases/kernel/mem/.gitignore
> b/testcases/kernel/mem/.gitignore
> index dfd372892..74edfa392 100644
> --- a/testcases/kernel/mem/.gitignore
> +++ b/testcases/kernel/mem/.gitignore
> @@ -21,6 +21,7 @@
>  /hugetlb/hugemmap/hugemmap18
>  /hugetlb/hugemmap/hugemmap19
>  /hugetlb/hugemmap/hugemmap20
> +/hugetlb/hugemmap/hugemmap21
>  /hugetlb/hugeshmat/hugeshmat01
>  /hugetlb/hugeshmat/hugeshmat02
>  /hugetlb/hugeshmat/hugeshmat03
> diff --git a/testcases/kernel/mem/hugetlb/hugemmap/hugemmap21.c
> b/testcases/kernel/mem/hugetlb/hugemmap/hugemmap21.c
> new file mode 100644
> index 000000000..481edb4b0
> --- /dev/null
> +++ b/testcases/kernel/mem/hugetlb/hugemmap/hugemmap21.c
> @@ -0,0 +1,131 @@
> +// SPDX-License-Identifier: LGPL-2.1-or-later
> +/*
> + * Copyright (C) 2005-2006 IBM Corporation.
> + * Author: David Gibson & Adam Litke
> + */
> +
> +/*\
> + * [Description]
> + *
> + * Tests copy-on-write semantics of large pages where a number of threads
> + * map the same file with the MAP_PRIVATE flag. The threads then write
> + * into their copy of the mapping and recheck the contents to ensure they
> + * were not corrupted by the other threads.
> + */
> +
> +#include <stdio.h>
> +#include <sys/mount.h>
> +#include <limits.h>
> +#include <sys/param.h>
> +#include <sys/types.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <unistd.h>
> +#include <sys/stat.h>
> +#include <fcntl.h>
> +#include <unistd.h>
> +#include <sys/shm.h>
> +#include <sys/wait.h>
> +
> +#include "hugetlb.h"
> +
> +#define BUF_SZ 256
> +#define THREADS 5
> +#define NR_HUGEPAGES 6
> +#define MNTPOINT "hugetlbfs/"
> +
> +static int fd = -1;
> +static long hpage_size;
> +
> +static void do_work(int thread, size_t size, int fd)
> +{
> +       char *addr;
> +       size_t i;
> +       char pattern = thread+65;
> +
> +       addr = SAFE_MMAP(NULL, size, PROT_READ|PROT_WRITE, MAP_PRIVATE,
> fd, 0);
> +
> +       tst_res(TINFO, "Thread %d (pid %d): Mapped at address %p",
> +              thread, getpid(), addr);
> +
> +       for (i = 0; i < size; i++)
> +               memcpy((char *)addr+i, &pattern, 1);
> +
> +       if (msync(addr, size, MS_SYNC))
> +               tst_brk(TBROK|TERRNO, "Thread %d (pid %d): msync() failed",
> +                               thread, getpid());
> +
> +       for (i = 0; i < size; i++)
> +               if (addr[i] != pattern) {
> +                       tst_res(TFAIL, "thread %d (pid: %d): Corruption at
> %p; "
> +                                  "Got %c, Expected %c", thread, getpid(),
> +                                  &addr[i], addr[i], pattern);
> +                       goto cleanup;
> +               }
> +       tst_res(TINFO, "Thread %d (pid %d): Pattern verified",
> +                       thread, getpid());
>

Maybe combining the address output with the content of patterns is better?

i.e.

--- a/testcases/kernel/mem/hugetlb/hugemmap/hugemmap21.c
+++ b/testcases/kernel/mem/hugetlb/hugemmap/hugemmap21.c
@@ -45,9 +45,6 @@ static void do_work(int thread, size_t size, int fd)

        addr = SAFE_MMAP(NULL, size, PROT_READ|PROT_WRITE, MAP_PRIVATE, fd,
0);

-       tst_res(TINFO, "Thread %d (pid %d): Mapped at address %p",
-              thread, getpid(), addr);
-
        for (i = 0; i < size; i++)
                memcpy((char *)addr+i, &pattern, 1);

@@ -62,8 +59,8 @@ static void do_work(int thread, size_t size, int fd)
                                   &addr[i], addr[i], pattern);
                        goto cleanup;
                }
-       tst_res(TINFO, "Thread %d (pid %d): Pattern verified",
-                       thread, getpid());
+       tst_res(TINFO, "Thread %d (pid %d): Pattern %c verified at address
%p",
+                       thread, getpid(), pattern, addr);

 cleanup:
        SAFE_MUNMAP(addr, size);



> +
> +cleanup:
> +       SAFE_MUNMAP(addr, size);
> +       exit(0);
> +}
> +
> +static void run_test(void)
> +{
> +       char *addr;
> +       size_t size, itr;
> +       int i, pid;
> +       pid_t *wait_list;
> +
> +       wait_list = SAFE_MALLOC(THREADS * sizeof(pid_t));
> +       size = (NR_HUGEPAGES / (THREADS+1)) * hpage_size;
> +
> +
> +       /* First, mmap the file with MAP_SHARED and fill with data
> +        * If this is not done, then the fault handler will not be
> +        * called in the kernel since private mappings will be
> +        * created for the children at prefault time.
> +        */
> +       addr = SAFE_MMAP(NULL, size, PROT_READ|PROT_WRITE, MAP_SHARED, fd,
> 0);
> +
> +       for (itr = 0; itr < size; itr += 8)
> +               memcpy(addr+itr, "deadbeef", 8);
> +
> +       for (i = 0; i < THREADS; i++) {
> +               pid = SAFE_FORK();
> +
> +               if (pid == 0)
> +                       do_work(i, size, fd);
> +
> +               wait_list[i] = pid;
> +       }
> +       tst_reap_children();
> +
> +       SAFE_MUNMAP(addr, size);
> +       free(wait_list);
> +       tst_res(TPASS, "mmap COW working as expected.");
> +}
> +
> +static void setup(void)
> +{
> +       hpage_size = SAFE_READ_MEMINFO(MEMINFO_HPAGE_SIZE)*1024;
>

We do have a dedicated function named tst_get_hugepage_size();
could be used for getting huge page size.

The rest part looks good to me.

Reviewed-by: Li Wang <liwang@redhat.com>



> +       fd = tst_creat_unlinked(MNTPOINT, 0);
> +}
> +
> +static void cleanup(void)
> +{
> +       if (fd >= 1)
> +               SAFE_CLOSE(fd);
> +}
> +
> +static struct tst_test test = {
> +       .needs_root = 1,
> +       .mntpoint = MNTPOINT,
> +       .needs_hugetlbfs = 1,
> +       .needs_tmpdir = 1,
> +       .forks_child = 1,
> +       .setup = setup,
> +       .cleanup = cleanup,
> +       .test_all = run_test,
> +       .hugepages = {NR_HUGEPAGES, TST_NEEDS},
> +};
> --
> 2.31.1
>
>

-- 
Regards,
Li Wang

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp