[PATCH v2 0/2] selftests/damon: fix memcg_path staging handling

[PATCH v2 0/2] selftests/damon: fix memcg_path staging handling

[Cheng Nie]

This series fixes memcg_path handling in DamosFilter and adds memcg
filter staging validation to the DAMON sysfs selftest.

Changes in v2:
- Use file-scoped commit subjects as suggested
- Move memcg_path readback check out of assert_filter_committed()
  into a dedicated test case at the end of main()

Cheng Nie (2):
  selftests/damon/_damon_sysfs.py: fix memcg_path assignment
  selftests/damon/sysfs.py: validate memcg_path staging readback

 tools/testing/selftests/damon/_damon_sysfs.py |  2 +-
 tools/testing/selftests/damon/sysfs.py        | 39 +++++++++++++++++++
 2 files changed, 40 insertions(+), 1 deletion(-)

-- 
2.51.0

[PATCH v2 1/2] selftests/damon/_damon_sysfs.py: fix memcg_path assignment

[Cheng Nie]

DamosFilter stores memcg_path for sysfs staging, but the constructor
assigns it with a trailing comma and therefore turns it into a tuple.

Fix the assignment so memcg_path is stored as the intended string.
This makes memcg filter staging and follow-up validation use the
written path correctly.

Signed-off-by: Cheng Nie <niecheng1@uniontech.com>
---
 tools/testing/selftests/damon/_damon_sysfs.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/damon/_damon_sysfs.py b/tools/testing/selftests/damon/_damon_sysfs.py
index 8b12cc048440..43075892215e 100644
--- a/tools/testing/selftests/damon/_damon_sysfs.py
+++ b/tools/testing/selftests/damon/_damon_sysfs.py
@@ -271,7 +271,7 @@ class DamosFilter:
         self.type_ = type_
         self.matching = matching
         self.allow = allow
-        self.memcg_path = memcg_path,
+        self.memcg_path = memcg_path
         self.addr_start = addr_start
         self.addr_end = addr_end
         self.target_idx = target_idx
-- 
2.51.0

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

[Cheng Nie]

Add a dedicated test at the end of main() that writes memcg_path to
sysfs and checks the readback path.  Validate the readback rather than
a derived memcg_id so the test stays focused on DAMON sysfs behavior
and avoids depending on the local userspace cgroup mount layout.

Signed-off-by: Cheng Nie <niecheng1@uniontech.com>
---
 tools/testing/selftests/damon/sysfs.py | 39 ++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/tools/testing/selftests/damon/sysfs.py b/tools/testing/selftests/damon/sysfs.py
index aa03a1187489..0f20bc9c46b7 100755
--- a/tools/testing/selftests/damon/sysfs.py
+++ b/tools/testing/selftests/damon/sysfs.py
@@ -250,6 +250,43 @@ def assert_ctxs_committed(kdamonds):
             if ctx in ctxs_paused_for_dump:
                 ctx.pause = False
 
