DAMON development mailing list
 help / color / mirror / Atom feed
* [PATCH v3 0/3] selftests/damon: misc fixes for test bugs
@ 2026-06-29 14:46 SJ Park
  2026-06-29 14:46 ` [PATCH v3 1/3] selftests/damon: prevent cross-context state pollution in DamonCtx SJ Park
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: SJ Park @ 2026-06-29 14:46 UTC (permalink / raw)
  To: Andrew Morton
  Cc: SJ Park, Kunwu Chan, Shuah Khan, Wang Lian, damon, linux-kernel,
	linux-kselftest, linux-mm

This series fixes several bugs in the DAMON selftests.  Most are trivial
but makes test output wrong or even silently pass the one test case for
'avail_operation' file existence check.

Patch 1 fixes mutable default arguments in DamonCtx.__init__()
that cause state to leak between test instances.

Patch 2 fixes wrong operator precedence and join TypeError in
damos_tried_regions.py.

Patch 3 fixes several wrong strings that produce dead elif
branches, skipped file existence checks, and broken dict key
lookups.

Changes from v2:
- v2: https://lore.kernel.org/20260601032314.424013-1-kunwu.chan@linux.dev
- Rebase to latest mm-new.
Changes from v1:
- v1: https://lore.kernel.org/20260531085633.48626-1-kunwu.chan@linux.dev
- Fix From:/Signed-off-by mismatch.
- Add Reviewed-by tags from SJ Park.
- Drop the duplicate memcg_path fix.
- Drop the empty aggregation-cycle retry fix pending root-cause
  investigation.
- No code change

Kunwu Chan (3):
  selftests/damon: prevent cross-context state pollution in DamonCtx
  selftests/damon/damos_tried_regions: fix expectation output and join
    TypeError
  selftests/damon: fix dead code, skipped checks, and broken lookups

 tools/testing/selftests/damon/_damon_sysfs.py        | 12 +++++++++---
 .../testing/selftests/damon/damos_apply_interval.py  |  2 +-
 tools/testing/selftests/damon/damos_quota_goal.py    |  2 +-
 tools/testing/selftests/damon/damos_tried_regions.py |  4 ++--
 .../selftests/damon/drgn_dump_damon_status.py        |  2 +-
 tools/testing/selftests/damon/sysfs.py               |  4 ++--
 tools/testing/selftests/damon/sysfs.sh               |  6 +++---
 ...fs_update_schemes_tried_regions_wss_estimation.py |  2 +-
 8 files changed, 20 insertions(+), 14 deletions(-)


base-commit: 1f7264da0cec89427b167cc51930dd8c169dade4
-- 
2.47.3

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v3 1/3] selftests/damon: prevent cross-context state pollution in DamonCtx
  2026-06-29 14:46 [PATCH v3 0/3] selftests/damon: misc fixes for test bugs SJ Park
@ 2026-06-29 14:46 ` SJ Park
  2026-06-29 14:55   ` sashiko-bot
  2026-06-29 14:46 ` [PATCH v3 2/3] selftests/damon/damos_tried_regions: fix expectation output and join TypeError SJ Park
  2026-06-29 14:46 ` [PATCH v3 3/3] selftests/damon: fix dead code, skipped checks, and broken lookups SJ Park
  2 siblings, 1 reply; 6+ messages in thread
From: SJ Park @ 2026-06-29 14:46 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Kunwu Chan, SJ Park, Shuah Khan, Wang Lian, damon, linux-kernel,
	linux-kselftest, linux-mm

From: Kunwu Chan <chentao@kylinos.cn>

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
Co-developed-by: Wang Lian <lianux.mm@gmail.com>
Signed-off-by: Wang Lian <lianux.mm@gmail.com>
Signed-off-by: Kunwu Chan <chentao@kylinos.cn>
Reviewed-by: SJ Park <sj@kernel.org>
Cc: Kunwu Chan <chentao@kylinos.cn>
Cc: Wang Lian <lianux.mm@gmail.com>
Signed-off-by: SJ Park <sj@kernel.org>
---
 tools/testing/selftests/damon/_damon_sysfs.py | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

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()
         self.monitoring_attrs = monitoring_attrs
         self.monitoring_attrs.context = self
 
+        if targets is None:
+            targets = []
         self.targets = targets
         for idx, target in enumerate(self.targets):
             target.idx = idx
             target.context = self
 
+        if schemes is None:
+            schemes = []
         self.schemes = schemes
         for idx, scheme in enumerate(self.schemes):
             scheme.idx = idx
-- 
2.47.3

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v3 2/3] selftests/damon/damos_tried_regions: fix expectation output and join TypeError
  2026-06-29 14:46 [PATCH v3 0/3] selftests/damon: misc fixes for test bugs SJ Park
  2026-06-29 14:46 ` [PATCH v3 1/3] selftests/damon: prevent cross-context state pollution in DamonCtx SJ Park
