[RFC PATCH v5 07/10] selftests/damon/_damon_sysfs: support pause file staging

SeongJae Park posted 10 patches 1 week, 3 days ago
[RFC PATCH v5 07/10] selftests/damon/_damon_sysfs: support pause file staging
Posted by SeongJae Park 1 week, 3 days ago
DAMON test-purpose sysfs interface control Python module, _damon_sysfs,
is not supporting the newly added pause file.  Add the support of the
file, for future test and use of the feature.

Signed-off-by: SeongJae Park <sj@kernel.org>
---
 tools/testing/selftests/damon/_damon_sysfs.py | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/damon/_damon_sysfs.py b/tools/testing/selftests/damon/_damon_sysfs.py
index 2b4df655d9fd0..120b96ecbd741 100644
--- a/tools/testing/selftests/damon/_damon_sysfs.py
+++ b/tools/testing/selftests/damon/_damon_sysfs.py
@@ -604,10 +604,11 @@ class DamonCtx:
     targets = None
     schemes = None
     kdamond = None
+    pause = None
     idx = None
 
     def __init__(self, ops='paddr', monitoring_attrs=DamonAttrs(), targets=[],
-            schemes=[]):
+            schemes=[], pause=False):
         self.ops = ops
         self.monitoring_attrs = monitoring_attrs
         self.monitoring_attrs.context = self
@@ -622,6 +623,8 @@ class DamonCtx:
             scheme.idx = idx
             scheme.context = self
 
+        self.pause=pause
+
     def sysfs_dir(self):
         return os.path.join(self.kdamond.sysfs_dir(), 'contexts',
                 '%d' % self.idx)
@@ -662,6 +665,11 @@ class DamonCtx:
             err = scheme.stage()
             if err is not None:
                 return err
+
+        err = write_file(os.path.join(self.sysfs_dir(), 'pause'), self.pause)
+        if err is not None:
+            return err
+
         return None
 
 class Kdamond:
-- 
2.47.3
Re: (sashiko review) [RFC PATCH v5 07/10] selftests/damon/_damon_sysfs: support pause file staging
Posted by SeongJae Park 1 week, 3 days ago
Forwarding Sashiko review for doing discussions via mails.

# review url: https://sashiko.dev/#/patchset/20260323231538.84452-8-sj@kernel.org
# start of sashiko.dev inline review
commit d9ca6c4a33fc4d613982730edda6f4b67ae30a67
Author: SeongJae Park <sj@kernel.org>
selftests/damon/_damon_sysfs: support pause file staging

This commit adds support for the newly added pause file to the DAMON
test-purpose sysfs interface control Python module.

> diff --git a/tools/testing/selftests/damon/_damon_sysfs.py b/tools/testing/selftests/damon/_damon_sysfs.py
> --- a/tools/testing/selftests/damon/_damon_sysfs.py
> +++ b/tools/testing/selftests/damon/_damon_sysfs.py
[ ... ]
>  class DamonCtx:
>      targets = None
>      schemes = None
>      kdamond = None
> +    pause = None
>      idx = None
>  
>      def __init__(self, ops='paddr', monitoring_attrs=DamonAttrs(), targets=[],
> -            schemes=[]):
> +            schemes=[], pause=False):

This isn't a bug introduced by this patch, but since the function signature
is being modified here, is it a good time to address the mutable default
arguments?

In Python, default arguments are evaluated exactly once at function
definition time. This means any instances of DamonCtx created without
explicitly providing targets, schemes, or monitoring_attrs will share
the same list structures and DamonAttrs instance in memory.

For example, self.monitoring_attrs.context = self overwrites the context
attribute of the shared DamonAttrs object for all default contexts.

Would it be better to change these defaults to None and instantiate them
within the method body to avoid unexpected state sharing across instances?

>          self.ops = ops
>          self.monitoring_attrs = monitoring_attrs
>          self.monitoring_attrs.context = self

# end of sashiko.dev inline review
# review url: https://sashiko.dev/#/patchset/20260323231538.84452-8-sj@kernel.org
#
# hkml [1] generated a draft of this mail.  It can be regenerated
# using below command:
#
#     hkml patch sashiko_dev --for_forwarding \
#             20260323231538.84452-8-sj@kernel.org
#
# [1] https://github.com/sjp38/hackermail

Sent using hkml (https://github.com/sjp38/hackermail)
Re: (sashiko review) [RFC PATCH v5 07/10] selftests/damon/_damon_sysfs: support pause file staging
Posted by SeongJae Park 1 week, 3 days ago
On Mon, 23 Mar 2026 18:28:24 -0700 SeongJae Park <sj@kernel.org> wrote:

> Forwarding Sashiko review for doing discussions via mails.
> 
> # review url: https://sashiko.dev/#/patchset/20260323231538.84452-8-sj@kernel.org
> # start of sashiko.dev inline review
> commit d9ca6c4a33fc4d613982730edda6f4b67ae30a67
> Author: SeongJae Park <sj@kernel.org>
> selftests/damon/_damon_sysfs: support pause file staging
> 
> This commit adds support for the newly added pause file to the DAMON
> test-purpose sysfs interface control Python module.
> 
> > diff --git a/tools/testing/selftests/damon/_damon_sysfs.py b/tools/testing/selftests/damon/_damon_sysfs.py
> > --- a/tools/testing/selftests/damon/_damon_sysfs.py
> > +++ b/tools/testing/selftests/damon/_damon_sysfs.py
> [ ... ]
> >  class DamonCtx:
> >      targets = None
> >      schemes = None
> >      kdamond = None
> > +    pause = None
> >      idx = None
> >  
> >      def __init__(self, ops='paddr', monitoring_attrs=DamonAttrs(), targets=[],
> > -            schemes=[]):
> > +            schemes=[], pause=False):
> 
> This isn't a bug introduced by this patch, but since the function signature
> is being modified here, is it a good time to address the mutable default
> arguments?
> 
> In Python, default arguments are evaluated exactly once at function
> definition time. This means any instances of DamonCtx created without
> explicitly providing targets, schemes, or monitoring_attrs will share
> the same list structures and DamonAttrs instance in memory.
> 
> For example, self.monitoring_attrs.context = self overwrites the context
> attribute of the shared DamonAttrs object for all default contexts.
> 
> Would it be better to change these defaults to None and instantiate them
> within the method body to avoid unexpected state sharing across instances?

I agree changing the defaults to None is a good idea.  But that would better to
be another patch, not this one.


Thanks,
SJ

[...]