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

[sashiko-bot@kernel.org]

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

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().

> +    # restore for the caller
> +    for kd in kdamonds.kdamonds:
> +        for ctx in kd.contexts:
> +            if ctx in ctxs_paused_for_dump:
> +                ctx.pause = False

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260427151231.113429-1-sj@kernel.org?part=10