@ 2026-06-29 14:46 ` SJ Park
  2026-06-29 14:46 ` [PATCH v3 3/3] selftests/damon: fix dead code, skipped checks, and broken lookups SJ Park
  2 siblings, 0 replies; 6+ messages in thread
From: SJ Park @ 2026-06-29 14:46 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Kunwu Chan, SJ Park, Shuah Khan, Wang Lian, damon, linux-kernel,
	linux-kselftest, linux-mm

From: Kunwu Chan <chentao@kylinos.cn>

The expectation print has wrong operator precedence: '%' binds
before the conditional expression, so the else branch prints
'not met' without the prefix 'expectation (>= 14) is'.  Add
parentheses to fix it.

Also, '\n'.join() on the list of ints raises TypeError;
convert to str in the list comprehension.

Link: https://lore.kernel.org/20260601032314.424013-3-kunwu.chan@linux.dev
Co-developed-by: Wang Lian <lianux.mm@gmail.com>
Signed-off-by: Wang Lian <lianux.mm@gmail.com>
Signed-off-by: Kunwu Chan <chentao@kylinos.cn>
Reviewed-by: SJ Park <sj@kernel.org>
Cc: Kunwu Chan <chentao@kylinos.cn>
Cc: Wang Lian <lianux.mm@gmail.com>
Signed-off-by: SJ Park <sj@kernel.org>
---
 tools/testing/selftests/damon/damos_tried_regions.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/damon/damos_tried_regions.py b/tools/testing/selftests/damon/damos_tried_regions.py
index 3b347eb28bd28..d6472e6a6e082 100755
--- a/tools/testing/selftests/damon/damos_tried_regions.py
+++ b/tools/testing/selftests/damon/damos_tried_regions.py
@@ -55,10 +55,10 @@ def main():
     collected_nr_regions.sort()
     sample = collected_nr_regions[4]
     print('50-th percentile nr_regions: %d' % sample)
-    print('expectation (>= 14) is %s' % 'met' if sample >= 14 else 'not met')
+    print('expectation (>= 14) is %s' % ('met' if sample >= 14 else 'not met'))
     if collected_nr_regions[4] < 14:
         print('full nr_regions:')
-        print('\n'.join(collected_nr_regions))
+        print('\n'.join(['%d' % x for x in collected_nr_regions]))
         exit(1)
 
 if __name__ == '__main__':
-- 
2.47.3

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v3 3/3] selftests/damon: fix dead code, skipped checks, and broken lookups
  2026-06-29 14:46 [PATCH v3 0/3] selftests/damon: misc fixes for test bugs SJ Park
  2026-06-29 14:46 ` [PATCH v3 1/3] selftests/damon: prevent cross-context state pollution in DamonCtx SJ Park
  2026-06-29 14:46 ` [PATCH v3 2/3] selftests/damon/damos_tried_regions: fix expectation output and join TypeError SJ Park
