Re: [LTP] [PATCH 01/13] Hugetlb: Migrating libhugetlbfs mlock

[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/mlock.c test
>
> Test Description: The test checks that mlocking hugetlb areas works
> with all combinations of MAP_PRIVATE and MAP_SHARED with and without
> MAP_LOCKED specified.
>
> Signed-off-by: Tarun Sahu <tsahu@linux.ibm.com>
> ---
>  runtest/hugetlb                               |  1 +
>  testcases/kernel/mem/.gitignore               |  1 +
>  .../kernel/mem/hugetlb/hugemmap/hugemmap20.c  | 88 +++++++++++++++++++
>  3 files changed, 90 insertions(+)
>  create mode 100644 testcases/kernel/mem/hugetlb/hugemmap/hugemmap20.c
>
> diff --git a/runtest/hugetlb b/runtest/hugetlb
> index 4da1525a7..2dffa8421 100644
> --- a/runtest/hugetlb
> +++ b/runtest/hugetlb
> @@ -21,6 +21,7 @@ hugemmap16 hugemmap16
>  hugemmap17 hugemmap17
>  hugemmap18 hugemmap18
>  hugemmap19 hugemmap19
> +hugemmap20 hugemmap20
>  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 b6b3e5ddd..dfd372892 100644
> --- a/testcases/kernel/mem/.gitignore
> +++ b/testcases/kernel/mem/.gitignore
> @@ -20,6 +20,7 @@
>  /hugetlb/hugemmap/hugemmap17
>  /hugetlb/hugemmap/hugemmap18
>  /hugetlb/hugemmap/hugemmap19
> +/hugetlb/hugemmap/hugemmap20
>  /hugetlb/hugeshmat/hugeshmat01
>  /hugetlb/hugeshmat/hugeshmat02
>  /hugetlb/hugeshmat/hugeshmat03
> diff --git a/testcases/kernel/mem/hugetlb/hugemmap/hugemmap20.c
> b/testcases/kernel/mem/hugetlb/hugemmap/hugemmap20.c
> new file mode 100644
> index 000000000..9607d58b7
> --- /dev/null
> +++ b/testcases/kernel/mem/hugetlb/hugemmap/hugemmap20.c
> @@ -0,0 +1,88 @@
> +// SPDX-License-Identifier: LGPL-2.1-or-later
> +/*
> + * Copyright (C) 2005-2006 David Gibson & Adam Litke, IBM Corporation.
> + * Author: David Gibson & Adam Litke
> + */
> +
> +/*\
> + * [Description]
> + *
> + * The test checks that mlocking hugetlb areas works with all combinations
> + * of MAP_PRIVATE and MAP_SHARED with and without MAP_LOCKED specified.
> + */
> +
> +#include <stdio.h>
> +#include <sys/mount.h>
> +#include <limits.h>
> +#include <sys/param.h>
> +#include <sys/types.h>
> +#include <sys/resource.h>
> +
> +#include "hugetlb.h"
> +
> +#define MNTPOINT "hugetlbfs/"
> +static int  fd = -1;
> +static unsigned long hpage_size;
> +
> +static void test_simple_mlock(int flags, char *flags_str)
> +{
> +       void *p;
> +       int ret;
> +
> +       fd = tst_creat_unlinked(MNTPOINT, 0);
> +       p = SAFE_MMAP(0, hpage_size, PROT_READ|PROT_WRITE, flags, fd, 0);
> +
> +       ret = mlock(p, hpage_size);
> +       if (ret) {
> +               tst_res(TFAIL|TERRNO, "mlock() failed (flags %s)",
> flags_str);
> +               goto cleanup;
> +       }
> +
> +       ret = munlock(p, hpage_size);
> +       if (ret)
> +               tst_res(TFAIL|TERRNO, "munlock() failed (flags %s)",
> flags_str);
> +       else
> +               tst_res(TPASS, "mlock/munlock with %s hugetlb mmap",
> +                               flags_str);
> +cleanup:
> +       SAFE_MUNMAP(p, hpage_size);
> +       SAFE_CLOSE(fd);
> +}
> +
> +static void run_test(void)
> +{
>



> +       struct rlimit limit_info;
> +
> +       if (getrlimit(RLIMIT_MEMLOCK, &limit_info))
> +               tst_res(TWARN|TERRNO, "Unable to read locked memory
> rlimit");
> +       if (limit_info.rlim_cur < hpage_size)
> +               tst_brk(TCONF, "Locked memory ulimit set below huge page
> size");
>

These lines are better for moving into setup() phase. And, I'd propose
printing the value of 'limit_info.rlim_cur' and 'hpage_size' when TCONF.

The default value of max-locked-memory is smaller than hpage_size on
both RHEL8 and 9, which means this test will TCONF and skip running.
I'm hesitating should we temporally cancel the limitations and make
this test can really perform then restore that value to the original,
is this change make sense? WDYT?



> +
> +       test_simple_mlock(MAP_PRIVATE, "MAP_PRIVATE");
> +       test_simple_mlock(MAP_SHARED, "MAP_SHARED");
> +       test_simple_mlock(MAP_PRIVATE|MAP_LOCKED,
> "MAP_PRIVATE|MAP_LOCKED");
> +       test_simple_mlock(MAP_SHARED|MAP_LOCKED, "MAP_SHARED|MAP_LOCKED");
>

If we define an additional function like flags_to_str(int flags) for
converting
the flag into a string, which will be more simple for reading.

static char *flags_to_str(int flags)
{
       ...
}

static void test_simple_mlock(int flags)
{
        char *flags_str = flags_to_str(flags);
        ...
}



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

-- 
Regards,
Li Wang

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