[PATCH] mm/damon: modified damon_call_control from static to kmalloc

[PATCH] mm/damon: modified damon_call_control from static to kmalloc

[JaeJoon Jung]

The damon_call_control structure is used by all damon modules.
In mm/damon/sysfs.c, it is dynamically allocated with kmalloc(), and in
the remaining modules, it is statically coded.  There are many daemon
modules and they are designed to be used only when needed.  It is more
efficient to use kmalloc dynamically whenever needed as in damon/sysfs.c.
Using damon_call_control dynamically and consistently has the advantage of
eliminating the dealloc_on_cancel member variable condition and reducing
code size.

Signed-off-by: JaeJoon Jung <rgbi3307@gmail.com>
---
 include/linux/damon.h |  2 --
 mm/damon/core.c       |  2 +-
 mm/damon/lru_sort.c   | 20 ++++++++++++--------
 mm/damon/reclaim.c    | 20 ++++++++++++--------
 mm/damon/stat.c       | 18 +++++++++++-------
 mm/damon/sysfs.c      |  1 -
 samples/damon/prcl.c  | 17 ++++++++++-------
 samples/damon/wsse.c  | 18 +++++++++++-------
 8 files changed, 57 insertions(+), 41 deletions(-)

diff --git a/include/linux/damon.h b/include/linux/damon.h
index cae8c613c5fc..7a4d7196cc75 100644
--- a/include/linux/damon.h
+++ b/include/linux/damon.h
@@ -636,7 +636,6 @@ struct damon_operations {
  * @data:		Data that will be passed to @fn.
  * @repeat:		Repeat invocations.
  * @return_code:	Return code from @fn invocation.
- * @dealloc_on_cancel:	De-allocate when canceled.
  *
  * Control damon_call(), which requests specific kdamond to invoke a given
  * function.  Refer to damon_call() for more details.
@@ -646,7 +645,6 @@ struct damon_call_control {
 	void *data;
 	bool repeat;
 	int return_code;
-	bool dealloc_on_cancel;
 /* private: internal use only */
 	/* informs if the kdamond finished handling of the request */
 	struct completion completion;
diff --git a/mm/damon/core.c b/mm/damon/core.c
index 109b050c795a..fb3f844d5da5 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -2519,7 +2519,7 @@ static void kdamond_call(struct damon_ctx *ctx, bool cancel)
 		mutex_unlock(&ctx->call_controls_lock);
 		if (!control->repeat) {
 			complete(&control->completion);
-		} else if (control->canceled && control->dealloc_on_cancel) {
+		} else if (control->canceled) {
 			kfree(control);
 			continue;
 		} else {
diff --git a/mm/damon/lru_sort.c b/mm/damon/lru_sort.c
index 42b9a656f9de..e3d6a46fa2fb 100644
--- a/mm/damon/lru_sort.c
+++ b/mm/damon/lru_sort.c
@@ -279,14 +279,10 @@ static int damon_lru_sort_damon_call_fn(void *arg)
 	return damon_lru_sort_handle_commit_inputs();
 }

-static struct damon_call_control call_control = {
-	.fn = damon_lru_sort_damon_call_fn,
-	.repeat = true,
-};
-
 static int damon_lru_sort_turn(bool on)
 {
 	int err;
+	struct damon_call_control *call_control;

 	if (!on) {
 		err = damon_stop(&ctx, 1);
@@ -302,8 +298,18 @@ static int damon_lru_sort_turn(bool on)
 	err = damon_start(&ctx, 1, true);
 	if (err)
 		return err;
+
 	kdamond_pid = ctx->kdamond->pid;
-	return damon_call(ctx, &call_control);
+
+	call_control = kmalloc(sizeof(*call_control), GFP_KERNEL);
+	if (!call_control)
+		return -ENOMEM;
+
+	call_control->fn = damon_lru_sort_damon_call_fn;
+	call_control->repeat = true;
+	call_control->data = ctx;
+
+	return damon_call(ctx, call_control);
 }

 static int damon_lru_sort_addr_unit_store(const char *val,
@@ -378,8 +384,6 @@ static int __init damon_lru_sort_init(void)
 	if (err)
 		goto out;

-	call_control.data = ctx;
-
 	/* 'enabled' has set before this function, probably via command line */
 	if (enabled)
 		err = damon_lru_sort_turn(true);
diff --git a/mm/damon/reclaim.c b/mm/damon/reclaim.c
index 7ba3d0f9a19a..967d06d938d6 100644
--- a/mm/damon/reclaim.c
+++ b/mm/damon/reclaim.c
@@ -283,14 +283,10 @@ static int damon_reclaim_damon_call_fn(void *arg)
 	return damon_reclaim_handle_commit_inputs();
 }

-static struct damon_call_control call_control = {
-	.fn = damon_reclaim_damon_call_fn,
-	.repeat = true,
-};
-
 static int damon_reclaim_turn(bool on)
 {
 	int err;
+	struct damon_call_control *call_control;

 	if (!on) {
 		err = damon_stop(&ctx, 1);
@@ -306,8 +302,18 @@ static int damon_reclaim_turn(bool on)
 	err = damon_start(&ctx, 1, true);
 	if (err)
 		return err;
+
 	kdamond_pid = ctx->kdamond->pid;
-	return damon_call(ctx, &call_control);
+
+	call_control = kmalloc(sizeof(*call_control), GFP_KERNEL);
+	if (!call_control)
+		return -ENOMEM;
+
+	call_control->fn = damon_reclaim_damon_call_fn;
+	call_control->repeat = true;
+	call_control->data = ctx;
+
+	return damon_call(ctx, call_control);
 }

 static int damon_reclaim_addr_unit_store(const char *val,
@@ -382,8 +388,6 @@ static int __init damon_reclaim_init(void)
 	if (err)
 		goto out;

-	call_control.data = ctx;
-
 	/* 'enabled' has set before this function, probably via command line */
 	if (enabled)
 		err = damon_reclaim_turn(true);
diff --git a/mm/damon/stat.c b/mm/damon/stat.c
index bf8626859902..6a21de62c3f4 100644
--- a/mm/damon/stat.c
+++ b/mm/damon/stat.c
@@ -196,14 +196,10 @@ static struct damon_ctx *damon_stat_build_ctx(void)
 	return NULL;
 }

-static struct damon_call_control call_control = {
-	.fn = damon_stat_damon_call_fn,
-	.repeat = true,
-};
-
 static int damon_stat_start(void)
 {
 	int err;
+	struct damon_call_control *call_control;

 	damon_stat_context = damon_stat_build_ctx();
 	if (!damon_stat_context)
@@ -213,8 +209,16 @@ static int damon_stat_start(void)
 		return err;

 	damon_stat_last_refresh_jiffies = jiffies;
-	call_control.data = damon_stat_context;
-	return damon_call(damon_stat_context, &call_control);
+
+	call_control = kmalloc(sizeof(*call_control), GFP_KERNEL);
+	if (!call_control)
+		return -ENOMEM;
+
+	call_control->fn = damon_stat_damon_call_fn;
+	call_control->repeat = true;
+	call_control->data = damon_stat_context;
+
+	return damon_call(damon_stat_context, call_control);
 }

 static void damon_stat_stop(void)
diff --git a/mm/damon/sysfs.c b/mm/damon/sysfs.c
index 3c0d727788c8..3cd0af16a92f 100644
--- a/mm/damon/sysfs.c
+++ b/mm/damon/sysfs.c
@@ -1614,7 +1614,6 @@ static int damon_sysfs_turn_damon_on(struct damon_sysfs_kdamond *kdamond)
 	repeat_call_control->fn = damon_sysfs_repeat_call_fn;
 	repeat_call_control->data = kdamond;
 	repeat_call_control->repeat = true;
-	repeat_call_control->dealloc_on_cancel = true;
 	damon_call(ctx, repeat_call_control);
 	return err;
 }
diff --git a/samples/damon/prcl.c b/samples/damon/prcl.c
index b7c50f2656ce..b71914549725 100644
--- a/samples/damon/prcl.c
+++ b/samples/damon/prcl.c
@@ -52,15 +52,11 @@ static int damon_sample_prcl_repeat_call_fn(void *data)
 	return 0;
 }

-static struct damon_call_control repeat_call_control = {
-	.fn = damon_sample_prcl_repeat_call_fn,
-	.repeat = true,
-};
-
 static int damon_sample_prcl_start(void)
 {
 	struct damon_target *target;
 	struct damos *scheme;
+	struct damon_call_control *call_control;
 	int err;

 	pr_info("start\n");
@@ -109,8 +105,15 @@ static int damon_sample_prcl_start(void)
 	if (err)
 		return err;

-	repeat_call_control.data = ctx;
-	return damon_call(ctx, &repeat_call_control);
+	call_control = kmalloc(sizeof(*call_control), GFP_KERNEL);
+	if (!call_control)
+		return -ENOMEM;
+
+	call_control->fn = damon_sample_prcl_repeat_call_fn;
+	call_control->repeat = true;
+	call_control->data = ctx;
+
+	return damon_call(ctx, call_control);
 }

 static void damon_sample_prcl_stop(void)
diff --git a/samples/damon/wsse.c b/samples/damon/wsse.c
index 799ad4443943..e9cbc4be3706 100644
--- a/samples/damon/wsse.c
+++ b/samples/damon/wsse.c
@@ -53,14 +53,10 @@ static int damon_sample_wsse_repeat_call_fn(void *data)
 	return 0;
 }

-static struct damon_call_control repeat_call_control = {
-	.fn = damon_sample_wsse_repeat_call_fn,
-	.repeat = true,
-};
-
 static int damon_sample_wsse_start(void)
 {
 	struct damon_target *target;
+	struct damon_call_control *call_control;
 	int err;

 	pr_info("start\n");
@@ -89,8 +85,16 @@ static int damon_sample_wsse_start(void)
 	err = damon_start(&ctx, 1, true);
 	if (err)
 		return err;
-	repeat_call_control.data = ctx;
-	return damon_call(ctx, &repeat_call_control);
+
+	call_control = kmalloc(sizeof(*call_control), GFP_KERNEL);
+	if (!call_control)
+		return -ENOMEM;
+
+	call_control->fn = damon_sample_wsse_repeat_call_fn;
+	call_control->repeat = true;
+	call_control->data = ctx;
+
+	return damon_call(ctx, call_control);
 }

 static void damon_sample_wsse_stop(void)
--
2.43.0

Re: [PATCH] mm/damon: modified damon_call_control from static to kmalloc

[SeongJae Park]

On Sun,  7 Dec 2025 07:47:23 +0900 JaeJoon Jung <rgbi3307@gmail.com> wrote:

> The damon_call_control structure is used by all damon modules.
> In mm/damon/sysfs.c, it is dynamically allocated with kmalloc(),

It is only for refresh_ms handling damon_call_control.
damon_sysfs_damon_call() doesn't use kmalloc().

> and in
> the remaining modules, it is statically coded.

I try not to use dynamic allocation when possible and *reasonable*, since doing
so eliminates the chances of bugs from dynamic allocation handling mistakes.

> There are many daemon
> modules and they are designed to be used only when needed.  It is more
> efficient to use kmalloc dynamically whenever needed as in damon/sysfs.c.

I agree that is more efficient in terms of kernel size.  Nevertheless, I don't
think it is a significant improvement, since it is only single data structure
per the DAMON module.  The total number of DAMON modules is only five.  Two
among those are samples.

> Using damon_call_control dynamically and consistently has the advantage of
> eliminating the dealloc_on_cancel member variable condition and reducing
> code size.

Disadvantage of this change is that it is adding four more dynamic allocations
that we need to cautiously maintain.  It is not significantly increasing the
maintenance overhead.  But, the advantage (kernel size reduction) also seems
only modest, if I'm not wrong.

So I'm sadly have to say I don't think this change is really needed.

Please let me know if I'm missing something, though.


Thanks,
SJ

[...]