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 <[email protected]> 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 <[email protected]> > --- > 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 [...]
