Re: [RFC PATCH v2 10/10] selftets/damon/sysfs.py: pause DAMON before dumping status

SeongJae Park
2026-03-20T15:41:40Z

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 …