mm/damon: misc improvements in tests and documents

Add various improvements to DAMON tests and documents.

Patch 1 and 2 from Zenghui Yu (Huawei) let DAMON selftest to clean up its state and output after running the tests. Patch 3 from Eva Kurchatova makes a few selftest be able to run with Python's safe path mode. Patch 4 from Kunwu Chan improves coverage of DAMON kunit tests. Finally, patches 5 and 6 from Liew Rui Yan clarify and fix typos on documents for DAMOS quota behaviors.

Below are notes that can be removed in the final commit message.

This is a batched repost of DAMON patches that were individually posted. Please refer to each patch for changelog.

Eva Kurchatova (1):
selftests/damon: add script dir to sys.path for PYTHONSAFEPATH
compatibility

Kunwu Chan (1):
mm/damon/tests/core-kunit: improve nr_samples_per_aggr test isolation

Liew Rui Yan (2):
Docs/mm/damon/design: clarify when qt_exceeds increases Docs/mm/damon/design: fix typos in temporal auto-tuning algorithm
section

Zenghui Yu (Huawei) (2):
selftests/damon: stop kdamond on error exits of no-op commit test selftests/damon: ignore test-generated damon_dump_output

 Documentation/mm/damon/design.rst             |  8 +++--
 mm/damon/tests/core-kunit.h                   | 19 +++++++---
 tools/testing/selftests/damon/.gitignore      |  1 +
 .../selftests/damon/damon_nr_regions.py       |  3 ++
 .../selftests/damon/damos_apply_interval.py   |  3 ++
 tools/testing/selftests/damon/damos_quota.py  |  3 ++
 .../selftests/damon/damos_quota_goal.py       |  3 ++
 .../selftests/damon/damos_tried_regions.py    |  3 ++
 .../damon/sysfs_no_op_commit_break.py         | 35 ++++++++++---------
 ...sysfs_update_schemes_tried_regions_hang.py |  3 ++
 ...te_schemes_tried_regions_wss_estimation.py |  3 ++
 11 files changed, 60 insertions(+), 24 deletions(-)

base-commit: d1153a22117b5c6171bdf24d32b7f6c314887d23

2.47.3

From: "Zenghui Yu (Huawei)" <[email protected]>

The sysfs_no_op_commit_break test starts a kdamond via sysfs, but its error paths (e.g., drgn not installed) exit without stopping it. The leaked kdamond then makes subsequent tests, e.g. lru_sort.sh and reclaim.sh, skip with "Another kdamond is running".

Wrap the post-start logic in try-finally so that kdamonds.stop() is executed on every exit path.

---
Changes from v1
- v1: https://lore.kernel.org/[email protected]
- Collect R-b: from SJ.
- Rebase to latest mm-new.

 .../damon/sysfs_no_op_commit_break.py         | 35 ++++++++++---------
 1 file changed, 18 insertions(+), 17 deletions(-)

index 2c65cffe6b545..81774ddc55ac4 100755
@@ -47,26 +47,27 @@ def main():
         print('kdamond start failed: %s' % err)
         exit(1)
 
-    before_commit_status, err = \
-        dump_damon_status_dict(kdamonds.kdamonds[0].pid)
-    if err is not None:
-        print('before-commit status dump failed: %s' % err)
-        exit(1)
+    try:
+        before_commit_status, err = \
+            dump_damon_status_dict(kdamonds.kdamonds[0].pid)
+        if err is not None:
+            print('before-commit status dump failed: %s' % err)
+            exit(1)
 
-    kdamonds.kdamonds[0].commit()
+        kdamonds.kdamonds[0].commit()
 
-    after_commit_status, err = \
-        dump_damon_status_dict(kdamonds.kdamonds[0].pid)
-    if err is not None:
-        print('after-commit status dump failed: %s' % err)
-        exit(1)
-
-    if before_commit_status != after_commit_status:
-        print(f'before: {json.dumps(before_commit_status, indent=2)}')
-        print(f'after: {json.dumps(after_commit_status, indent=2)}')
-        exit(1)
+        after_commit_status, err = \
+            dump_damon_status_dict(kdamonds.kdamonds[0].pid)
+        if err is not None:
+            print('after-commit status dump failed: %s' % err)
+            exit(1)
 
-    kdamonds.stop()
+        if before_commit_status != after_commit_status:
+            print(f'before: {json.dumps(before_commit_status, indent=2)}')
+            print(f'after: {json.dumps(after_commit_status, indent=2)}')
+            exit(1)
+    finally:
+        kdamonds.stop()
 
 if __name__ == '__main__':
     main()

2.47.3

From: "Zenghui Yu (Huawei)" <[email protected]>

sysfs.py and sysfs_no_op_commit_break.py dump the DAMON status collected via drgn_dump_damon_status.py into a damon_dump_output file in the working directory. Ignore it so that running the tests in-tree does not pollute git status.

---
Changes from v1
- v1: https://lore.kernel.org/[email protected]
- Collect R-b: from SJ.
- Rebase to latest mm-new.

 tools/testing/selftests/damon/.gitignore | 1 +
 1 file changed, 1 insertion(+)

