[PATCH 1/2] selftests/damon: fix DamosFilter memcg_path assignment

[PATCH 1/2] selftests/damon: fix DamosFilter memcg_path assignment

[niecheng]

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: niecheng <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 2b4df655d9fd..022f0d392204 100644
--- a/tools/testing/selftests/damon/_damon_sysfs.py
+++ b/tools/testing/selftests/damon/_damon_sysfs.py
@@ -254,7 +254,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 2/2] selftests/damon: validate memcg filter sysfs readback

[niecheng]

Add memcg filter validation to the DAMON sysfs selftest by checking the
memcg_path sysfs readback.

Validate the readback path rather than a derived memcg_id so that the
test stays focused on DAMON sysfs behavior and avoids depending on the
local userspace cgroup mount layout.

Also compare the readback path while stripping only the trailing
newline.

Signed-off-by: niecheng <niecheng1@uniontech.com>
---
 tools/testing/selftests/damon/sysfs.py | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/damon/sysfs.py b/tools/testing/selftests/damon/sysfs.py
index 3aa5c91548a5..b595da24f698 100755
--- a/tools/testing/selftests/damon/sysfs.py
+++ b/tools/testing/selftests/damon/sysfs.py
@@ -33,6 +33,7 @@ def assert_true(condition, expectation, status):
     if condition is not True:
         fail(expectation, status)
 
+
 def assert_watermarks_committed(watermarks, dump):
     wmark_metric_val = {
             'none': 0,
@@ -90,8 +91,14 @@ def assert_filter_committed(filter_, dump):
     assert_true(filter_.type_ == dump['type'], 'type', dump)
     assert_true(filter_.matching == dump['matching'], 'matching', dump)
     assert_true(filter_.allow == dump['allow'], 'allow', dump)
-    # TODO: check memcg_path and memcg_id if type is memcg
-    if filter_.type_ == 'addr':
+    if filter_.type_ == 'memcg':
+        shown, rd_err = _damon_sysfs.read_file(
+                os.path.join(filter_.sysfs_dir(), 'memcg_path'))
+        if rd_err is not None:
+            fail('memcg_path sysfs read', {'error': rd_err})
+        assert_true(shown.rstrip('\n') == ('%s' % filter_.memcg_path),
+                    'memcg_path_sysfs', dump)
+    elif filter_.type_ == 'addr':
         assert_true([filter_.addr_start, filter_.addr_end] ==
                     dump['addr_range'], 'addr_range', dump)
     elif filter_.type_ == 'target':
@@ -258,6 +265,8 @@ def main():
                 ops_filters=[
                     _damon_sysfs.DamosFilter(type_='anon', matching=True,
                                              allow=True),
+                    _damon_sysfs.DamosFilter(type_='memcg', matching=True,
+                                             allow=True, memcg_path='/'),
                     ],
                 )])
     context.idx = 0
-- 
2.51.0

Re: [PATCH 1/2] selftests/damon: fix DamosFilter memcg_path assignment

[SeongJae Park]

Hello nicheng,


Could you please update the subject to 'selftets/ddamon/_damon_sysfs.py'?

On Thu, 28 May 2026 16:10:38 +0800 niecheng <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.

Nice catch!

> 
> 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: niecheng <niecheng1@uniontech.com>

From the git log, I found it seems you also use 'Cheng Nie' as your name.  Are
you intentionally use 'niecheng' as your identity?  We disallow anonymous but
allow using known identity, so I'm just checking if this is an unintended
mistake.  Some of us (mm community) prefer real name, though.

> ---
>  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 2b4df655d9fd..022f0d392204 100644
> --- a/tools/testing/selftests/damon/_damon_sysfs.py
> +++ b/tools/testing/selftests/damon/_damon_sysfs.py
> @@ -254,7 +254,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


The change looks good!


Thanks,
SJ

Re: [PATCH 2/2] selftests/damon: validate memcg filter sysfs readback

[SeongJae Park]

Hello niecheng,


Could we use 'selftests/damn/sysfs.py:' as the prefix of the subject?  That
will make it consistent with other commits and let us easily know to what test
this change is made.

On Thu, 28 May 2026 16:10:39 +0800 niecheng <niecheng1@uniontech.com> wrote:

> Add memcg filter validation to the DAMON sysfs selftest by checking the
> memcg_path sysfs readback.
> 
> Validate the readback path rather than a derived memcg_id so that the
> test stays focused on DAMON sysfs behavior and avoids depending on the
> local userspace cgroup mount layout.

