[PATCH mm-hotfixes-unstable 0/3] mm/damon: fix watermark state, kunit pid leak and selftest fake pass

[PATCH mm-hotfixes-unstable 0/3] mm/damon: fix watermark state, kunit pid leak and selftest fake pass

[SJ Park]

Three hotfixes for core, kunit and selftest.  Fix kdamond deactivation
from missed watermark reset with patch 1.  Fix pid leak in kunit test
with patch 2.  Fix a selftest wrongly passing when it should skip, with
patch 3.

Note for reviewers.  This series is a collection of fixes that were
initially developed by different people.  Changelog is in each patch.

Aveline Noir (1):
  mm/damon/core: reset watermark wait state on each iteration

SJ Park (1):
  selftests/damon/sysfs_memcg_path_leak: define ksft_skip

Wang Xiaolei (1):
  mm/damon/tests/sysfs-kunit: fix leaked pid reference in add_targets
    test

 mm/damon/core.c                                        | 5 +++--
 mm/damon/tests/sysfs-kunit.h                           | 9 +++++++++
 tools/testing/selftests/damon/sysfs_memcg_path_leak.sh | 3 +++
 3 files changed, 15 insertions(+), 2 deletions(-)


base-commit: 395206c0e287bdd981918e033a2b1621ea6a3b9a
-- 
2.47.3

[PATCH mm-hotfixes-unstable 1/3] mm/damon/core: reset watermark wait state on each iteration

[SJ Park]

From: Aveline Noir <jm5905938@gmail.com>

Found while reviewing the DAMON source code that if an online commit
removes all schemes while kdamond is waiting for watermark activation,
the scheme loop becomes empty but min_wait_time keeps the previous
positive value.  kdamond then keeps sleeping instead of resuming
monitoring without schemes.

The way to wake up the kdamond again without DAMON is bit complicated.
Install a scheme having watermarks that can activate the kdamond again,
and remove the scheme again.  Doable but cumbersome and complicated.

Initialize min_wait_time and init_wait_time on each loop iteration so
the wait is calculated from the current schemes and an empty scheme
list resumes monitoring.

Fixes: da87878010e5 ("mm/damon/sysfs: support online inputs update")
Cc: <stable@vger.kernel.org> # 5.19.x
Assisted-by: LLM
Signed-off-by: Aveline Noir <jm5905938@gmail.com>
Reviewed-by: SJ Park <sj@kernel.org>
Signed-off-by: SJ Park <sj@kernel.org>
---
Changes from v1
- v1: https://lore.kernel.org/179277129124.2.16132047926074873101.damon-empty-schemes@gmail.com
- Collect R-b: from SJ.
- Add how to work around the problem.
- Update Fixes: and add Cc: stable@.
- Rebase to latest mm-new.

 mm/damon/core.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/mm/damon/core.c b/mm/damon/core.c
