Reviewed-by: Pavel Tikhomirov <[email protected]>

On 12/8/25 19:46, Dmitry Sepp wrote:
> The patches introduce a set of tests used to verify the following 
> functionality:
>  - Controlling the amount of memory used by page cache with memory.cache
>  - Enforcing migration of cgroup's pages to one or multiple target NUMA nodes
>    with memory.numa_migrate.
> 
> Changes in v5:
>  - Replace EXPECT with ASSERT.
>  - Drop crc32 as it is slow (~1 GiB/s on the test machine) and prevents the 
> test
>    region pages from becoming "active" enough.
>  - Improve error checking by explicitly asserting a positive return code 
> instead
>    of asserting absence of a negative error code (where possible).
>  - Use periodic stat checks with timeouts instead of sleep() calls.
>  - Remove the expected-to-fail markers from the "inactive" tests. The
>    inactive_anon test is executed with MGLRU (lru_gen) disabled. See the 
> commit
>    message for more details.
>  - Unify verification functionality for active and inactive pages in
>    numa_migrate.
>  - Provide some test description at the beginning of the source files.
> 
> Notes for reviewers:
>  - The kselftest harness is designed in such a way that it does not allow to
>    skip any tests that have been registered before the execution started. This
>    means, there is no safe and sane way to interrupt the overall flow if one 
> of
>    the tests has failed. I tried using a state variable but each of the
>    individual tests seems to be executed in a subprocess so if it fails it has
>    no way to change anything outside. I also tried creating a marker file but 
> it
>    is also troublesome as the main harness function terminates the calling
>    process on its own without letting the external caller perform any cleanup
>    (e.g. to remove the marker file after all tests have finished).
>  - Regarding the ASSERT macro and invocation of the teardown function:
>    - Teardown is not automatically called if ASSERT is used in FIXTURE_SETUP.
>    - Teardown is called if ASSERT is used from the test functions defined with
>      the TEST macro and its variants.
>    - Teardown will be called if ASSERT is used from any non-harness-macro
>      defined function if the "context" variables (_metadata  etc.) are visible
>      within the function's scope and the function is called from the process
>      running the main test function.
>    - Teardown can theoretically be called if ASSERT is used from a child 
> process
>      started by the main test function if "context" is passed to the 
> function(s)
>      executed by the child process but in reality it makes little sense as it 
> is
>      possible that teardown would be then executed twice leading to 
> potentially
>      harmful outcome (once by the ASSERT plumbing from the child process and
>      once by the main test function because the child has failed).
> 
> 
> Dmitry Sepp (2):
>   selftests: cgroup: test page cache limiting feature
>   selftests: cgroup: test numa migration of lruvecs
> 
>  tools/testing/selftests/cgroup/Makefile       |   4 +
>  tools/testing/selftests/cgroup/config         |   4 +
>  tools/testing/selftests/cgroup/test_cache.c   | 327 +++++++
>  .../selftests/cgroup/test_numa_migrate.c      | 823 ++++++++++++++++++
>  4 files changed, 1158 insertions(+)
>  create mode 100644 tools/testing/selftests/cgroup/test_cache.c
>  create mode 100644 tools/testing/selftests/cgroup/test_numa_migrate.c
> 

-- 
Best regards, Pavel Tikhomirov
Senior Software Developer, Virtuozzo.

_______________________________________________
Devel mailing list
[email protected]
https://lists.openvz.org/mailman/listinfo/devel

Reply via email to