Re: (sashiko review) [RFC PATCH v4 10/10] selftets/damon/sysfs.py: pause DAMON before dumping status

SeongJae Park
2026-03-22T17:48:01Z

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 …