Re: [PATCH v3 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] Test fails on teardown when `start()` fails, defeating the stated intent of decoupling the test from `CONFIG_MEMCG`.
--

commit 995afe8f607b12ca4f4469ac19ac53b1340cae11
Author: Cheng Nie <niecheng1@uniontech.com>

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

This commit adds a dedicated test to write memcg_path to sysfs and verify
the readback path. It intentionally ignores kdamonds.start() failures so
the test operates independently of CONFIG_MEMCG or cgroup layout.

> diff --git a/tools/testing/selftests/damon/sysfs.py b/tools/testing/selftests/damon/sysfs.py
> index aa03a1187489f..7a17ab88361bc 100755
> --- a/tools/testing/selftests/damon/sysfs.py
> +++ b/tools/testing/selftests/damon/sysfs.py
> @@ -250,6 +250,38 @@ def assert_ctxs_committed(kdamonds):
[ ... ]
> +def test_memcg_filter_memcg_path_staging():
[ ... ]
> +    kdamonds.start()
> +
> +    shown, rd_err = _damon_sysfs.read_file(
> +            os.path.join(memcg_filter.sysfs_dir(), 'memcg_path'))
> +    if rd_err is not None:
> +        print('memcg_path staging: sysfs read (%s)' % rd_err)
> +        kdamonds.stop()
> +        exit(1)
> +    if shown.rstrip('\n') != memcg_filter.memcg_path:
> +        print('memcg_path staging: memcg_path readback '
> +              '(shown=%s, expected=%s)' %
> +              (shown.rstrip('\n'), memcg_filter.memcg_path))
> +        kdamonds.stop()
> +        exit(1)
> +
> +    err = kdamonds.stop()
> +    if err is not None:
> +        print('memcg_path staging: kdamond stop (%s)' % err)
> +        exit(1)

[Severity: Medium]
If start() fails as expected when CONFIG_MEMCG is disabled, won't this
subsequent call to kdamonds.stop() return an error?

Writing 'off' to the 'state' sysfs file of an unstarted kdamond causes
damon_sysfs_turn_damon_off() to unconditionally return -EINVAL. This error
propagates back to the Python script via write_file(), causing the script
to exit with status 1.

Does checking this error and exiting with a failure defeat the stated intent
of ignoring the start() failure in the first place?

> +    kdamonds = None
> +

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