@ 2026-06-29 14:46 ` SJ Park
  2 siblings, 0 replies; 6+ messages in thread
From: SJ Park @ 2026-06-29 14:46 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Kunwu Chan, SJ Park, Shuah Khan, Wang Lian, damon, linux-kernel,
	linux-kselftest, linux-mm

From: Kunwu Chan <chentao@kylinos.cn>

'hugeapge_size' in drgn_dump_damon_status.py was a dead elif branch.
$fail_reason in sysfs.sh was undefined, silently emptying the error
message.  'exit' instead of 'exist' in sysfs.sh skipped a file
existence check.  'nohugeapge' in sysfs.py broke an action dict
lookup.

Fix other wrong strings in the same files.

Link: https://lore.kernel.org/20260601032314.424013-4-kunwu.chan@linux.dev
Co-developed-by: Wang Lian <lianux.mm@gmail.com>
Signed-off-by: Wang Lian <lianux.mm@gmail.com>
Signed-off-by: Kunwu Chan <chentao@kylinos.cn>
Reviewed-by: SJ Park <sj@kernel.org>
Cc: Kunwu Chan <chentao@kylinos.cn>
Cc: Wang Lian <lianux.mm@gmail.com>
Signed-off-by: SJ Park <sj@kernel.org>
---
 tools/testing/selftests/damon/_damon_sysfs.py               | 2 +-
 tools/testing/selftests/damon/damos_apply_interval.py       | 2 +-
 tools/testing/selftests/damon/damos_quota_goal.py           | 2 +-
 tools/testing/selftests/damon/drgn_dump_damon_status.py     | 2 +-
 tools/testing/selftests/damon/sysfs.py                      | 4 ++--
 tools/testing/selftests/damon/sysfs.sh                      | 6 +++---
 .../sysfs_update_schemes_tried_regions_wss_estimation.py    | 2 +-
 7 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/tools/testing/selftests/damon/_damon_sysfs.py b/tools/testing/selftests/damon/_damon_sysfs.py
index 2f6f2699db256..f6127081dfb2e 100644
--- a/tools/testing/selftests/damon/_damon_sysfs.py
+++ b/tools/testing/selftests/damon/_damon_sysfs.py
@@ -837,7 +837,7 @@ class Kdamond:
                 for goal in scheme.quota.goals:
                     err = goal.stage()
                     if err is not None:
-                        print('commit_schemes_quota_goals failed stagign: %s'%
+                        print('commit_schemes_quota_goals failed staging: %s'%
                               err)
                         exit(1)
         return write_file(os.path.join(self.sysfs_dir(), 'state'),
diff --git a/tools/testing/selftests/damon/damos_apply_interval.py b/tools/testing/selftests/damon/damos_apply_interval.py
index f04d43702481c..0f2f36584e48c 100755
--- a/tools/testing/selftests/damon/damos_apply_interval.py
+++ b/tools/testing/selftests/damon/damos_apply_interval.py
@@ -56,7 +56,7 @@ def main():
     # Because the second scheme was having the apply interval that is ten times
     # lower than that of the first scheme, the second scheme should be tried
     # about ten times more frequently than the first scheme.  For possible
-    # timing errors, check if it was at least nine times more freuqnetly tried.
+    # timing errors, check if it was at least nine times more frequently tried.
     ratio = nr_tried_stats[1] / nr_tried_stats[0]
     if ratio < 9:
         print('%d / %d = %f (< 9)' %
diff --git a/tools/testing/selftests/damon/damos_quota_goal.py b/tools/testing/selftests/damon/damos_quota_goal.py
index f76e0412b564c..661e4ba4765ae 100755
--- a/tools/testing/selftests/damon/damos_quota_goal.py
+++ b/tools/testing/selftests/damon/damos_quota_goal.py
@@ -66,7 +66,7 @@ def main():
             # effective quota was already minimum that cannot be more reduced
             if expect_increase is False and last_effective_bytes == 1:
                 continue
-            print('efective bytes not changed: %d' % goal.effective_bytes)
+            print('effective bytes not changed: %d' % goal.effective_bytes)
             exit(1)
 
         increased = last_effective_bytes < goal.effective_bytes
diff --git a/tools/testing/selftests/damon/drgn_dump_damon_status.py b/tools/testing/selftests/damon/drgn_dump_damon_status.py
index 972948e6215f1..26b207e44268d 100755
--- a/tools/testing/selftests/damon/drgn_dump_damon_status.py
+++ b/tools/testing/selftests/damon/drgn_dump_damon_status.py
@@ -163,7 +163,7 @@ def damos_filter_to_dict(damos_filter):
                                int(damos_filter.addr_range.end)]
     elif type_ == 'target':
         dict_['target_idx'] = int(damos_filter.target_idx)
-    elif type_ == 'hugeapge_size':
+    elif type_ == 'hugepage_size':
         dict_['sz_range'] = [int(damos_filter.sz_range.min),
                              int(damos_filter.sz_range.max)]
     return dict_
diff --git a/tools/testing/selftests/damon/sysfs.py b/tools/testing/selftests/damon/sysfs.py
index aa03a1187489f..99412f0d31f37 100755
--- a/tools/testing/selftests/damon/sysfs.py
+++ b/tools/testing/selftests/damon/sysfs.py
@@ -119,7 +119,7 @@ def assert_access_pattern_committed(pattern, dump):
                 'max_nr_accesses', dump)
     assert_true(dump['min_age_region'] == pattern.age[0], 'min_age_region',
                 dump)
-    assert_true(dump['max_age_region'] == pattern.age[1], 'miaxage_region',
+    assert_true(dump['max_age_region'] == pattern.age[1], 'max_age_region',
                 dump)
 
 def assert_scheme_committed(scheme, dump):
@@ -129,7 +129,7 @@ def assert_scheme_committed(scheme, dump):
             'cold': 1,
             'pageout': 2,
             'hugepage': 3,
-            'nohugeapge': 4,
+            'nohugepage': 4,
             'collapse': 5,
             'lru_prio': 6,
             'lru_deprio': 7,
diff --git a/tools/testing/selftests/damon/sysfs.sh b/tools/testing/selftests/damon/sysfs.sh
index 78f4badb5bebb..2eaaa5ae3c5ed 100755
--- a/tools/testing/selftests/damon/sysfs.sh
+++ b/tools/testing/selftests/damon/sysfs.sh
@@ -3,7 +3,7 @@
 
 source _common.sh
 
-# Kselftest frmework requirement - SKIP code is 4.
+# Kselftest framework requirement - SKIP code is 4.
 ksft_skip=4
 
 ensure_write_succ()
@@ -28,7 +28,7 @@ ensure_write_fail()
 
 	if (echo "$content" > "$file") 2> /dev/null
 	then
-		echo "writing $content to $file succeed ($fail_reason)"
+		echo "writing $content to $file succeeded ($reason)"
 		echo "expected failure because $reason"
 		exit 1
 	fi
@@ -363,7 +363,7 @@ test_context()
 {
 	context_dir=$1
 	ensure_dir "$context_dir" "exist"
-	ensure_file "$context_dir/avail_operations" "exit" 400
+	ensure_file "$context_dir/avail_operations" "exist" 400
 	ensure_file "$context_dir/operations" "exist" 600
 	ensure_file "$context_dir/addr_unit" "exist" 600
 	ensure_file "$context_dir/pause" "exist" 600
diff --git a/tools/testing/selftests/damon/sysfs_update_schemes_tried_regions_wss_estimation.py b/tools/testing/selftests/damon/sysfs_update_schemes_tried_regions_wss_estimation.py
index 35c724a63f6cf..16fdc6e7fc566 100755
--- a/tools/testing/selftests/damon/sysfs_update_schemes_tried_regions_wss_estimation.py
+++ b/tools/testing/selftests/damon/sysfs_update_schemes_tried_regions_wss_estimation.py
@@ -7,7 +7,7 @@ import time
 import _damon_sysfs
 
 def pass_wss_estimation(sz_region):
-    # access two regions of given size, 2 seocnds per each region
+    # access two regions of given size, 2 seconds per each region
     proc = subprocess.Popen(
             ['./access_memory', '2', '%d' % sz_region, '2000', 'repeat'])
     kdamonds = _damon_sysfs.Kdamonds([_damon_sysfs.Kdamond(
-- 
2.47.3

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v3 1/3] selftests/damon: prevent cross-context state pollution in DamonCtx
  2026-06-29 14:46 ` [PATCH v3 1/3] selftests/damon: prevent cross-context state pollution in DamonCtx SJ Park
