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

[Li Wang]

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.


-- 
Regards,
Li Wang