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

[SeongJae Park <sj@kernel.org>]

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

[...]