Re: [PATCH v2 2/2] selftests/damon/sysfs.py: validate memcg_path staging readback

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The newly added `test_memcg_filter_memcg_path_staging` test inadvertently depends on `CONFIG_MEMCG` and active cgroup configurations due to a call to `commit()`, causing the test to fail inappropriately on environments lacking them.
--

commit 45ed1607d8ff8754a1a322fb6da3631cdc56f283
Author: Cheng Nie <niecheng1@uniontech.com>

selftests/damon/sysfs.py: validate memcg_path staging readback

This commit adds a dedicated test to selftests/damon/sysfs.py that writes
memcg_path to sysfs and verifies its readback. It aims to test the sysfs
behavior independently of the local userspace cgroup mount layout.

> diff --git a/tools/testing/selftests/damon/sysfs.py b/tools/testing/selftests/damon/sysfs.py
> index aa03a1187489f..0f20bc9c46b78 100755
> --- a/tools/testing/selftests/damon/sysfs.py
> +++ b/tools/testing/selftests/damon/sysfs.py
> @@ -250,6 +250,43 @@ def assert_ctxs_committed(kdamonds):
[ ... ]
> +    context = _damon_sysfs.DamonCtx(
> +            targets=[_damon_sysfs.DamonTarget(pid=-1)],
> +            schemes=[_damon_sysfs.Damos(ops_filters=[memcg_filter])])
> +    context.idx = 0
> +    context.kdamond = kdamonds.kdamonds[0]
> +    kdamonds.kdamonds[0].contexts = [context]
> +
> +    err = kdamonds.kdamonds[0].commit()

[Severity: Medium]
Could this cause a regression where the test fails on environments lacking
memcg support or active cgroups?

The stated goal of this test is to validate the sysfs staging readback
without depending on local userspace cgroup mount layouts. However, when
commit() is called, it instructs the kernel to apply the configuration.

This triggers damon_sysfs_memcg_path_to_id(), which traverses online memory
cgroups and will return -EINVAL if memcg support is disabled or the root
cgroup '/' cannot be resolved to an active memcg. This causes the commit()
operation to fail, which makes the entire selftest report a regression.

Would it be better to only stage the values (e.g., via context.stage())
and then verify the readback, without triggering the operational commit
step?

> +    if err is not None:
> +        fail('memcg_path staging: kdamond commit', {'error': err})

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/4CDA28DC581FDCF8+20260529090845.1696845-1-niecheng1@uniontech.com?part=2