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

[SeongJae Park <sj@kernel.org>]

On Mon, 27 Apr 2026 21:04:19 +0000 sashiko-bot@kernel.org wrote:

> Sashiko AI review found 1 potential issue(s):
> - [Medium] Mutating the state before assertion bypasses the verification of the original `pause` state.

Not a blocker of this patch in my opinion.  Please read below for more details.

> --
> 
> commit 2c32bc147a1cb335e85629e00bb0efe1bbb44638
> Author: SeongJae Park <sj@kernel.org>
> 
> selftests/damon/sysfs.py: pause DAMON before dumping status
> 
> This commit updates the DAMON sysfs selftest to pause DAMON execution
> during state dumping. It attempts to prevent race conditions where DAMON
> could add or remove regions while the dump is ongoing.
> 
> > diff --git a/tools/testing/selftests/damon/sysfs.py b/tools/testing/selftests/damon/sysfs.py
> > index e6d34ba05893..b8d6e0f8fd90 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()
> 
> [ ... ]
> 
> > +    # 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])
> 
> Does this bypass the verification of the original pause state?
> 
> By temporarily pausing the DAMON context and committing this new state
> before dumping the sysfs status, the test overwrites the actual pause state
> in the kernel. Then, setting the Python object's ctx.pause to True for
> comparison means the test always compares True against True for the pause
> parameter.
> 
> Because of this, the test seems fundamentally incapable of verifying
> whether a pause = False configuration was ever correctly committed to the
> kernel prior to calling assert_ctxs_committed().

This is an intended behavior as I explained in a previous version.  We are
using pause for testing.  Testing something that being used for testing with
the test itself is complicated.  It would be good to have a test for the
feature for the test itself in a way, but not necessarily by the test the
feature is helping for.  We have at least pause parameter commit kunit.  So I
think this is not a blocker of this patch.


Thanks,
SJ

[...]