index 73b571b429cb3..7441cdea57dc5 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -3976,10 +3976,11 @@ static int kdamond_wait_activation(struct damon_ctx *ctx)
 {
 	struct damos *s;
 	unsigned long wait_time;
-	unsigned long min_wait_time = 0;
-	bool init_wait_time = false;
 
 	while (!kdamond_need_stop(ctx)) {
+		unsigned long min_wait_time = 0;
+		bool init_wait_time = false;
+
 		damon_for_each_scheme(s, ctx) {
 			wait_time = damos_wmark_wait_us(s);
 			if (!init_wait_time || wait_time < min_wait_time) {
-- 
2.47.3

Re: [PATCH mm-hotfixes-unstable 1/3] mm/damon/core: reset watermark wait state on each iteration

[sashiko-bot]

> From: Aveline Noir <jm5905938@gmail.com>
> 
> Found while reviewing the DAMON source code that if an online commit
> removes all schemes while kdamond is waiting for watermark activation,
> the scheme loop becomes empty but min_wait_time keeps the previous
> positive value.  kdamond then keeps sleeping instead of resuming
> monitoring without schemes.
> 
> The way to wake up the kdamond again without DAMON is bit complicated.
> Install a scheme having watermarks that can activate the kdamond again,
> and remove the scheme again.  Doable but cumbersome and complicated.
> 
> Initialize min_wait_time and init_wait_time on each loop iteration so
> the wait is calculated from the current schemes and an empty scheme
> list resumes monitoring.
> 
> Fixes: da87878010e5 ("mm/damon/sysfs: support online inputs update")
> Cc: <stable@vger.kernel.org> # 5.19.x
> Assisted-by: LLM
> Signed-off-by: Aveline Noir <jm5905938@gmail.com>
> Reviewed-by: SJ Park <sj@kernel.org>
> Signed-off-by: SJ Park <sj@kernel.org>

Sashiko has reviewed this patch and found no issues.

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

[PATCH mm-hotfixes-unstable 2/3] mm/damon/tests/sysfs-kunit: fix leaked pid reference in add_targets test

[SJ Park]

From: Wang Xiaolei <xiaoleiwangxiaolei@gmail.com>

damon_sysfs_test_add_targets() creates its monitoring context with
damon_new_ctx(), which leaves ctx->ops zero-initialized.  Since
DAMON_OPS_VADDR is zero, damon_target_has_pid() returns true for the
context, and damon_sysfs_add_targets() therefore calls
damon_set_target_pid() for each target it adds.  That function grabs a
reference to the target's struct pid via find_get_pid().

The test then tears the context down with damon_destroy_ctx(), which
releases such a reference from ctx->ops.cleanup_target().  For the vaddr
operations set that callback is damon_va_cleanup_target(), and it is the
only place where the reference is put.  The test never selects an
operations set, though, so cleanup_target is NULL and both references --
one for each of the two damon_sysfs_add_targets() calls -- are leaked on
every run of the test.  Running the test with CONFIG_DEBUG_KMEMLEAK
enabled reports the leaked struct pid objects as unreferenced.

Fix this by selecting the vaddr operations set for the context before
adding the targets, mirroring what the sysfs interface does.
damon_select_ops() fails if the vaddr operations set is not registered,
in which case the test cannot exercise the pid handling and skips
itself.

Fixes: b8ee5575f763 ("mm/damon/sysfs-test: add a unit test for damon_sysfs_set_targets()")
Cc: <stable@vger.kernel.org> # 6.7.x
Cc: Michal Hocko <mhocko@suse.com>
Cc: Mike Rapoport <rppt@kernel.org>
Cc: Suren Baghdasaryan <surenb@google.com>
Cc: Vlastimil Babka <vbabka@kernel.org>
Cc: "Liam R. Howlett" <liam@infradead.org>
Signed-off-by: Wang Xiaolei <xiaoleiwangxiaolei@gmail.com>
Reviewed-by: SJ Park <sj@kernel.org>
Signed-off-by: SJ Park <sj@kernel.org>
---
Changes from v1
- v1: https://lore.kernel.org/20261029075609.35661-1-xiaoleiwangxiaolei@gmail.com
- Add Cc: stable@.
- Collect R-b: from SJ.
- Rebase to latest mm-new.

 mm/damon/tests/sysfs-kunit.h | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/mm/damon/tests/sysfs-kunit.h b/mm/damon/tests/sysfs-kunit.h
index 138a4b8d14e73..16373ba751dbc 100644
--- a/mm/damon/tests/sysfs-kunit.h
+++ b/mm/damon/tests/sysfs-kunit.h
@@ -79,6 +79,15 @@ static void damon_sysfs_test_add_targets(struct kunit *test)
 		kunit_skip(test, "ctx alloc fail");
 	}
 
+	if (damon_select_ops(ctx, DAMON_OPS_VADDR)) {
+		damon_destroy_ctx(ctx);
+		kfree(sysfs_targets->targets_arr);
+		kfree(sysfs_targets);
+		kfree(sysfs_target->regions);
+		kfree(sysfs_target);
+		kunit_skip(test, "vaddr ops not registered");
+	}
+
 	damon_sysfs_add_targets(ctx, sysfs_targets);
 	KUNIT_EXPECT_EQ(test, 1u, nr_damon_targets(ctx));
 
-- 
2.47.3

Re: [PATCH mm-hotfixes-unstable 2/3] mm/damon/tests/sysfs-kunit: fix leaked pid reference in add_targets test

[sashiko-bot]

> From: Wang Xiaolei <xiaoleiwangxiaolei@gmail.com>
> 
> damon_sysfs_test_add_targets() creates its monitoring context with
> damon_new_ctx(), which leaves ctx->ops zero-initialized.  Since
> DAMON_OPS_VADDR is zero, damon_target_has_pid() returns true for the
> context, and damon_sysfs_add_targets() therefore calls
> damon_set_target_pid() for each target it adds.  That function grabs a
> reference to the target's struct pid via find_get_pid().
> 
> The test then tears the context down with damon_destroy_ctx(), which
> releases such a reference from ctx->ops.cleanup_target().  For the vaddr
> operations set that callback is damon_va_cleanup_target(), and it is the
> only place where the reference is put.  The test never selects an
> operations set, though, so cleanup_target is NULL and both references --
> one for each of the two damon_sysfs_add_targets() calls -- are leaked on
> [ ... ]
> Cc: Vlastimil Babka <vbabka@kernel.org>
> Cc: "Liam R. Howlett" <liam@infradead.org>
> Signed-off-by: Wang Xiaolei <xiaoleiwangxiaolei@gmail.com>
> Reviewed-by: SJ Park <sj@kernel.org>
> Signed-off-by: SJ Park <sj@kernel.org>

Sashiko has reviewed this patch and found no issues.

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

[PATCH mm-hotfixes-unstable 3/3] selftests/damon/sysfs_memcg_path_leak: define ksft_skip

[SJ Park]

sysfs_memcg_path_leak test uses $ksft_skip without defining it.  As a
result, the test is marked as passed while it was skipped, like below.

    $ ./sysfs_memcg_path_leak.sh
    Run as root
    $ echo $?
    0

Users who run the test could therefore wrongly believe the test was run
and passed while it was skipped.

Define the value at the beginning of the test.  After this patch, the
exit code reflects the skip.

    $ ./sysfs_memcg_path_leak.sh
    Run as root
    $ echo $?
    4

Fixes: dfa3cf0bc018 ("selftets/damon: add a test for memcg_path leak")
Cc: <stable@vger.kernel.org> # 6.17.x
Closes: https://sashiko.dev/#/bug/linux-a0456336-0608-4fec-ac22-0ee61d1d6a91
Reviewed-by: Aveline Noir <jm5905938@gmail.com>
Tested-by: Aveline Noir <jm5905938@gmail.com>
Signed-off-by: SJ Park <sj@kernel.org>
---
Changes from RFC
- RFC: https://lore.kernel.org/20261030044643.157105-1-sj@kernel.org
- Drop RFC tag.
- Rebase to latest mm-new.

 tools/testing/selftests/damon/sysfs_memcg_path_leak.sh | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tools/testing/selftests/damon/sysfs_memcg_path_leak.sh b/tools/testing/selftests/damon/sysfs_memcg_path_leak.sh
index 34c37129c49fe..d407dddb879ec 100755
--- a/tools/testing/selftests/damon/sysfs_memcg_path_leak.sh
+++ b/tools/testing/selftests/damon/sysfs_memcg_path_leak.sh
@@ -1,6 +1,9 @@
 #!/bin/bash
 # SPDX-License-Identifier: GPL-2.0
 
+# Kselftest framework requirement - SKIP code is 4.
+ksft_skip=4
+
 if [ $EUID -ne 0 ]
 then
 	echo "Run as root"
-- 
2.47.3

Re: [PATCH mm-hotfixes-unstable 3/3] selftests/damon/sysfs_memcg_path_leak: define ksft_skip

[sashiko-bot]

> sysfs_memcg_path_leak test uses $ksft_skip without defining it.  As a
> result, the test is marked as passed while it was skipped, like below.
> 
>     $ ./sysfs_memcg_path_leak.sh
>     Run as root
>     $ echo $?
>     0
> 
> Users who run the test could therefore wrongly believe the test was run
> and passed while it was skipped.
> 
> Define the value at the beginning of the test.  After this patch, the
> exit code reflects the skip.
> 
>     $ ./sysfs_memcg_path_leak.sh
>     Run as root
>     $ echo $?
>     4
> 
> Fixes: dfa3cf0bc018 ("selftets/damon: add a test for memcg_path leak")
> Cc: <stable@vger.kernel.org> # 6.17.x
> Closes: https://sashiko.dev/#/bug/linux-a0456336-0608-4fec-ac22-0ee61d1d6a91
> Reviewed-by: Aveline Noir <jm5905938@gmail.com>
> Tested-by: Aveline Noir <jm5905938@gmail.com>
> Signed-off-by: SJ Park <sj@kernel.org>

Sashiko has reviewed this patch and found no issues.

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