@ 2026-06-29 14:55   ` sashiko-bot
  2026-06-30  0:24     ` SJ Park
  0 siblings, 1 reply; 6+ messages in thread
From: sashiko-bot @ 2026-06-29 14:55 UTC (permalink / raw)
  To: SJ Park; +Cc: damon

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3 1/3] selftests/damon: prevent cross-context state pollution in DamonCtx
  2026-06-29 14:55   ` sashiko-bot
@ 2026-06-30  0:24     ` SJ Park
  0 siblings, 0 replies; 6+ messages in thread
From: SJ Park @ 2026-06-30  0:24 UTC (permalink / raw)
  To: sashiko-bot; +Cc: SJ Park, damon

On Mon, 29 Jun 2026 14:55:52 +0000 sashiko-bot@kernel.org wrote:

> 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,
> }

Good findings.  But no blocker of this patch that makes some improvements.  And
fortunately there is no real callers that causing the issue for now, to my best
knowledge.  So not real urgent.  Someone who get interested in this change will
take time to fix this.


Thanks,
SJ

[...]

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-06-30  0:24 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-29 14:46 [PATCH v3 0/3] selftests/damon: misc fixes for test bugs SJ Park
2026-06-29 14:46 ` [PATCH v3 1/3] selftests/damon: prevent cross-context state pollution in DamonCtx SJ Park
2026-06-29 14:55   ` sashiko-bot
2026-06-30  0:24     ` SJ Park
2026-06-29 14:46 ` [PATCH v3 2/3] selftests/damon/damos_tried_regions: fix expectation output and join TypeError SJ Park
2026-06-29 14:46 ` [PATCH v3 3/3] selftests/damon: fix dead code, skipped checks, and broken lookups SJ Park

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox