Skip to contentSkip to conversation

mm/damon/stat: add kdamond_pid parameter

RFC PATCH Superseded SeongJae Park 2 participants 7 messages Apr 16

Subsystems (4)
Memory management Documentation MM misc DAMON

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

SeongJae Park

author maintainer
#0 lore raw

cover mm/damon/stat: add kdamond_pid parameter

DAMON_STAT doesn't provide the pid of its kdamond, unlike DAMON_RECLAIM and DAMON_LRU_SORT. This makes user-space management of DAMON_STAT unnecessarily complicated. Provide the information via a new parameter, namely kdamond_pid, and document it.

Changes from RFC v1.1
- rfc v1.1: https://lore.kernel.org/[email protected]
- Close the parentheses of error handling block.
Changes from RFC
- rfc: https://lore.kernel.org/[email protected]
- Fix damon_kdamond_pid() failure handling.

SeongJae Park (2):
mm/damon/stat: add a parameter for reading kdamond pid Docs/admin-guide/mm/damon/stat: document kdamond_pid parameter

 Documentation/admin-guide/mm/damon/stat.rst |  7 +++++++
 mm/damon/stat.c                             | 18 ++++++++++++++++++
 2 files changed, 25 insertions(+)

base-commit: bf44f59d29186d80db01e4124a8ab23b3b235b32

Signature · 1 line
2.47.3
author maintainer
#1 lore raw

RFC PATCH 1/2 mm/damon/stat: add a parameter for reading kdamond pid

Knowing the pid of the kdamonds can help user-space management including monitoring of DAMON's system resource consumption. To make it easier, DAMON_SYSFS, DAMON_RECLAIM and DAMON_LRU_SORT provide the pid information. DAMON_STAT is not providing it, though. Expose the pid of DAMON_STAT kdamond via a new read-only module parameter, namely kdamond_pid. This also makes DAMON modules usage more standardized, because DAMON_RECLAIM and DAMON_LRU_SORT also provide the information via their read-only parameters of the same name.

Signed-off-by SeongJae Park <[email protected]>
---
 mm/damon/stat.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)
mm/damon/stat.c +18 -0
index 99ba346f9e325..4901e9a7c3398 100644
@@ -44,6 +44,15 @@ module_param(aggr_interval_us, ulong, 0400);
 MODULE_PARM_DESC(aggr_interval_us,
 		"Current tuned aggregation interval in microseconds");
 
+/*
+ * PID of the DAMON thread
+ *
+ * If DAMON_STAT is enabled, this becomes the PID of the worker thread.
+ * Else, -1.
+ */
+static int kdamond_pid __read_mostly = -1;
+module_param(kdamond_pid, int, 0400);
+
 static struct damon_ctx *damon_stat_context;
 
 static unsigned long damon_stat_last_refresh_jiffies;

@@ -260,6 +269,14 @@ static int damon_stat_start(void)
 		damon_stat_context = NULL;
 		return err;
 	}
+	kdamond_pid = damon_kdamond_pid(damon_stat_context);
+	if (kdamond_pid < 0) {
+		err = kdamond_pid;
+		kdamond_pid = -1;
+		damon_destroy_ctx(damon_stat_context);
+		damon_stat_context = NULL;
+		return err;
+	}
 
 	damon_stat_last_refresh_jiffies = jiffies;
 	call_control.data = damon_stat_context;

@@ -269,6 +286,7 @@ static int damon_stat_start(void)
 static void damon_stat_stop(void)
 {
 	damon_stop(&damon_stat_context, 1);
+	kdamond_pid = -1;
 	damon_destroy_ctx(damon_stat_context);
 	damon_stat_context = NULL;
 }
Signature · 1 line
2.47.3
AI review
#2 lore raw

Re: [RFC PATCH v1.2 1/2] mm/damon/stat: add a parameter for reading kdamond pid

Sashiko AI review found 1 potential issue(s): - [Medium] Stale `kdamond_pid` exposed to userspace if `damon_call()` fails during startup. --

Sashiko AI review found 1 potential issue(s):
- [Medium] Stale kdamond_pid exposed to userspace if damon_call() fails during startup. --

commit 46eb0a4278ccd2dec190db28b78b0766b8865901
Author: SeongJae Park <[email protected]>

