mempolicy/mbind: update syscall tests for weighted interleave

Add the MPOL_WEIGHTED_INTERLEAVE policy to the syscall test suite for
get_mempolicy, set_mempolicy, and mbind.

The functional memory distribution test only tests that the default
behavior of WEIGHTED_INTERLEAVE is the same as INTERLEAVE, since the
values for each node default to '1', and because behavior actually
depends on what node the test runs on.  This test at least demonstrates
the interleave mechanism is in use and works by default as intended.

MPOL_WEIGHTED_INTERLEAVE is ifdef defined because it is not upstream
in libnuma yet, so this ensures compilation.

Signed-off-by: Gregory Price <gregory.price@memverge.com>
---
 .../syscalls/get_mempolicy/get_mempolicy01.c  |  15 ++
 testcases/kernel/syscalls/mbind/mbind01.c     |  17 ++
 testcases/kernel/syscalls/mbind/mbind02.c     |   5 +
 testcases/kernel/syscalls/mbind/mbind03.c     |   5 +
 testcases/kernel/syscalls/mbind/mbind04.c     |   5 +
 .../kernel/syscalls/set_mempolicy/.gitignore  |   2 +
 .../kernel/syscalls/set_mempolicy/Makefile    |   2 +-
 .../syscalls/set_mempolicy/set_mempolicy06.c  | 122 +++++++++++++++
 .../syscalls/set_mempolicy/set_mempolicy07.c  | 148 ++++++++++++++++++
 9 files changed, 320 insertions(+), 1 deletion(-)
 create mode 100644 testcases/kernel/syscalls/set_mempolicy/set_mempolicy06.c
 create mode 100644 testcases/kernel/syscalls/set_mempolicy/set_mempolicy07.c

Hi Gregory,

Thank you for starting this, comments are inline below.

On Tue, Apr 2, 2024 at 5:22 PM Gregory Price <gourry.memverge@gmail.com>
wrote:

> Add the MPOL_WEIGHTED_INTERLEAVE policy to the syscall test suite for
> get_mempolicy, set_mempolicy, and mbind.
>
> The functional memory distribution test only tests that the default
> behavior of WEIGHTED_INTERLEAVE is the same as INTERLEAVE, since the
> values for each node default to '1', and because behavior actually
> depends on what node the test runs on.  This test at least demonstrates
> the interleave mechanism is in use and works by default as intended.
>
> MPOL_WEIGHTED_INTERLEAVE is ifdef defined because it is not upstream
> in libnuma yet, so this ensures compilation.
>
> Signed-off-by: Gregory Price <gregory.price@memverge.com>
> ---
>  .../syscalls/get_mempolicy/get_mempolicy01.c  |  15 ++
>  testcases/kernel/syscalls/mbind/mbind01.c     |  17 ++
>  testcases/kernel/syscalls/mbind/mbind02.c     |   5 +
>  testcases/kernel/syscalls/mbind/mbind03.c     |   5 +
>  testcases/kernel/syscalls/mbind/mbind04.c     |   5 +
>  .../kernel/syscalls/set_mempolicy/.gitignore  |   2 +
>  .../kernel/syscalls/set_mempolicy/Makefile    |   2 +-
>  .../syscalls/set_mempolicy/set_mempolicy06.c  | 122 +++++++++++++++
>  .../syscalls/set_mempolicy/set_mempolicy07.c  | 148 ++++++++++++++++++
>  9 files changed, 320 insertions(+), 1 deletion(-)
>  create mode 100644
> testcases/kernel/syscalls/set_mempolicy/set_mempolicy06.c
>  create mode 100644
> testcases/kernel/syscalls/set_mempolicy/set_mempolicy07.c
>
> diff --git a/testcases/kernel/syscalls/get_mempolicy/get_mempolicy01.c
> b/testcases/kernel/syscalls/get_mempolicy/get_mempolicy01.c
> index 23f62091a..8b0aa60bf 100644
> --- a/testcases/kernel/syscalls/get_mempolicy/get_mempolicy01.c
> +++ b/testcases/kernel/syscalls/get_mempolicy/get_mempolicy01.c
> @@ -26,6 +26,10 @@
>  #include <errno.h>
>  #include "tst_numa.h"
>
> +#ifndef
>

#ifndef MPOL_WEIGHTED_INTERLEAVE ?


> +#define MPOL_WEIGHTED_INTERLEAVE 6
> +#endif
>

And can we move this common part into include/lapi/numaif.h,
to avoid defining this again and again in test cases?


+
>  #define MEM_LENGTH     (4 * 1024 * 1024)
>  #define PAGES_ALLOCATED 16u
>
> @@ -68,6 +72,11 @@ static struct test_case tcase[] = {
>                 .alloc = test_set_mempolicy_default,
>                 .exp_nodemask = &nodemask,
>         },
> +       {
> +               POLICY_DESC(MPOL_WEIGHTED_INTERLEAVE),
> +               .alloc = test_set_mempolicy_default,
> +               .exp_nodemask = &nodemask,
> +       },
>         {
>                 POLICY_DESC_NO_TARGET(MPOL_PREFERRED),
>                 .alloc = test_set_mempolicy_none,
> @@ -95,6 +104,12 @@ static struct test_case tcase[] = {
>                 .alloc = test_set_mempolicy_default,
>                 .exp_nodemask = &nodemask,
>         },
> +       {
> +               POLICY_DESC_FLAGS(MPOL_WEIGHTED_INTERLEAVE, MPOL_F_ADDR),
> +               .pre_test = test_mbind_default,
> +               .alloc = test_set_mempolicy_default,
> +               .exp_nodemask = &nodemask,
> +       },
>         {
>                 POLICY_DESC_FLAGS_NO_TARGET(MPOL_PREFERRED, MPOL_F_ADDR),
>                 .pre_test = test_mbind_none,
> diff --git a/testcases/kernel/syscalls/mbind/mbind01.c
> b/testcases/kernel/syscalls/mbind/mbind01.c
> index 4b8d168cd..d3d3b0049 100644
> --- a/testcases/kernel/syscalls/mbind/mbind01.c
> +++ b/testcases/kernel/syscalls/mbind/mbind01.c
> @@ -20,6 +20,10 @@
>  #include "tst_numa.h"
>  #include "lapi/numaif.h"
>
> +#ifndef
> +#define MPOL_WEIGHTED_INTERLEAVE 6
> +#endif
> +
>  #ifdef HAVE_NUMA_V2
>
>  #define MEM_LENGTH (4 * 1024 * 1024)
> @@ -80,6 +84,12 @@ static struct test_case tcase[] = {
>                 .err = EINVAL,
>                 .test = test_none,
>         },
> +       {
> +               POLICY_DESC_TEXT(MPOL_WEIGHTED_INTERLEAVE, "no target"),
> +               .ret = -1,
> +               .err = EINVAL,
> +               .test = test_none,
> +       },
>         {
>                 POLICY_DESC(MPOL_INTERLEAVE),
>                 .ret = 0,
> @@ -87,6 +97,13 @@ static struct test_case tcase[] = {
>                 .test = test_default,
>                 .exp_nodemask = &nodemask,
>         },
> +       {
> +               POLICY_DESC(MPOL_WEIGHTED_INTERLEAVE),
> +               .ret = 0,
> +               .err = 0,
> +               .test = test_default,
> +               .exp_nodemask = &nodemask,
> +       },
>         {
>                 POLICY_DESC_TEXT(MPOL_PREFERRED, "no target"),
>                 .ret = 0,
> diff --git a/testcases/kernel/syscalls/mbind/mbind02.c
> b/testcases/kernel/syscalls/mbind/mbind02.c
> index cd6a03226..66d9cd826 100644
> --- a/testcases/kernel/syscalls/mbind/mbind02.c
> +++ b/testcases/kernel/syscalls/mbind/mbind02.c
> @@ -23,6 +23,10 @@
>  #include "tst_test.h"
>  #include "tst_numa.h"
>
> +#ifndef MPOL_WEIGHTED_INTERLEAVE
> +#define MPOL_WEIGHTED_INTERLEAVE 6
> +#endif
>

^

+
>  #ifdef HAVE_NUMA_V2
>
>  static size_t page_size;
> @@ -92,6 +96,7 @@ static const int modes[] = {
>         MPOL_PREFERRED,
>         MPOL_BIND,
>         MPOL_INTERLEAVE,
> +       MPOL_WEIGHTED_INTERLEAVE,
>  };
>
>  static void verify_mbind(unsigned int n)
> diff --git a/testcases/kernel/syscalls/mbind/mbind03.c
> b/testcases/kernel/syscalls/mbind/mbind03.c
> index 3d477c9cb..7da3c217d 100644
> --- a/testcases/kernel/syscalls/mbind/mbind03.c
> +++ b/testcases/kernel/syscalls/mbind/mbind03.c
> @@ -20,6 +20,10 @@
>  #include "tst_test.h"
>  #include "tst_numa.h"
>
> +#ifndef MPOL_WEIGHTED_INTERLEAVE
> +#define MPOL_WEIGHTED_INTERLEAVE 6
> +#endif
>

^

+
>  #ifdef HAVE_NUMA_V2
>
>  static size_t page_size;
> @@ -103,6 +107,7 @@ static const int modes[] = {
>         MPOL_PREFERRED,
>         MPOL_BIND,
>         MPOL_INTERLEAVE,
> +       MPOL_WEIGHTED_INTERLEAVE,
>  };
>
>  static void verify_mbind(unsigned int n)
> diff --git a/testcases/kernel/syscalls/mbind/mbind04.c
> b/testcases/kernel/syscalls/mbind/mbind04.c
> index 50028835f..db41f9e29 100644
> --- a/testcases/kernel/syscalls/mbind/mbind04.c
> +++ b/testcases/kernel/syscalls/mbind/mbind04.c
> @@ -20,6 +20,10 @@
>  #include "tst_test.h"
>  #include "tst_numa.h"
>
> +#ifndef MPOL_WEIGHTED_INTERLEAVE
> +#define MPOL_WEIGHTED_INTERLEAVE 6
> +#endif
>

^

+
>  #ifdef HAVE_NUMA_V2
>
>  static size_t page_size;
> @@ -114,6 +118,7 @@ static const int modes[] = {
>         MPOL_PREFERRED,
>         MPOL_BIND,
>         MPOL_INTERLEAVE,
> +       MPOL_WEIGHTED_INTERLEAVE,
>  };
>
>  static void verify_mbind(unsigned int n)
> diff --git a/testcases/kernel/syscalls/set_mempolicy/.gitignore
> b/testcases/kernel/syscalls/set_mempolicy/.gitignore
> index 4c121d2e0..3af10e9f9 100644
> --- a/testcases/kernel/syscalls/set_mempolicy/.gitignore
> +++ b/testcases/kernel/syscalls/set_mempolicy/.gitignore
> @@ -3,3 +3,5 @@
>  /set_mempolicy03
>  /set_mempolicy04
>  /set_mempolicy05
>


> +/set_mempolicy06
>


> +/set_mempolicy07
>


First, we do not suggest adding any new tests by applying one "big"
patch especially since this contains too many other irrelevant
modifications.
We'd better separate them in single to guarantee everything goes
well for traceability of the commit.

Second, I don't see any new code in set_mempolicy06/07, since you
only copied them from set_mempolicy02/04, even without any change of the
comments, this is bad for test maintenance work and involves redundant
stuff.

The recommended way is to add MPOL_WEIGHTED_INTERLEAVE in
the original case if you just want to validate the behavior similarly with
MPOL_INTERLEAVE.

But if you want to test something special/new of MPOL_WEIGHTED_INTERLEAVE,
I think that's necessary to create set_mempolicy06/07, and do something
validate that interleaving behavior is executed based on weights set in '
/sys/kernel/mm/mempolicy/weighted_interleave/'.



> diff --git a/testcases/kernel/syscalls/set_mempolicy/Makefile
> b/testcases/kernel/syscalls/set_mempolicy/Makefile
> index 100780dc3..04208837d 100644
> --- a/testcases/kernel/syscalls/set_mempolicy/Makefile
> +++ b/testcases/kernel/syscalls/set_mempolicy/Makefile
> @@ -5,7 +5,7 @@ LTPLIBS = ltpnuma
>
>  include $(top_srcdir)/include/mk/testcases.mk
>
> -NEEDS_LIBS = set_mempolicy01 set_mempolicy02 set_mempolicy03
> set_mempolicy04
> +NEEDS_LIBS = set_mempolicy01 set_mempolicy02 set_mempolicy03
> set_mempolicy04 set_mempolicy05 set_mempolicy06 set_mempolicy07
>
>  $(NEEDS_LIBS): LDLIBS  += $(NUMA_LIBS)
>  $(NEEDS_LIBS): LTPLDLIBS = -lltpnuma
> diff --git a/testcases/kernel/syscalls/set_mempolicy/set_mempolicy06.c
> b/testcases/kernel/syscalls/set_mempolicy/set_mempolicy06.c
> new file mode 100644
> index 000000000..287fd316a
> --- /dev/null
> +++ b/testcases/kernel/syscalls/set_mempolicy/set_mempolicy06.c
> @@ -0,0 +1,122 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (c) 2018 Cyril Hrubis <chrubis@suse.cz>
> + */
> +
> +/*
> + * We are testing set_mempolicy() with MPOL_WEIGHTED_INTERLEAVE.
> + *
> + * The test tries different subsets of memory nodes, sets the mask with
> + * memopolicy, and checks that the memory was interleaved between the
> nodes
> + * accordingly.
> + */
> +
> +#include <errno.h>
> +#include "config.h"
> +#ifdef HAVE_NUMA_V2
> +# include <numa.h>
> +# include <numaif.h>
> +#endif
> +#include "tst_test.h"
> +#include "tst_numa.h"
> +
> +#ifdef HAVE_NUMA_V2
> +
> +#include "set_mempolicy.h"
> +
> +#ifndef MPOL_WEIGHTED_INTERLEAVE
> +#define MPOL_WEIGHTED_INTERLEAVE 6
> +#endif
> +
> +#define ALLOC_ON_NODE 8
> +
> +static size_t page_size;
> +static struct tst_nodemap *nodes;
> +
> +static void setup(void)
> +{
> +       page_size = getpagesize();
> +
> +       nodes = tst_get_nodemap(TST_NUMA_MEM, 2 * ALLOC_ON_NODE *
> page_size / 1024);
> +       if (nodes->cnt <= 1)
> +               tst_brk(TCONF, "Test requires at least two NUMA memory
> nodes");
> +}
> +
> +static void cleanup(void)
> +{
> +       tst_nodemap_free(nodes);
> +}
> +
> +static void alloc_and_check(size_t size, unsigned int *exp_alloc)
> +{
> +       unsigned int i;
> +       const char *prefix = "child: ";
> +
> +       if (SAFE_FORK()) {
> +               prefix = "parent: ";
> +               tst_reap_children();
> +       }
> +
> +       tst_nodemap_reset_counters(nodes);
> +       alloc_fault_count(nodes, NULL, size * page_size);
> +
> +       for (i = 0; i < nodes->cnt; i++) {
> +               if (nodes->counters[i] == exp_alloc[i]) {
> +                       tst_res(TPASS, "%sNode %u allocated %u",
> +                                       prefix, nodes->map[i],
> exp_alloc[i]);
> +               } else {
> +                       tst_res(TFAIL, "%sNode %u allocated %u, expected
> %u",
> +                                       prefix, nodes->map[i],
> nodes->counters[i],
> +                                       exp_alloc[i]);
> +               }
> +       }
> +}
> +
> +static void verify_set_mempolicy(unsigned int n)
> +{
> +       struct bitmask *bm = numa_allocate_nodemask();
> +       unsigned int exp_alloc[nodes->cnt];
> +       unsigned int alloc_per_node = n ? ALLOC_ON_NODE : 2;
> +       unsigned int alloc_on_nodes = n ? 2 : nodes->cnt;
> +       unsigned int alloc_total = alloc_per_node * alloc_on_nodes;
> +       unsigned int i;
> +
> +       memset(exp_alloc, 0, sizeof(exp_alloc));
> +
> +       for (i = 0; i < alloc_on_nodes; i++) {
> +               exp_alloc[i] = alloc_per_node;
> +               numa_bitmask_setbit(bm, nodes->map[i]);
> +       }
> +
> +       TEST(set_mempolicy(MPOL_WEIGHTED_INTERLEAVE, bm->maskp,
> bm->size+1));
> +
> +       tst_res(TINFO, "Allocating on nodes 1-%u - %u pages",
> +                       alloc_on_nodes, alloc_total);
> +
> +       if (TST_RET) {
> +               tst_res(TFAIL | TTERRNO,
> +                               "set_mempolicy(MPOL_WEIGHTED_INTERLEAVE)");
> +               return;
> +       }
> +
> +       tst_res(TPASS, "set_mempolicy(MPOL_WEIGHTED_INTERLEAVE)");
> +
> +       numa_free_nodemask(bm);
> +
> +       alloc_and_check(alloc_total, exp_alloc);
> +}
> +
> +static struct tst_test test = {
> +       .setup = setup,
> +       .cleanup = cleanup,
> +       .test = verify_set_mempolicy,
> +       .tcnt = 2,
> +       .forks_child = 1,
> +       .needs_checkpoints = 1,
> +};
> +
> +#else
> +
> +TST_TEST_TCONF(NUMA_ERROR_MSG);
> +
> +#endif /* HAVE_NUMA_V2 */
> diff --git a/testcases/kernel/syscalls/set_mempolicy/set_mempolicy07.c
> b/testcases/kernel/syscalls/set_mempolicy/set_mempolicy07.c
> new file mode 100644
> index 000000000..d88998ed5
> --- /dev/null
> +++ b/testcases/kernel/syscalls/set_mempolicy/set_mempolicy07.c
> @@ -0,0 +1,148 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (c) 2018 Cyril Hrubis <chrubis@suse.cz>
> + */
> +
> +/*
> + * We are testing set_mempolicy() with MPOL_WEIGHTED_INTERLEAVE on mmaped
> buffers
> + * backed by files.
> + *
> + * Apparently it takes a larger sample for the allocations to be correctly
> + * interleaved. The reason for this is that buffers for file metadata are
> + * allocated in batches in order not to loose performance. Also the pages
> + * cannot be interleaved completely evenly unless the number of pages is
> + * divideable by the number of nodes, which will not happen even if we
> tried
> + * hard since we do not have control over metadata blocks for instance.
> Hence
> + * we cannot really expect to allocate a single file and have the memory
> + * interleaved precisely but it works well if we allocate statistic for a
> more
> + * than a few files.
> + */
> +
> +#include <stdio.h>
> +#include <errno.h>
> +#include "config.h"
> +#ifdef HAVE_NUMA_V2
> +# include <numa.h>
> +# include <numaif.h>
> +#endif
> +#include "tst_test.h"
> +#include "tst_numa.h"
> +
> +#define MNTPOINT "mntpoint"
> +#define FILES 10
> +
> +#ifdef HAVE_NUMA_V2
> +
> +#ifndef MPOL_WEIGHTED_INTERLEAVE
> +#define MPOL_WEIGHTED_INTERLEAVE 6
> +#endif
> +
> +#include "set_mempolicy.h"
> +
> +static size_t page_size;
> +static struct tst_nodemap *nodes;
> +
> +static void setup(void)
> +{
> +       int node_min_pages = FILES * (FILES + 1) / 2 * 10 + FILES * 10;
> +
> +       page_size = getpagesize();
> +
> +       nodes = tst_get_nodemap(TST_NUMA_MEM, node_min_pages * page_size /
> 1024);
> +       if (nodes->cnt <= 1)
> +               tst_brk(TCONF, "Test requires at least two NUMA memory
> nodes");
> +}
> +
> +static void cleanup(void)
> +{
> +       tst_nodemap_free(nodes);
> +}
> +
> +static void alloc_and_check(void)
> +{
> +       unsigned int i, j;
> +       char path[1024];
> +       unsigned int total_pages = 0;
> +       unsigned int sum_pages = 0;
> +
> +       tst_nodemap_reset_counters(nodes);
> +
> +       /*
> +        * The inner loop loops node->cnt times to ensure the sum could
> +        * be evenly distributed among the nodes.
> +        */
> +       for (i = 1; i <= FILES; i++) {
> +               for (j = 1; j <= nodes->cnt; j++) {
> +                       size_t size = 10 * i + j % 10;
> +
> +                       snprintf(path, sizeof(path), MNTPOINT
> "/numa-test-file-%i-%i", i, j);
> +                       alloc_fault_count(nodes, path, size * page_size);
> +                       total_pages += size;
> +               }
> +       }
> +
> +       for (i = 0; i < nodes->cnt; i++) {
> +               float threshold = 1.00 * total_pages / 60; /* five
> percents */
> +               float min_pages = 1.00 * total_pages / nodes->cnt -
> threshold;
> +               float max_pages = 1.00 * total_pages / nodes->cnt +
> threshold;
> +
> +               if (nodes->counters[i] > min_pages && nodes->counters[i] <
> max_pages) {
> +                       tst_res(TPASS, "Node %u allocated %u <%.2f,%.2f>",
> +                                       nodes->map[i], nodes->counters[i],
> min_pages, max_pages);
> +               } else {
> +                       tst_res(TFAIL, "Node %u allocated %u, expected
> <%.2f,%.2f>",
> +                                       nodes->map[i], nodes->counters[i],
> min_pages, max_pages);
> +               }
> +
> +               sum_pages += nodes->counters[i];
> +       }
> +
> +       if (sum_pages != total_pages) {
> +               tst_res(TFAIL, "Sum of nodes %u != allocated pages %u",
> +                               sum_pages, total_pages);
> +               return;
> +       }
> +
> +       tst_res(TPASS, "Sum of nodes equals to allocated pages (%u)",
> total_pages);
> +}
> +
> +static void verify_set_mempolicy(void)
> +{
> +       struct bitmask *bm = numa_allocate_nodemask();
> +       unsigned int alloc_on_nodes = nodes->cnt;
> +       unsigned int i;
> +
> +       for (i = 0; i < alloc_on_nodes; i++)
> +               numa_bitmask_setbit(bm, nodes->map[i]);
> +
> +       TEST(set_mempolicy(MPOL_WEIGHTED_INTERLEAVE, bm->maskp,
> bm->size+1));
> +
> +       if (TST_RET) {
> +               tst_res(TFAIL | TTERRNO,
> +                               "set_mempolicy(MPOL_WEIGHTED_INTERLEAVE)");
> +               return;
> +       }
> +
> +       tst_res(TPASS, "set_mempolicy(MPOL_WEIGHTED_INTERLEAVE)");
> +
> +       alloc_and_check();
> +
> +       numa_free_nodemask(bm);
> +}
> +
> +static struct tst_test test = {
> +       .setup = setup,
> +       .cleanup = cleanup,
> +       .test_all = verify_set_mempolicy,
> +       .forks_child = 1,
> +       .needs_root = 1,
> +       .all_filesystems = 1,
> +       .mntpoint = MNTPOINT,
> +       .needs_checkpoints = 1,
> +};
> +
> +#else
> +
> +TST_TEST_TCONF(NUMA_ERROR_MSG);
> +
> +#endif /* HAVE_NUMA_V2 */
> --
> 2.39.1
>
>
> --
> Mailing list info: https://lists.linux.it/listinfo/ltp
>
>

On Mon, Apr 08, 2024 at 03:40:38PM +0800, Li Wang wrote:
> Hi Gregory,
> 
> Thank you for starting this, comments are inline below.
> 
> > +#define MPOL_WEIGHTED_INTERLEAVE 6
> > +#endif
> >
> 
> And can we move this common part into include/lapi/numaif.h,
> to avoid defining this again and again in test cases?
> 

I have a pending patch to do just that, but it is not upstream yet.

This was a comment in the changelog:

> > MPOL_WEIGHTED_INTERLEAVE is ifdef defined because it is not upstream
> > in libnuma yet, so this ensures compilation.

Thought it was useful to shoot out a version of this before it all lands
for the sake of getting ahead of the curve a bit.

> 
> First, we do not suggest adding any new tests by applying one "big"
> patch especially since this contains too many other irrelevant
> modifications.
> We'd better separate them in single to guarantee everything goes
> well for traceability of the commit.
> 

Will do.

> Second, I don't see any new code in set_mempolicy06/07, since you
> only copied them from set_mempolicy02/04, even without any change of the
> comments, this is bad for test maintenance work and involves redundant
> stuff.
> 

the only major differences between the tests, presently, are that the
policy applied is weighted interleave

TEST(set_mempolicy(MPOL_WEIGHTED_INTERLEAVE, bm->maskp, bm->size+1));
                   ^^^^^^^^^^^^^^^^^^^^^^^^

In truth, this test isn't really completely, as it should also:

1) Set the sysfs values located at
/sys/kernel/mm/mempolicy/weighted_interleave/

2) Validate allocations actually match those settings

However, this test is quite complicated to write and make fully
reliable, as you also need to know

1) The environment (available nodes, cpu nodes, memory-only nodes)
2) The node the test will be run on (which can be forced)
3) Where allocations will start from (node X or node Y) as this can
   ultimately affect the final distribution.

In my tests separately, the test itself can also cause allocation
(stack, other regions) which may result in an unexpected distribution of
memory on the target region.  This is because those allocations are
credited as part of the interleaving, but the existing code of the test
cannot adjust for that. This may cause failures for no obvious reason.

This is ultimately why I left the tests mostly unchanged, because I
found it only reasonable to test the default behavior.

> The recommended way is to add MPOL_WEIGHTED_INTERLEAVE in
> the original case if you just want to validate the behavior similarly with
> MPOL_INTERLEAVE.
> 
> But if you want to test something special/new of MPOL_WEIGHTED_INTERLEAVE,
> I think that's necessary to create set_mempolicy06/07, and do something
> validate that interleaving behavior is executed based on weights set in '
> /sys/kernel/mm/mempolicy/weighted_interleave/'.
> 

Was hoping to get input on this.  I think probably trying to write a
test to track page-distribution of real weighted interleave will be
difficult to make reliable, so maybe I should just fold these tests back
into the original and note that this simply tests the default behavior
(which is equivalent to MPOL_INTERLEAVE).

That may require changing the original tests to ensure the sysfs files
are set to 1 to explicitly avoid failures.  But I wasn't sure if that
was ok since it would be making silent system-wide changes, and would
require root.

Thank you for the feedback
~Gregory

Hi Gregory,

On Wed, Apr 10, 2024 at 7:40 AM Gregory Price <gregory.price@memverge.com>
wrote:

> On Mon, Apr 08, 2024 at 03:40:38PM +0800, Li Wang wrote:
> > Hi Gregory,
> >
> > Thank you for starting this, comments are inline below.
> >
> > > +#define MPOL_WEIGHTED_INTERLEAVE 6
> > > +#endif
> > >
> >
> > And can we move this common part into include/lapi/numaif.h,
> > to avoid defining this again and again in test cases?
> >
>
> I have a pending patch to do just that, but it is not upstream yet.
>
> This was a comment in the changelog:
>
> > > MPOL_WEIGHTED_INTERLEAVE is ifdef defined because it is not upstream
> > > in libnuma yet, so this ensures compilation.
>
> Thought it was useful to shoot out a version of this before it all lands
> for the sake of getting ahead of the curve a bit.
>
> >
> > First, we do not suggest adding any new tests by applying one "big"
> > patch especially since this contains too many other irrelevant
> > modifications.
> > We'd better separate them in single to guarantee everything goes
> > well for traceability of the commit.
> >
>
> Will do.
>
> > Second, I don't see any new code in set_mempolicy06/07, since you
> > only copied them from set_mempolicy02/04, even without any change of the
> > comments, this is bad for test maintenance work and involves redundant
> > stuff.
> >
>
> the only major differences between the tests, presently, are that the
> policy applied is weighted interleave
>
> TEST(set_mempolicy(MPOL_WEIGHTED_INTERLEAVE, bm->maskp, bm->size+1));
>                    ^^^^^^^^^^^^^^^^^^^^^^^^
>
> In truth, this test isn't really completely, as it should also:
>
> 1) Set the sysfs values located at
> /sys/kernel/mm/mempolicy/weighted_interleave/
>
> 2) Validate allocations actually match those settings
>
> However, this test is quite complicated to write and make fully
> reliable, as you also need to know
>
> 1) The environment (available nodes, cpu nodes, memory-only nodes)
> 2) The node the test will be run on (which can be forced)
> 3) Where allocations will start from (node X or node Y) as this can
>    ultimately affect the final distribution.
>
> In my tests separately, the test itself can also cause allocation
> (stack, other regions) which may result in an unexpected distribution of
> memory on the target region.  This is because those allocations are
> credited as part of the interleaving, but the existing code of the test
> cannot adjust for that. This may cause failures for no obvious reason.
>
> This is ultimately why I left the tests mostly unchanged, because I
> found it only reasonable to test the default behavior.
>
> > The recommended way is to add MPOL_WEIGHTED_INTERLEAVE in
> > the original case if you just want to validate the behavior similarly
> with
> > MPOL_INTERLEAVE.
> >
> > But if you want to test something special/new of
> MPOL_WEIGHTED_INTERLEAVE,
> > I think that's necessary to create set_mempolicy06/07, and do something
> > validate that interleaving behavior is executed based on weights set in '
> > /sys/kernel/mm/mempolicy/weighted_interleave/'.
> >
>
> Was hoping to get input on this.  I think probably trying to write a
> test to track page-distribution of real weighted interleave will be
> difficult to make reliable, so maybe I should just fold these tests back
> into the original and note that this simply tests the default behavior
> (which is equivalent to MPOL_INTERLEAVE).
>

Ok, sure. If so merge the MPOL_WEIGHTED_INTERLEAVE into
MPOL_INTERLEAVE test should be enough.



>
> That may require changing the original tests to ensure the sysfs files
> are set to 1 to explicitly avoid failures.  But I wasn't sure if that
> was ok since it would be making silent system-wide changes, and would
> require root.
>

LTP lib provides a way to save/restore the '/proc | /sys' value
before and after doing the test, so it would not be difficult to
satisfy the requirement if it is just set 1 to a relative file.

see:
https://github.com/linux-test-project/ltp/blob/master/doc/old/C-Test-API.asciidoc#127-saving--restoring-procsys-values

Or, use tst_sys_conf_save() separately to traverse and save the value of
"mempolicy/weighted_interleave/node*" file if we are unsure how many nodes
are on the system, then set the value via:
    SAFE_FILE_PRINTF("../weighted_interleave/node*", "%d", 1);
Ultimately, LTP lib will restore all of the values to the original.

And set ".needs_root = 1," in the struct tst_test will be the requested
root permission.