+def test_memcg_filter_memcg_path_staging():
+    global kdamonds
+    memcg_filter = _damon_sysfs.DamosFilter(
+            type_='memcg', matching=True, allow=True, memcg_path='/')
+    kdamonds = _damon_sysfs.Kdamonds(
+            [_damon_sysfs.Kdamond(
+                contexts=[_damon_sysfs.DamonCtx(
+                    targets=[_damon_sysfs.DamonTarget(pid=-1)],
+                    schemes=[_damon_sysfs.Damos()],
+                    )])])
+    err = kdamonds.start()
+    if err is not None:
+        fail('memcg_path staging: kdamond start', {'error': err})
+
+    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()
+    if err is not None:
+        fail('memcg_path staging: kdamond commit', {'error': err})
+
+    shown, rd_err = _damon_sysfs.read_file(
+            os.path.join(memcg_filter.sysfs_dir(), 'memcg_path'))
+    if rd_err is not None:
+        fail('memcg_path staging: sysfs read', {'error': rd_err})
+    assert_true(shown.rstrip('\n') == memcg_filter.memcg_path,
+                'memcg_path readback', {'shown': shown})
+
+    err = kdamonds.stop()
+    if err is not None:
+        fail('memcg_path staging: kdamond stop', {'error': err})
+    kdamonds = None
+
 def main():
     global kdamonds
     kdamonds = _damon_sysfs.Kdamonds(
@@ -356,5 +393,7 @@ def main():
     assert_ctxs_committed(kdamonds)
     kdamonds.stop()
 
+    test_memcg_filter_memcg_path_staging()
+
 if __name__ == '__main__':
     main()
-- 
2.51.0

Re: [PATCH v2 1/2] selftests/damon/_damon_sysfs.py: fix memcg_path assignment

[SeongJae Park]

On Fri, 29 May 2026 17:19:48 +0800 Cheng Nie <niecheng1@uniontech.com> wrote:

> DamosFilter stores memcg_path for sysfs staging, but the constructor
> assigns it with a trailing comma and therefore turns it into a tuple.
> 
> Fix the assignment so memcg_path is stored as the intended string.
> This makes memcg filter staging and follow-up validation use the
> written path correctly.

Nice catch, thank you!

> 
> Signed-off-by: Cheng Nie <niecheng1@uniontech.com>

Reviewed-by: SeongJae Park <sj@kernel.org>


Thanks,
SJ

[...]

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

[SeongJae Park]

On Fri, 29 May 2026 17:21:31 +0800 Cheng Nie <niecheng1@uniontech.com> wrote:

> Add a dedicated test at the end of main() that writes memcg_path to
> sysfs and checks the readback path.  Validate the readback rather than
> a derived memcg_id so the test stays focused on DAMON sysfs behavior
> and avoids depending on the local userspace cgroup mount layout.

Nice addition of a test, thank you!

> 
> Signed-off-by: Cheng Nie <niecheng1@uniontech.com>
> ---
>  tools/testing/selftests/damon/sysfs.py | 39 ++++++++++++++++++++++++++
>  1 file changed, 39 insertions(+)
> 
> diff --git a/tools/testing/selftests/damon/sysfs.py b/tools/testing/selftests/damon/sysfs.py
> index aa03a1187489..0f20bc9c46b7 100755
> --- a/tools/testing/selftests/damon/sysfs.py
> +++ b/tools/testing/selftests/damon/sysfs.py
> @@ -250,6 +250,43 @@ def assert_ctxs_committed(kdamonds):
>              if ctx in ctxs_paused_for_dump:
>                  ctx.pause = False
>  
> +def test_memcg_filter_memcg_path_staging():
> +    global kdamonds
> +    memcg_filter = _damon_sysfs.DamosFilter(
> +            type_='memcg', matching=True, allow=True, memcg_path='/')
> +    kdamonds = _damon_sysfs.Kdamonds(
> +            [_damon_sysfs.Kdamond(
> +                contexts=[_damon_sysfs.DamonCtx(
> +                    targets=[_damon_sysfs.DamonTarget(pid=-1)],
> +                    schemes=[_damon_sysfs.Damos()],
> +                    )])])
> +    err = kdamonds.start()
> +    if err is not None:
> +        fail('memcg_path staging: kdamond start', {'error': err})

I'd prefer the error message to be in form of something like "'memcg_path
staging: kdamond start (%s)' % err" for consistency.

> +
> +    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()
> +    if err is not None:
> +        fail('memcg_path staging: kdamond commit', {'error': err})

It seems we don't really need to do commit().  As Sashiko mentioned [1], it
also depends on CONFIG_MEMCG and active cgroup configuration.   It would be
suffice to simply 'stage'.  We don't have stage() method on Kdamonds, though.
What about setting all parameters including the memcg filter at the beginning,
do 'kdamonds.start()', but ignore the failure, and check the content of the
memcg_path file?

[1] https://lore.kernel.org/20260529094327.6D5031F00893@smtp.kernel.org


Thanks,
SJ

[...]