mm/damon/stat: add a parameter for reading kdamond pid

This commit exposes the pid of the DAMON_STAT kdamond via a new read-only module parameter named kdamond_pid, standardizing usage with other DAMON modules.

Quoted 4 lines
mm/damon/stat.c +0 -0
index 99ba346f9e325..4901e9a7c3398 100644

[ ... ]

Quoted 5 lines
mm/damon/stat.c +1 -0
@@ -260,6 +269,14 @@ static int damon_stat_start(void)
 		damon_stat_context = NULL;
 		return err;
 	}
+	kdamond_pid = damon_kdamond_pid(damon_stat_context);

mm/damon/stat.c static int damon_stat_start(void) · line 282

Quoted 10 lines
mm/damon/stat.c +7 -0
+	if (kdamond_pid < 0) {
+		err = kdamond_pid;
+		kdamond_pid = -1;
+		damon_destroy_ctx(damon_stat_context);
+		damon_stat_context = NULL;
+		return err;
+	}
 
 	damon_stat_last_refresh_jiffies = jiffies;
 	call_control.data = damon_stat_context;

If damon_call() fails right after this and returns an error, damon_stat_start() will pass that error back to its caller and the module will revert to a disabled state.

Does this leave a stale kdamond_pid exposed to userspace? The newly added documentation states: "If DAMON_STAT is enabled, this becomes the PID of the worker thread. Else, -1." Since the initialization fails, the module remains disabled, but kdamond_pid isn't reset to -1 on this error path.

Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
author maintainer
#3 lore raw

Re: [RFC PATCH v1.2 1/2] mm/damon/stat: add a parameter for reading kdamond pid

TL; DR: Sashiko found good point. I will fix it in the next revision.

Quoted 40 lines · On Thu, 16 Apr 2026 00:59:35 +0000 [email protected] wrote:

Sashiko AI review found 1 potential issue(s):
- [Medium] Stale kdamond_pid exposed to userspace if damon_call() fails during startup. --

commit 46eb0a4278ccd2dec190db28b78b0766b8865901
Author: SeongJae Park <[email protected]>

mm/damon/stat: add a parameter for reading kdamond pid

This commit exposes the pid of the DAMON_STAT kdamond via a new read-only module parameter named kdamond_pid, standardizing usage with other DAMON modules.

Quoted 4 lines
mm/damon/stat.c +0 -0
index 99ba346f9e325..4901e9a7c3398 100644

[ ... ]

Quoted 5 lines
mm/damon/stat.c +1 -0
@@ -260,6 +269,14 @@ static int damon_stat_start(void)
 		damon_stat_context = NULL;
 		return err;
 	}
+	kdamond_pid = damon_kdamond_pid(damon_stat_context);

mm/damon/stat.c static int damon_stat_start(void) · line 282

Quoted 10 lines
mm/damon/stat.c +7 -0
+	if (kdamond_pid < 0) {
+		err = kdamond_pid;
+		kdamond_pid = -1;
+		damon_destroy_ctx(damon_stat_context);
+		damon_stat_context = NULL;
+		return err;
+	}
 
 	damon_stat_last_refresh_jiffies = jiffies;
 	call_control.data = damon_stat_context;

If damon_call() fails right after this and returns an error, damon_stat_start() will pass that error back to its caller and the module will revert to a disabled state.

Quoted 4 lines

Does this leave a stale kdamond_pid exposed to userspace? The newly added documentation states: "If DAMON_STAT is enabled, this becomes the PID of the worker thread. Else, -1." Since the initialization fails, the module remains disabled, but kdamond_pid isn't reset to -1 on this error path.

Nice catch. I will do the kdamond_pid setup after damon_call() success, like below. Also, I will remove the damon_stat_context deallocation and unset when damon_kdamond_pid() failure, because it will be deallocated when damon_stat_start() is called again. The change will look like below:

mm/damon/stat.c +7 -6
@@ -222,18 +222,19 @@ static int damon_stat_start(void)
                damon_stat_context = NULL;
                return err;
        }
