Re: [PATCH v3 1/3] selftests/damon: prevent cross-context state pollution in DamonCtx

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The patch introduces a split-brain state corruption in `DamonCtx` by implementing an incomplete fix for Python mutable default arguments. While it replaces the defaults in `DamonCtx.__init__` with `None` sentinels and instantiates them at runtime (e.g., `monitoring_attrs = DamonAttrs()`), it misses the fact that `DamonAttrs.__init__` still uses a mutable default (`intervals_goal=IntervalsGoal()`).

Because `DamonAttrs()` is now called multiple times (once per `DamonCtx`), multiple `DamonAttrs` instances are created, but they all silently share the exact same `IntervalsGoal` instance. During `DamonAttrs.__init__`, the shared `IntervalsGoal` instance's `attrs` back-reference is overwritten to point to the newest `DamonAttrs` (`self.intervals_goal.attrs = self`). 

As a result, when an older `DamonCtx` calls `stage()`, its base attributes correctly route to its own sysfs directory, but its `intervals_goal` writes route to the sysfs directory of the most recently created `DamonCtx`. This is a newly introduced inconsistency, as prior to the patch, all properties incorrectly routed to the same directory together. Additionally, the patch leaves identical mutable default bugs untouched in `DamosFilters`, `DamosDests`, `Damos`, `Kdamond`, and `Kdamonds`.
--

commit 7d9280d7450287d38ab4b080317de0093ab71119
Author: Kunwu Chan <chentao@kylinos.cn>

selftests/damon: prevent cross-context state pollution in DamonCtx

DamonCtx.__init__() uses mutable default values for monitoring_attrs, targets,
and schemes. In Python these are evaluated once at function definition time,
so multiple DamonCtx instances can unintentionally share the same lists and
DamonAttrs instance. Replace the mutable defaults with None sentinels and
initialize the objects when needed.

Link: https://lore.kernel.org/20260601032314.424013-2-kunwu.chan@linux.dev

> diff --git a/tools/testing/selftests/damon/_damon_sysfs.py b/tools/testing/selftests/damon/_damon_sysfs.py
> index 8b12cc0484405..2f6f2699db256 100644
> --- a/tools/testing/selftests/damon/_damon_sysfs.py
> +++ b/tools/testing/selftests/damon/_damon_sysfs.py
> @@ -624,17 +624,23 @@ class DamonCtx:
>      pause = None
>      idx = None
>  
> -    def __init__(self, ops='paddr', monitoring_attrs=DamonAttrs(), targets=[],
> -            schemes=[], pause=False):
> +    def __init__(self, ops='paddr', monitoring_attrs=None, targets=None,
> +            schemes=None, pause=False):
>          self.ops = ops
> +        if monitoring_attrs is None:
> +            monitoring_attrs = DamonAttrs()

[Severity: Medium]
Does this incomplete fix introduce a split-brain state regression?

Because DamonAttrs() is now called multiple times (once per DamonCtx),
multiple DamonAttrs instances are created. However, DamonAttrs.__init__
still uses a mutable default for intervals_goal:

tools/testing/selftests/damon/_damon_sysfs.py:DamonAttrs.__init__() {
    def __init__(self, sample_us=5000, aggr_us=100000,
                 intervals_goal=IntervalsGoal(), update_us=1000000,
...
        self.intervals_goal.attrs = self
}

This causes all newly created DamonCtx instances to silently share the
exact same IntervalsGoal instance. During DamonAttrs.__init__, the shared
IntervalsGoal instance's attrs back-reference is overwritten to point to
the newest DamonAttrs.

As a result, when an older DamonCtx calls stage(), its base attributes
correctly route to its own sysfs directory, but its intervals_goal writes
route to the sysfs directory of the most recently created DamonCtx.

Additionally, could there be other similar mutable default bugs left
untouched, such as in Damos.__init__:

tools/testing/selftests/damon/_damon_sysfs.py:Damos.__init__() {
    def __init__(self, action='stat', access_pattern=DamosAccessPattern(),
                 quota=DamosQuota(), watermarks=DamosWatermarks(),
                 core_filters=[], ops_filters=[], filters=[], target_nid=0,
}

>          self.monitoring_attrs = monitoring_attrs
>          self.monitoring_attrs.context = self

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