index 2f0297657c816..226bd7c4e0728 100644
@@ -1,3 +1,4 @@
 # SPDX-License-Identifier: GPL-2.0-only
 access_memory
 access_memory_even
+damon_dump_output

2.47.3

From: Eva Kurchatova <[email protected]>

Running these tests under Python's safe path mode, either with -P on the interpreter command line or with PYTHONSAFEPATH set in the environment, stops the script's own directory being prepended to sys.path. Some distributions (RHEL, for example) build the tests with -P in the shebang.

This breaks all 7 DAMON Python selftests that import the _damon_sysfs helper module located in the same directory:

ModuleNotFoundError: No module named '_damon_sysfs'

Fix this by explicitly adding the script's directory to sys.path before importing _damon_sysfs, following the same pattern used in commit c3b3eb565bd7 ("tools: ynl: add script dir to sys.path") which fixed the identical issue for the YNL tools.

---
Changes from v1
- v1: https://lore.kernel.org/[email protected]
- Collect R-b: from SJ.
- Rebase to latest mm-new.

 tools/testing/selftests/damon/damon_nr_regions.py              | 3 +++
 tools/testing/selftests/damon/damos_apply_interval.py          | 3 +++
 tools/testing/selftests/damon/damos_quota.py                   | 3 +++
 tools/testing/selftests/damon/damos_quota_goal.py              | 3 +++
 tools/testing/selftests/damon/damos_tried_regions.py           | 3 +++
 .../selftests/damon/sysfs_update_schemes_tried_regions_hang.py | 3 +++
 .../damon/sysfs_update_schemes_tried_regions_wss_estimation.py | 3 +++
 7 files changed, 21 insertions(+)

index 58f3291fed12a..e55239813c654 100755
@@ -1,9 +1,12 @@
 #!/usr/bin/env python3
 # SPDX-License-Identifier: GPL-2.0
 
+import os
 import subprocess
+import sys
 import time
 
+sys.path.append(os.path.dirname(os.path.abspath(__file__)))
 import _damon_sysfs
 
 def test_nr_regions(real_nr_regions, min_nr_regions, max_nr_regions):

index 0f2f36584e48c..0bf7768b2006a 100755
@@ -1,9 +1,12 @@
 #!/usr/bin/env python3
 # SPDX-License-Identifier: GPL-2.0
 
+import os
 import subprocess
+import sys
 import time
 
+sys.path.append(os.path.dirname(os.path.abspath(__file__)))
 import _damon_sysfs
 
 def main():

index 57c4937aaed28..879115a499bf8 100755
@@ -1,9 +1,12 @@
 #!/usr/bin/env python3
 # SPDX-License-Identifier: GPL-2.0
 
+import os
 import subprocess
+import sys
 import time
 
+sys.path.append(os.path.dirname(os.path.abspath(__file__)))
 import _damon_sysfs
 
 def main():

index 661e4ba4765ae..fed033a0afdf9 100755
@@ -1,9 +1,12 @@
 #!/usr/bin/env python3
 # SPDX-License-Identifier: GPL-2.0
 
+import os
 import subprocess
+import sys
 import time
 
+sys.path.append(os.path.dirname(os.path.abspath(__file__)))
 import _damon_sysfs
 
 def main():

index d6472e6a6e082..6941f87c10b1c 100755
@@ -1,9 +1,12 @@
 #!/usr/bin/env python3
 # SPDX-License-Identifier: GPL-2.0
 
+import os
 import subprocess
+import sys
 import time
 
+sys.path.append(os.path.dirname(os.path.abspath(__file__)))
 import _damon_sysfs
 
 def main():

index 28c887a0108fd..625761c243b58 100755
@@ -1,9 +1,12 @@
 #!/usr/bin/env python3
 # SPDX-License-Identifier: GPL-2.0
 
+import os
 import subprocess
+import sys
 import time
 
+sys.path.append(os.path.dirname(os.path.abspath(__file__)))
 import _damon_sysfs
 
 def main():

index 16fdc6e7fc566..36e7ae5f826d8 100755
@@ -1,9 +1,12 @@
 #!/usr/bin/env python3
 # SPDX-License-Identifier: GPL-2.0
 
+import os
 import subprocess
+import sys
 import time
 
+sys.path.append(os.path.dirname(os.path.abspath(__file__)))
 import _damon_sysfs
 
 def pass_wss_estimation(sz_region):

2.47.3

Agreed, but not necessarily a blocker of this patch. I will separately work on the other tests.

Thanks,
SJ

From: Kunwu Chan <[email protected]>

Test the zero sample interval case with a non-zero aggregation interval, and keep the zero/zero case to cover the zero-result fallback.

Also make the overflow case use an explicit sample interval so that it does not depend on the zero sample interval fallback.

