[LTP] [PATCH] mempolicy/mbind: update syscall tests for weighted interleave

[Gregory Price]

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