[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 2/2] selftests/damon/sysfs.py: validate memcg_path staging readback

[sashiko-bot]

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

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

[...]

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

[SeongJae Park]

On Fri, 29 May 2026 09:43:26 +0000 sashiko-bot@kernel.org wrote:

> 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.

Good finding, thanks.

> 
> 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?

We have no stage() method for Kdamonds, though.  I'm suggesting [1] to just do
'start()', but ignore the failure.  Cheng, let's keep discussion on the
original thread [1].

[1] https://lore.kernel.org/20260529150057.84753-1-sj@kernel.org


Thanks,
SJ

[...]

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

[Cheng Nie]

On Fri, 29 May 2026 08:00:56 -0700 SeongJae Park wrote:

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

Will do.

> 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?

Agreed, thanks for the suggestion.  I will drop commit() and configure the
memcg filter up front, then verify memcg_path readback after staging via
start() as you suggested.

I will send a follow-up patch with these changes.

Thanks,
Cheng Nie