The sysfs.py test commits DAMON parameters, dump the internal DAMON
state, and show if the parameters are committed as expected using the
dumped state.  While the dumping is ongoing, DAMON is alive.  It can
make internal changes including addition and removal of regions.  It can
therefore make a race that can result in false test results.  Pause
DAMON execution during the state dumping to avoid such races.

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

diff --git a/tools/testing/selftests/damon/sysfs.py b/tools/testing/selftests/damon/sysfs.py
index e6d34ba05893f..704729c7a318e 100755
--- a/tools/testing/selftests/damon/sysfs.py
+++ b/tools/testing/selftests/damon/sysfs.py
@@ -193,18 +193,48 @@ def assert_ctx_committed(ctx, dump):
     assert_true(dump['pause'] == ctx.pause, 'pause', dump)
 
 def assert_ctxs_committed(kdamonds):
+    ctxs_paused_for_dump = []
+    for kd in kdamonds.kdamonds:
+        for ctx in kd.contexts:
+            if ctx.pause is False:
+                ctx.pause = True
+                err = kd.commit()
+                if err is not None:
+                    print('pause fail (%s)' % err)
+                    kdamonds.stop()
+                    exit(1)
+                ctxs_paused_for_dump.append(ctx)
+
     status, err = dump_damon_status_dict(kdamonds.kdamonds[0].pid)
     if err is not None:
         print(err)
         kdamonds.stop()
         exit(1)
 
+    for kd in kdamonds.kdamonds:
+        for ctx in kd.contexts:
+            if ctx in ctxs_paused_for_dump:
+                ctx.pause = False
+                err = kd.commit()
+                if err is not None:
+                    print('resume fail (%s)' % err)
+                    kdamonds.stop()
+                    exit(1)
+                # restore for comparison
+                ctx.pause = True
+
     ctxs = kdamonds.kdamonds[0].contexts
     dump = status['contexts']
     assert_true(len(ctxs) == len(dump), 'ctxs length', dump)
     for idx, ctx in enumerate(ctxs):
         assert_ctx_committed(ctx, dump[idx])
 