Nice, thank you!

> 
> Also compare the readback path while stripping only the trailing
> newline.
> 
> Signed-off-by: niecheng <niecheng1@uniontech.com>
> ---
>  tools/testing/selftests/damon/sysfs.py | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/testing/selftests/damon/sysfs.py b/tools/testing/selftests/damon/sysfs.py
> index 3aa5c91548a5..b595da24f698 100755
> --- a/tools/testing/selftests/damon/sysfs.py
> +++ b/tools/testing/selftests/damon/sysfs.py
> @@ -33,6 +33,7 @@ def assert_true(condition, expectation, status):
>      if condition is not True:
>          fail(expectation, status)
>  
> +
>  def assert_watermarks_committed(watermarks, dump):
>      wmark_metric_val = {
>              'none': 0,
> @@ -90,8 +91,14 @@ def assert_filter_committed(filter_, dump):
>      assert_true(filter_.type_ == dump['type'], 'type', dump)
>      assert_true(filter_.matching == dump['matching'], 'matching', dump)
>      assert_true(filter_.allow == dump['allow'], 'allow', dump)
> -    # TODO: check memcg_path and memcg_id if type is memcg
> -    if filter_.type_ == 'addr':
> +    if filter_.type_ == 'memcg':
> +        shown, rd_err = _damon_sysfs.read_file(
> +                os.path.join(filter_.sysfs_dir(), 'memcg_path'))
> +        if rd_err is not None:
> +            fail('memcg_path sysfs read', {'error': rd_err})
> +        assert_true(shown.rstrip('\n') == ('%s' % filter_.memcg_path),
> +                    'memcg_path_sysfs', dump)
> +    elif filter_.type_ == 'addr':
>          assert_true([filter_.addr_start, filter_.addr_end] ==
>                      dump['addr_range'], 'addr_range', dump)
>      elif filter_.type_ == 'target':

But this is in the middle of assert_filter_committed().  As the name says, this
is for validating if the user inputs are passed to DAMON internal.  I think
this is not the right place to test its 'staging' functionality.

> @@ -258,6 +265,8 @@ def main():
>                  ops_filters=[
>                      _damon_sysfs.DamosFilter(type_='anon', matching=True,
>                                               allow=True),
> +                    _damon_sysfs.DamosFilter(type_='memcg', matching=True,
> +                                             allow=True, memcg_path='/'),
>                      ],
>                  )])
>      context.idx = 0

What about adding another test case at the end of the main() function for the
validation of memcg string staging functionality?


Thanks,
SJ

[...]

Re: [PATCH 1/2] selftests/damon: fix DamosFilter memcg_path assignment

[Cheng Nie]

On Thu, 28 May 2026 07:17:17 -0700 SeongJae Park wrote:

> Could you please update the subject to 'selftets/ddamon/_damon_sysfs.py'?

Will do in v2.

> From the git log, I found it seems you also use 'Cheng Nie' as your name.
> Are you intentionally use 'niecheng' as your identity?  We disallow
> anonymous but allow using known identity, so I'm just checking if this
> is an unintended mistake.  Some of us (mm community) prefer real name,
> though.

Sorry for the confusion.  In China we usually write names with the family
name first (Nie Cheng), while the Western order is Cheng Nie.  I had
also used "niecheng" as a compact form from my email username, which
was not ideal for upstream.  I will use Cheng Nie consistently from
v2 onward.

> The change looks good!

Thank you!

Thanks,
Cheng Nie

Re: [PATCH 2/2] selftests/damon: validate memcg filter sysfs readback

[Cheng Nie]

On Thu, 28 May 2026 07:28:18 -0700 SeongJae Park wrote:

> Could we use 'selftests/damn/sysfs.py:' as the prefix of the subject?
> That will make it consistent with other commits and let us easily know
> to what test this change is made.

Will do in v2.

> But this is in the middle of assert_filter_committed().  As the name
> says, this is for validating if the user inputs are passed to DAMON
> internal.  I think this is not the right place to test its 'staging'
> functionality.

Will do.  I will remove the memcg_path readback check from
assert_filter_committed() in v2.

> What about adding another test case at the end of the main() function
> for the validation of memcg string staging functionality?

Will do.  I will add a dedicated test case at the end of main() in v2.

Thanks,
Cheng Nie