+
+       damon_stat_last_refresh_jiffies = jiffies;
+       call_control.data = damon_stat_context;
+       err = damon_call(damon_stat_context, &call_control);
+       if (err)
+               return err;
+
        kdamond_pid = damon_kdamond_pid(damon_stat_context);
        if (kdamond_pid < 0) {
                err = kdamond_pid;
                kdamond_pid = -1;
-               damon_destroy_ctx(damon_stat_context);
-               damon_stat_context = NULL;
                return err;
        }
-
-       damon_stat_last_refresh_jiffies = jiffies;
-       call_control.data = damon_stat_context;
-       return damon_call(damon_stat_context, &call_control);
 }
 
 static void damon_stat_stop(void)
Quoted 3 lines
Signature · 2 lines
-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1

Thanks,
SJ

author maintainer
#4 lore raw

Re: [RFC PATCH v1.2 1/2] mm/damon/stat: add a parameter for reading kdamond pid

Quoted 44 lines · On Wed, 15 Apr 2026 20:35:45 -0700 SeongJae Park <[email protected]> wrote:

TL; DR: Sashiko found good point. I will fix it in the next revision.

Quoted 40 lines · On Thu, 16 Apr 2026 00:59:35 +0000 [email protected] wrote:

Sashiko AI review found 1 potential issue(s):
- [Medium] Stale kdamond_pid exposed to userspace if damon_call() fails during startup. --

commit 46eb0a4278ccd2dec190db28b78b0766b8865901
Author: SeongJae Park <[email protected]>

mm/damon/stat: add a parameter for reading kdamond pid

This commit exposes the pid of the DAMON_STAT kdamond via a new read-only module parameter named kdamond_pid, standardizing usage with other DAMON modules.

Quoted 4 lines
mm/damon/stat.c +0 -0
index 99ba346f9e325..4901e9a7c3398 100644

[ ... ]

Quoted 5 lines
mm/damon/stat.c +1 -0
@@ -260,6 +269,14 @@ static int damon_stat_start(void)
 		damon_stat_context = NULL;
 		return err;
 	}
+	kdamond_pid = damon_kdamond_pid(damon_stat_context);

mm/damon/stat.c static int damon_stat_start(void) · line 282

Quoted 10 lines
mm/damon/stat.c +7 -0
+	if (kdamond_pid < 0) {
+		err = kdamond_pid;
+		kdamond_pid = -1;
+		damon_destroy_ctx(damon_stat_context);
+		damon_stat_context = NULL;
+		return err;
+	}
 
 	damon_stat_last_refresh_jiffies = jiffies;
 	call_control.data = damon_stat_context;

If damon_call() fails right after this and returns an error, damon_stat_start() will pass that error back to its caller and the module will revert to a disabled state.

Quoted 7 lines
Quoted 4 lines

Does this leave a stale kdamond_pid exposed to userspace? The newly added documentation states: "If DAMON_STAT is enabled, this becomes the PID of the worker thread. Else, -1." Since the initialization fails, the module remains disabled, but kdamond_pid isn't reset to -1 on this error path.

Nice catch. I will do the kdamond_pid setup after damon_call() success, like below.

Wait... The same question was also raised [1] by Sashiko on the previous version. And I was wrongly saying it is not this patch's fault. That was completely wrong.

And the kdamond can stop even after the damon_kdamond_pid() call, if the region_store_histogram allocation fails. In the case, kdamond_pid will again be stale. 'enabled' parameter value will also be stale in the case. The worst part of it is that in the case, DAMON_STAT cannot be restarted before the system is reboot. The bug is similar to the one [2] that Liew found and working on.

I will hold this patch and fix the bug first.

Quoted 3 lines

Also, I will remove the damon_stat_context deallocation and unset when damon_kdamond_pid() failure, because it will be deallocated when damon_stat_start() is called again.

This also means my reply [1] to Sashiko's question about the damon_stat_context leak on the previous version was wrong. I was saying it is indeed a bug, but it was not.

[1] https://lore.kernel.org/[email protected]
[2] https://lore.kernel.org/[email protected]

Thanks,
SJ

[...]

author maintainer
#5 lore raw

Re: [RFC PATCH v1.2 1/2] mm/damon/stat: add a parameter for reading kdamond pid

Quoted 58 lines · On Wed, 15 Apr 2026 21:06:01 -0700 SeongJae Park <[email protected]> wrote:
Quoted 44 lines · On Wed, 15 Apr 2026 20:35:45 -0700 SeongJae Park <[email protected]> wrote:

TL; DR: Sashiko found good point. I will fix it in the next revision.

Quoted 40 lines · On Thu, 16 Apr 2026 00:59:35 +0000 [email protected] wrote:

Sashiko AI review found 1 potential issue(s):
- [Medium] Stale kdamond_pid exposed to userspace if damon_call() fails during startup. --

commit 46eb0a4278ccd2dec190db28b78b0766b8865901
Author: SeongJae Park <[email protected]>

mm/damon/stat: add a parameter for reading kdamond pid

This commit exposes the pid of the DAMON_STAT kdamond via a new read-only module parameter named kdamond_pid, standardizing usage with other DAMON modules.

Quoted 4 lines
mm/damon/stat.c +0 -0
index 99ba346f9e325..4901e9a7c3398 100644

[ ... ]

Quoted 5 lines
mm/damon/stat.c +1 -0
@@ -260,6 +269,14 @@ static int damon_stat_start(void)
 		damon_stat_context = NULL;
 		return err;
 	}
+	kdamond_pid = damon_kdamond_pid(damon_stat_context);

mm/damon/stat.c static int damon_stat_start(void) · line 282

Quoted 10 lines
mm/damon/stat.c +7 -0
+	if (kdamond_pid < 0) {
+		err = kdamond_pid;
+		kdamond_pid = -1;
+		damon_destroy_ctx(damon_stat_context);
+		damon_stat_context = NULL;
+		return err;
+	}
 
 	damon_stat_last_refresh_jiffies = jiffies;
 	call_control.data = damon_stat_context;

If damon_call() fails right after this and returns an error, damon_stat_start() will pass that error back to its caller and the module will revert to a disabled state.

Quoted 7 lines
Quoted 4 lines

Does this leave a stale kdamond_pid exposed to userspace? The newly added documentation states: "If DAMON_STAT is enabled, this becomes the PID of the worker thread. Else, -1." Since the initialization fails, the module remains disabled, but kdamond_pid isn't reset to -1 on this error path.

Nice catch. I will do the kdamond_pid setup after damon_call() success, like below.

Wait... The same question was also raised [1] by Sashiko on the previous version. And I was wrongly saying it is not this patch's fault. That was completely wrong.

Quoted 6 lines

And the kdamond can stop even after the damon_kdamond_pid() call, if the region_store_histogram allocation fails. In the case, kdamond_pid will again be stale. 'enabled' parameter value will also be stale in the case. The worst part of it is that in the case, DAMON_STAT cannot be restarted before the system is reboot. The bug is similar to the one [2] that Liew found and working on.

The last two sentences are wrong... In case of damon_stat(), writing 'N' to 'enabled' will just update the parameter even if damon_stop() fails. Hence, users can restart DAMON_STAT after 'enabled' value became stale, by writing 'N' to 'enabled' parameter to reset the value, and then writing 'Y' to 'enabled' parameter again, to invoke damon_start() call. Still far from good user experience, but less critical than the bug that Liew is working on.

Quoted 2 lines

I will hold this patch and fix the bug first.

My above opinion is same. I will fix the bugs first.

Thanks,
SJ

[...]

author maintainer
#6 lore raw

RFC PATCH 2/2 Docs/admin-guide/mm/damon/stat: document kdamond_pid parameter

In reply to SeongJae Park

Update DAMON_STAT usage document for newly added kdamond_pid parameter.

Signed-off-by SeongJae Park <[email protected]>
---
 Documentation/admin-guide/mm/damon/stat.rst | 7 +++++++
 1 file changed, 7 insertions(+)
Documentation/admin-guide/mm/damon/stat.rst +7 -0
index c4b14daeb2dd6..46c5dd96aa2ed 100644
@@ -89,3 +89,10 @@ percentiles of the idle time values via this read-only parameter.  Reading the
 parameter returns 101 idle time values in milliseconds, separated by comma.
 Each value represents 0-th, 1st, 2nd, 3rd, ..., 99th and 100th percentile idle
 times.
+
+kdamond_pid
+-----------
+
+PID of the DAMON thread.
+
+If DAMON_STAT is enabled, this becomes the PID of the worker thread.  Else, -1.
Signature · 1 line
2.47.3