---
Changes from v2
- v2: https://lore.kernel.org/[email protected]
- Collect R-b: from SJ.
- Rebase to latest mm-new.
Changes from v1
- v1: https://lore.kernel.org/damon/[email protected]/
- Use 0/0 for the zero-result case, consistent with damon_set_attrs().
- Update commit msg and add Reviewed-by tag.

 mm/damon/tests/core-kunit.h | 19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)

index c01e6a75cadc1..a47a5cdf285c8 100644
@@ -627,12 +627,20 @@ static void damon_test_set_regions(struct kunit *test)
 
 static void damon_test_nr_samples_per_aggr(struct kunit *test)
 {
-	struct damon_attrs attrs = {
+	struct damon_attrs attrs;
+
+	/* Zero sample interval is treated as one. */
+	attrs = (struct damon_attrs){
 		.sample_interval = 0,
-		.aggr_interval = 0,
+		.aggr_interval = 5000,
 	};
+	KUNIT_EXPECT_EQ(test, damon_nr_samples_per_aggr(&attrs), 5000);
 
-	/* Zero aggregation interval doesn't cause division by zero */
+	/* Zero sample and aggregation intervals cover the zero-result fallback. */
+	attrs = (struct damon_attrs){
+		.sample_interval = 0,
+		.aggr_interval = 0,
+	};
 	KUNIT_EXPECT_EQ(test, damon_nr_samples_per_aggr(&attrs), 1);
 
 	/*

@@ -640,7 +648,10 @@ static void damon_test_nr_samples_per_aggr(struct kunit *test)
 	 * overflow
 	 */
 	if (ULONG_MAX > UINT_MAX) {
-		attrs.aggr_interval = (unsigned long)UINT_MAX + 1;
+		attrs = (struct damon_attrs){
+			.sample_interval = 1,
+			.aggr_interval = (unsigned long)UINT_MAX + 1,
+		};
 		KUNIT_EXPECT_EQ(test, damon_nr_samples_per_aggr(&attrs),
 				UINT_MAX);
 	}

2.47.3

From: Liew Rui Yan <[email protected]>

qt_exceeds counts how many times a scheme's quota has been exceeded.

When using the temporal auto-tuning algorithm, the effective size quota becomes zero once the goal is [over-]achieved. In this case, the quotas are still set, so qt_exceeds keeps increasing once per quota reset interval while the goal stays achieved.

Clarify this behavior in the design document.

---
Changes from v1
- v1: https://lore.kernel.org/[email protected]
- Collect R-b: From SJ.
- Rebase to latest mm-new.
Changes from RFC v1
- RFC v1: https://lore.kernel.org/damon/[email protected]
- Simplify the explanation. Do not provide a complete definition of qt_exceeds, but rather explain only the increment condition when using temporal auto-tuning algorithm, simply to prevent users from becoming confused and being unable to find any explanation.

 Documentation/mm/damon/design.rst | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

index cb116a82ff20b..ad09416dfb21a 100644
@@ -696,7 +696,9 @@ There are two such tuning algorithms that users can select as they need.
   fast as possible, using maximum allowed quota, but only for a temporal short
   time.  When the quota is under-achieved, this algorithm keeps tuning quota to
   a maximum allowed one.  Once the quota is [over]-achieved, this sets the
-  quota zero.  Useful for deterministic control required environments.
+  quota zero.  Useful for deterministic control required environments.  Note
+  that the zero quota is a valid quota, and therefore ``qt_exceeds`` :ref:`stat
+  <damon_design_damos_stat>` will keep increasing in this case.
 
 The goal can be specified with five parameters, namely ``target_metric``,
 ``target_value``, ``current_value``, ``nid`` and ``path``.  The auto-tuning

2.47.3

From: Liew Rui Yan <[email protected]>

Correct incorrect references of "quota" to "goal" in the temporal auto-tuning algorithm description and fix the square bracket formatting.

---
Changes from v1
- v1: https://lore.kernel.org/[email protected]
- Collect R-b: from SJ.
- Rebase to latest mm-new.

 Documentation/mm/damon/design.rst | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

index ad09416dfb21a..707170bcd1b33 100644
@@ -694,8 +694,8 @@ There are two such tuning algorithms that users can select as they need.
   This is the default selection.  If unsure, use this.
 - ``temporal``: More straightforward algorithm.  Tries to achieve the goal as
   fast as possible, using maximum allowed quota, but only for a temporal short
-  time.  When the quota is under-achieved, this algorithm keeps tuning quota to
-  a maximum allowed one.  Once the quota is [over]-achieved, this sets the
+  time.  When the goal is under-achieved, this algorithm keeps tuning quota to
+  a maximum allowed one.  Once the goal is [over-]achieved, this sets the
   quota zero.  Useful for deterministic control required environments.  Note
   that the zero quota is a valid quota, and therefore ``qt_exceeds`` :ref:`stat
   <damon_design_damos_stat>` will keep increasing in this case.

2.47.3

Sashiko found no blocker for this series. Sashiko send its findings to damon@ mailing list [1], and I reply those if anything needs to be clarified. Please refer to those for details.

[1] https://lore.kernel.org/damon/

Thanks,
SJ

[...]

All queued, thanks,