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 | 38 ++++++++++++++++++++++++++
1 file changed, 38 insertions(+)
diff --git a/tools/testing/selftests/damon/sysfs.py b/tools/testing/selftests/damon/sysfs.py
index e6d34ba05893f..5f00e97f019f4 100755
--- a/tools/testing/selftests/damon/sysfs.py
+++ b/tools/testing/selftests/damon/sysfs.py
@@ -193,18 +193,55 @@ def assert_ctx_committed(ctx, dump):
assert_true(dump['pause'] == ctx.pause, 'pause', dump)
def assert_ctxs_committed(kdamonds):
+ ctxs_paused_for_dump = []
+ kdamonds_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
+ ctxs_paused_for_dump.append(ctx)
+ if not kd in kdamonds_paused_for_dump:
+ kdamonds_paused_for_dump.append(kd)
+ if kd in kdamonds_paused_for_dump:
+ 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:
print(err)
kdamonds.stop()
exit(1)
+ # resume contexts paused for safe state dumping
+ for ctx in ctxs_paused_for_dump:
+ ctx.pause = False
+ if kd in kdamonds_paused_for_dump:
+ 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']
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 +339,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
Forwarding Sashiko review for doing discussions via mails.
# review url: https://sashiko.dev/#/patchset/20260322155728.81434-11-sj@kernel.org
# start of sashiko.dev inline review
commit 67adf8e7b8416252faf583e176f0e8424fd4fb95
Author: SeongJae Park <sj@kernel.org>
selftets/damon/sysfs.py: pause DAMON before dumping status
The sysfs.py test commits DAMON parameters, dumps the internal DAMON
state, and verifies the parameters. This commit pauses DAMON execution
during the state dumping to avoid races that can result in false test
results.
> selftets/damon/sysfs.py: pause DAMON before dumping status
This isn't a bug, but there's a typo (selftets) in the subject line.
> diff --git a/tools/testing/selftests/damon/sysfs.py b/tools/testing/selftests/damon/sysfs.py
> --- a/tools/testing/selftests/damon/sysfs.py
> +++ b/tools/testing/selftests/damon/sysfs.py
[ ... ]
> @@ -193,18 +193,55 @@ def assert_ctx_committed(ctx, dump):
> def assert_ctxs_committed(kdamonds):
> + ctxs_paused_for_dump = []
> + kdamonds_paused_for_dump = []
> + # pause for safe state dumping
> + for kd in kdamonds.kdamonds:
[ ... ]
> + # resume contexts paused for safe state dumping
> + for ctx in ctxs_paused_for_dump:
> + ctx.pause = False
> + if kd in kdamonds_paused_for_dump:
> + err = kd.commit()
Is this if statement intended to be a for loop?
Since kd is just retaining the value from the last iteration of the earlier
loop, using an if statement here means that if there are multiple kdamonds,
only the last one might have its state committed.
Could this leave other kdamonds un-resumed in the kernel?
> + if err is not None:
> + print('resume fail (%s)' % err)
> + kdamonds.stop()
> + exit(1)
[ ... ]
# end of sashiko.dev inline review
# review url: https://sashiko.dev/#/patchset/20260322155728.81434-11-sj@kernel.org
#
# hkml [1] generated a draft of this mail. It can be regenerated
# using below command:
#
# hkml patch sashiko_dev --for_forwarding \
# 20260322155728.81434-11-sj@kernel.org
#
# [1] https://github.com/sjp38/hackermail
Sent using hkml (https://github.com/sjp38/hackermail)
On Sun, 22 Mar 2026 10:15:33 -0700 SeongJae Park <sj@kernel.org> wrote:
> Forwarding Sashiko review for doing discussions via mails.
>
> # review url: https://sashiko.dev/#/patchset/20260322155728.81434-11-sj@kernel.org
> # start of sashiko.dev inline review
> commit 67adf8e7b8416252faf583e176f0e8424fd4fb95
> Author: SeongJae Park <sj@kernel.org>
>
> selftets/damon/sysfs.py: pause DAMON before dumping status
>
> The sysfs.py test commits DAMON parameters, dumps the internal DAMON
> state, and verifies the parameters. This commit pauses DAMON execution
> during the state dumping to avoid races that can result in false test
> results.
>
> > selftets/damon/sysfs.py: pause DAMON before dumping status
>
> This isn't a bug, but there's a typo (selftets) in the subject line.
Oops, Sashiko mentioned this in the previous one, but I forgot fixing this.
I will fix this in the next spin.
>
> > diff --git a/tools/testing/selftests/damon/sysfs.py b/tools/testing/selftests/damon/sysfs.py
> > --- a/tools/testing/selftests/damon/sysfs.py
> > +++ b/tools/testing/selftests/damon/sysfs.py
> [ ... ]
> > @@ -193,18 +193,55 @@ def assert_ctx_committed(ctx, dump):
> > def assert_ctxs_committed(kdamonds):
> > + ctxs_paused_for_dump = []
> > + kdamonds_paused_for_dump = []
> > + # pause for safe state dumping
> > + for kd in kdamonds.kdamonds:
> [ ... ]
> > + # resume contexts paused for safe state dumping
> > + for ctx in ctxs_paused_for_dump:
> > + ctx.pause = False
> > + if kd in kdamonds_paused_for_dump:
> > + err = kd.commit()
>
> Is this if statement intended to be a for loop?
>
> Since kd is just retaining the value from the last iteration of the earlier
> loop, using an if statement here means that if there are multiple kdamonds,
> only the last one might have its state committed.
>
> Could this leave other kdamonds un-resumed in the kernel?
Ah, correct... There is no multiple kdamonds use case, but let's make it
complete. I will fix this in the next spin, like below.
'''
--- a/tools/testing/selftests/damon/sysfs.py
+++ b/tools/testing/selftests/damon/sysfs.py
@@ -226,7 +226,7 @@ def assert_ctxs_committed(kdamonds):
# resume contexts paused for safe state dumping
for ctx in ctxs_paused_for_dump:
ctx.pause = False
- if kd in kdamonds_paused_for_dump:
+ for kd in kdamonds_paused_for_dump:
err = kd.commit()
if err is not None:
print('resume fail (%s)' % err)
'''
Thanks,
SJ
[...]
© 2016 - 2026 Red Hat, Inc.