+    # restore for the caller
+    for kd in kdamonds.kdamonds:
+        for ctx in kd.contexts:
+            if ctx in ctxs_paused_for_dump:
+                ctx.pause = False
+
 def main():
     kdamonds = _damon_sysfs.Kdamonds(
             [_damon_sysfs.Kdamond(
@@ -302,6 +332,7 @@ def main():
         print('kdamond start failed: %s' % err)
         exit(1)
     kdamonds.kdamonds[0].contexts[0].targets[1].obsolete = True
+    kdamonds.kdamonds[0].contexts[0].pause = True
     kdamonds.kdamonds[0].commit()
     del kdamonds.kdamonds[0].contexts[0].targets[1]
     assert_ctxs_committed(kdamonds)
-- 
2.47.3

Adding Sashiko comment
(https://lore.kernel.org/https://sashiko.dev/#/patchset/20260319052157.99433-11-sj@kernel.org)
with ': ' line prefix, and my replies in line.

On Wed, 18 Mar 2026 22:21:53 -0700 SeongJae Park <sj@kernel.org> wrote:

> The sysfs.py test commits DAMON parameters, dump the internal DAMON
> state, and show if the parameters are committed as expected using the
> dumped state.  While the dumping is ongoing, DAMON is alive.  It can
> make internal changes including addition and removal of regions.  It can
> therefore make a race that can result in false test results.  Pause
> DAMON execution during the state dumping to avoid such races.
> 
> Signed-off-by: SeongJae Park <sj@kernel.org>
> ---
>  tools/testing/selftests/damon/sysfs.py | 31 ++++++++++++++++++++++++++
>  1 file changed, 31 insertions(+)
> 
> diff --git a/tools/testing/selftests/damon/sysfs.py b/tools/testing/selftests/damon/sysfs.py
> index e6d34ba05893f..704729c7a318e 100755
> --- a/tools/testing/selftests/damon/sysfs.py
> +++ b/tools/testing/selftests/damon/sysfs.py
> @@ -193,18 +193,48 @@ def assert_ctx_committed(ctx, dump):
>      assert_true(dump['pause'] == ctx.pause, 'pause', dump)
>  
>  def assert_ctxs_committed(kdamonds):
> +    ctxs_paused_for_dump = []
> +    for kd in kdamonds.kdamonds:
> +        for ctx in kd.contexts:
> +            if ctx.pause is False:
> +                ctx.pause = True
> +                err = kd.commit()
> +                if err is not None:
> +                    print('pause fail (%s)' % err)
> +                    kdamonds.stop()
> +                    exit(1)
> +                ctxs_paused_for_dump.append(ctx)
> +
>      status, err = dump_damon_status_dict(kdamonds.kdamonds[0].pid)
>      if err is not None:
>          print(err)
>          kdamonds.stop()
>          exit(1)
>  
> +    for kd in kdamonds.kdamonds:
> +        for ctx in kd.contexts:
> +            if ctx in ctxs_paused_for_dump:
> +                ctx.pause = False
> +                err = kd.commit()
> +                if err is not None:
> +                    print('resume fail (%s)' % err)
> +                    kdamonds.stop()
> +                    exit(1)
> +                # restore for comparison
> +                ctx.pause = True

: If a kdamond contains multiple contexts, does this loop leave earlier contexts
: paused in the kernel?
: 
: Since kd.commit() stages and commits the state of all contexts associated with
: the kdamond, when kd.commit() is called for the second context, the first
: context's local pause attribute is already back to True.
: 
: This would cause kd.commit() to write to sysfs and instruct the kernel to
: pause the first context again. By the end of this loop, only the last context
: in the kdamond would remain unpaused in the kernel.

No.  The pause field of the earlier context is set to False, so later
kd.commit() will commit the False 'pause' again.  But this finds a good point.
There is no reason to call kd.commit() for each context.  It is more efficient
to be called for each kdamond., thouth currently we support only one context
per kdamond.  I will update the code so, in the next spin.

> +
>      ctxs = kdamonds.kdamonds[0].contexts
>      dump = status['contexts']
>      assert_true(len(ctxs) == len(dump), 'ctxs length', dump)
>      for idx, ctx in enumerate(ctxs):
>          assert_ctx_committed(ctx, dump[idx])
>  
> +    # restore for the caller
> +    for kd in kdamonds.kdamonds:
> +        for ctx in kd.contexts:
> +            if ctx in ctxs_paused_for_dump:
> +                ctx.pause = False

: Since kd.commit() is not called after restoring the Python objects here, does
: this leave the previous contexts permanently paused in the kernel while their
: Python state reflects them as running?

No, we already unpaused the unpause-required contexts above.


Thanks,
SJ

[...]

On Fri, 20 Mar 2026 08:29:39 -0700 SeongJae Park <sj@kernel.org> wrote:

> Adding Sashiko comment
> (https://lore.kernel.org/https://sashiko.dev/#/patchset/20260319052157.99433-11-sj@kernel.org)
> with ': ' line prefix, and my replies in line.
> 
> On Wed, 18 Mar 2026 22:21:53 -0700 SeongJae Park <sj@kernel.org> wrote:
> 
> > The sysfs.py test commits DAMON parameters, dump the internal DAMON
> > state, and show if the parameters are committed as expected using the
> > dumped state.  While the dumping is ongoing, DAMON is alive.  It can
> > make internal changes including addition and removal of regions.  It can
> > therefore make a race that can result in false test results.  Pause
> > DAMON execution during the state dumping to avoid such races.
> > 
> > Signed-off-by: SeongJae Park <sj@kernel.org>
> > ---
> >  tools/testing/selftests/damon/sysfs.py | 31 ++++++++++++++++++++++++++
> >  1 file changed, 31 insertions(+)
> > 
> > diff --git a/tools/testing/selftests/damon/sysfs.py b/tools/testing/selftests/damon/sysfs.py
> > index e6d34ba05893f..704729c7a318e 100755
> > --- a/tools/testing/selftests/damon/sysfs.py
> > +++ b/tools/testing/selftests/damon/sysfs.py
> > @@ -193,18 +193,48 @@ def assert_ctx_committed(ctx, dump):
> >      assert_true(dump['pause'] == ctx.pause, 'pause', dump)
> >  
> >  def assert_ctxs_committed(kdamonds):
> > +    ctxs_paused_for_dump = []
> > +    for kd in kdamonds.kdamonds:
> > +        for ctx in kd.contexts:
> > +            if ctx.pause is False:
> > +                ctx.pause = True
> > +                err = kd.commit()
> > +                if err is not None:
> > +                    print('pause fail (%s)' % err)
> > +                    kdamonds.stop()
> > +                    exit(1)
> > +                ctxs_paused_for_dump.append(ctx)
> > +
> >      status, err = dump_damon_status_dict(kdamonds.kdamonds[0].pid)
> >      if err is not None:
> >          print(err)
> >          kdamonds.stop()
> >          exit(1)
> >  
> > +    for kd in kdamonds.kdamonds:
> > +        for ctx in kd.contexts:
> > +            if ctx in ctxs_paused_for_dump:
> > +                ctx.pause = False
> > +                err = kd.commit()
> > +                if err is not None:
> > +                    print('resume fail (%s)' % err)
> > +                    kdamonds.stop()
> > +                    exit(1)
> > +                # restore for comparison
> > +                ctx.pause = True
> 
> : If a kdamond contains multiple contexts, does this loop leave earlier contexts
> : paused in the kernel?
> : 
> : Since kd.commit() stages and commits the state of all contexts associated with
> : the kdamond, when kd.commit() is called for the second context, the first
> : context's local pause attribute is already back to True.
> : 
> : This would cause kd.commit() to write to sysfs and instruct the kernel to
> : pause the first context again. By the end of this loop, only the last context
> : in the kdamond would remain unpaused in the kernel.
> 
> No.  The pause field of the earlier context is set to False, so later
> kd.commit() will commit the False 'pause' again.  But this finds a good point.
> There is no reason to call kd.commit() for each context.  It is more efficient
> to be called for each kdamond., thouth currently we support only one context
> per kdamond.  I will update the code so, in the next spin.

I'm wrong, Sashiko is correct.  I missed the last 'ctx.pause = True' in the
loop.  I will fix this together, like below:

'''
--- a/tools/testing/selftests/damon/sysfs.py
+++ b/tools/testing/selftests/damon/sysfs.py
@@ -201,16 +201,17 @@ def assert_ctx_committed(ctx, dump):

 def assert_ctxs_committed(kdamonds):
     ctxs_paused_for_dump = []
+    # pause for safe state dumping
     for kd in kdamonds.kdamonds:
         for ctx in kd.contexts:
             if ctx.pause is False:
                 ctx.pause = True
-                err = kd.commit()
-                if err is not None:
-                    print('pause fail (%s)' % err)
-                    kdamonds.stop()
-                    exit(1)
                 ctxs_paused_for_dump.append(ctx)
+        err = kd.commit()
+        if err is not None:
+            print('pause fail (%s)' % err)
+            kdamonds.stop()
+            exit(1)

     status, err = dump_damon_status_dict(kdamonds.kdamonds[0].pid)
     if err is not None:
@@ -218,17 +219,19 @@ def assert_ctxs_committed(kdamonds):
         kdamonds.stop()
         exit(1)

+    # resume contexts paused for safe state dumping
     for kd in kdamonds.kdamonds:
-        for ctx in kd.contexts:
-            if ctx in ctxs_paused_for_dump:
-                ctx.pause = False
-                err = kd.commit()
-                if err is not None:
-                    print('resume fail (%s)' % err)
-                    kdamonds.stop()
-                    exit(1)
-                # restore for comparison
-                ctx.pause = True
+        for ctx in ctxs_paused_for_dump:
+            ctx.pause = False
+        err = kd.commit()
+        if err is not None:
+            print('resume fail (%s)' % err)
+            kdamonds.stop()
+            exit(1)
+
+    # restore for comparison
+    for ctx in ctxs_paused_for_dump:
+        ctx.pause = True

     ctxs = kdamonds.kdamonds[0].contexts
     dump = status['contexts']
'''


Thanks,
SJ

[...]

On Fri, 20 Mar 2026 08:41:33 -0700 SeongJae Park <sj@kernel.org> wrote:

> On Fri, 20 Mar 2026 08:29:39 -0700 SeongJae Park <sj@kernel.org> wrote:
> 
> > Adding Sashiko comment
> > (https://lore.kernel.org/https://sashiko.dev/#/patchset/20260319052157.99433-11-sj@kernel.org)
> > with ': ' line prefix, and my replies in line.
> > 
> > On Wed, 18 Mar 2026 22:21:53 -0700 SeongJae Park <sj@kernel.org> wrote:
> > 
> > > The sysfs.py test commits DAMON parameters, dump the internal DAMON
> > > state, and show if the parameters are committed as expected using the
> > > dumped state.  While the dumping is ongoing, DAMON is alive.  It can
> > > make internal changes including addition and removal of regions.  It can
> > > therefore make a race that can result in false test results.  Pause
> > > DAMON execution during the state dumping to avoid such races.
> > > 
> > > Signed-off-by: SeongJae Park <sj@kernel.org>
> > > ---
> > >  tools/testing/selftests/damon/sysfs.py | 31 ++++++++++++++++++++++++++
> > >  1 file changed, 31 insertions(+)
> > > 
> > > diff --git a/tools/testing/selftests/damon/sysfs.py b/tools/testing/selftests/damon/sysfs.py
> > > index e6d34ba05893f..704729c7a318e 100755
> > > --- a/tools/testing/selftests/damon/sysfs.py
> > > +++ b/tools/testing/selftests/damon/sysfs.py
> > > @@ -193,18 +193,48 @@ def assert_ctx_committed(ctx, dump):
> > >      assert_true(dump['pause'] == ctx.pause, 'pause', dump)
> > >  
> > >  def assert_ctxs_committed(kdamonds):
> > > +    ctxs_paused_for_dump = []
> > > +    for kd in kdamonds.kdamonds:
> > > +        for ctx in kd.contexts:
> > > +            if ctx.pause is False:
> > > +                ctx.pause = True
> > > +                err = kd.commit()
> > > +                if err is not None:
> > > +                    print('pause fail (%s)' % err)
> > > +                    kdamonds.stop()
> > > +                    exit(1)
> > > +                ctxs_paused_for_dump.append(ctx)
> > > +
> > >      status, err = dump_damon_status_dict(kdamonds.kdamonds[0].pid)
> > >      if err is not None:
> > >          print(err)
> > >          kdamonds.stop()
> > >          exit(1)
> > >  
> > > +    for kd in kdamonds.kdamonds:
> > > +        for ctx in kd.contexts:
> > > +            if ctx in ctxs_paused_for_dump:
> > > +                ctx.pause = False
> > > +                err = kd.commit()
> > > +                if err is not None:
> > > +                    print('resume fail (%s)' % err)
> > > +                    kdamonds.stop()
> > > +                    exit(1)
> > > +                # restore for comparison
> > > +                ctx.pause = True
> > 
> > : If a kdamond contains multiple contexts, does this loop leave earlier contexts
> > : paused in the kernel?
> > : 
> > : Since kd.commit() stages and commits the state of all contexts associated with
> > : the kdamond, when kd.commit() is called for the second context, the first
> > : context's local pause attribute is already back to True.
> > : 
> > : This would cause kd.commit() to write to sysfs and instruct the kernel to
> > : pause the first context again. By the end of this loop, only the last context
> > : in the kdamond would remain unpaused in the kernel.
> > 
> > No.  The pause field of the earlier context is set to False, so later
> > kd.commit() will commit the False 'pause' again.  But this finds a good point.
> > There is no reason to call kd.commit() for each context.  It is more efficient
> > to be called for each kdamond., thouth currently we support only one context
> > per kdamond.  I will update the code so, in the next spin.
> 
> I'm wrong, Sashiko is correct.  I missed the last 'ctx.pause = True' in the
> loop.  I will fix this together, like below:
> 
> '''
> --- a/tools/testing/selftests/damon/sysfs.py
> +++ b/tools/testing/selftests/damon/sysfs.py
> @@ -201,16 +201,17 @@ def assert_ctx_committed(ctx, dump):
> 
>  def assert_ctxs_committed(kdamonds):
>      ctxs_paused_for_dump = []
> +    # pause for safe state dumping
>      for kd in kdamonds.kdamonds:
>          for ctx in kd.contexts:
>              if ctx.pause is False:
>                  ctx.pause = True
> -                err = kd.commit()
> -                if err is not None:
> -                    print('pause fail (%s)' % err)
> -                    kdamonds.stop()
> -                    exit(1)
>                  ctxs_paused_for_dump.append(ctx)
> +        err = kd.commit()
> +        if err is not None:
> +            print('pause fail (%s)' % err)
> +            kdamonds.stop()
> +            exit(1)
> 
>      status, err = dump_damon_status_dict(kdamonds.kdamonds[0].pid)
>      if err is not None:
> @@ -218,17 +219,19 @@ def assert_ctxs_committed(kdamonds):
>          kdamonds.stop()
>          exit(1)
> 
> +    # resume contexts paused for safe state dumping
>      for kd in kdamonds.kdamonds:
> -        for ctx in kd.contexts:
> -            if ctx in ctxs_paused_for_dump:
> -                ctx.pause = False
> -                err = kd.commit()
> -                if err is not None:
> -                    print('resume fail (%s)' % err)
> -                    kdamonds.stop()
> -                    exit(1)
> -                # restore for comparison
> -                ctx.pause = True
> +        for ctx in ctxs_paused_for_dump:
> +            ctx.pause = False
> +        err = kd.commit()
> +        if err is not None:
> +            print('resume fail (%s)' % err)
> +            kdamonds.stop()
> +            exit(1)
> +
> +    # restore for comparison
> +    for ctx in ctxs_paused_for_dump:
> +        ctx.pause = True
> 
>      ctxs = kdamonds.kdamonds[0].contexts
>      dump = status['contexts']
> '''

The above fixup makes the test fails, for the obsolete targets.  The fixup
needs another fixup, like below.  I will add that to the next spin.

'''
--- a/tools/testing/selftests/damon/sysfs.py
+++ b/tools/testing/selftests/damon/sysfs.py
@@ -200,11 +200,12 @@ def assert_ctxs_committed(kdamonds):
             if ctx.pause is False:
                 ctx.pause = True
                 ctxs_paused_for_dump.append(ctx)
-        err = kd.commit()
-        if err is not None:
-            print('pause fail (%s)' % err)
-            kdamonds.stop()
-            exit(1)
+        if len(ctxs_paused_for_dump) > 0:
+            err = kd.commit()
+            if err is not None:
+                print('pause fail (%s)' % err)
+                kdamonds.stop()
+                exit(1)

     status, err = dump_damon_status_dict(kdamonds.kdamonds[0].pid)
     if err is not None:
@@ -216,11 +217,12 @@ def assert_ctxs_committed(kdamonds):
     for kd in kdamonds.kdamonds:
         for ctx in ctxs_paused_for_dump:
             ctx.pause = False
-        err = kd.commit()
-        if err is not None:
-            print('resume fail (%s)' % err)
-            kdamonds.stop()
-            exit(1)
+        if len(ctxs_paused_for_dump) > 0:
+            err = kd.commit()
+            if err is not None:
+                print('resume fail (%s)' % err)
+                kdamonds.stop()
+                exit(1)

     # restore for comparison
     for ctx in ctxs_paused_for_dump:
'''


Thanks,
SJ