public inbox for linux-mm@kvack.org
 help / color / mirror / Atom feed
* [RFC v2] mm/damon: add synchronous validation for commit_inputs
@ 2026-03-21 14:09 Liew Rui Yan
  2026-03-21 17:06 ` (Sashiko) " SeongJae Park
  2026-03-21 17:32 ` SeongJae Park
  0 siblings, 2 replies; 6+ messages in thread
From: Liew Rui Yan @ 2026-03-21 14:09 UTC (permalink / raw)
  To: sj; +Cc: damon, linux-mm, Liew Rui Yan

Problem
=======
Writing invalid parameters to sysfs followed by 'commit_inputs=Y' fails
silently (no error returned to shell), because the validation happens
aynchronously in the kdamond.

Solution
========
To fix this, I proposed adding synchronous validation in
damon_lru_sort_commit_inputs_store() and
damon_reclaim_commit_inputs_store() before setting the 'commit_inputs'
flag. This allow users to receive immediate feedback (e.g., -EINVAL) if
the parameters are invalid.

Changes
=======
1. Added damon_lru_sort_commit_inputs_store() and
   damon_reclaim_commit_inputs_store() to handle parameter validation
   before setting 'commit_inputs'.
2. Added damon_validate_attrs() to centralize validation logic.

This change is motivated from another discussion [1].

[1] https://lore.kernel.org/20260318153731.97470-1-aethernet65535@gmail.com

Signed-off-by: Liew Rui Yan <aethernet65535@gmail.com>
---
Changes from RFC-v1:
- Remove question from commit message area.
- Added synchronous validation for DAMON_RECLAIM.
- Rename damon_valid_attrs() -> damon_validate_attrs().
- Exported a new function damon_validate_attrs() and declared it in
  damon.h.
- Link to RFC-v1: https://lore.kernel.org/20260321002642.22712-1-aethernet65535@gmail.com

 include/linux/damon.h |  2 ++
 mm/damon/core.c       | 34 +++++++++++++++++++++++++---------
 mm/damon/lru_sort.c   | 34 +++++++++++++++++++++++++++++++++-
 mm/damon/reclaim.c    | 34 +++++++++++++++++++++++++++++++++-
 4 files changed, 93 insertions(+), 11 deletions(-)

diff --git a/include/linux/damon.h b/include/linux/damon.h
index d9a3babbafc1..00ca58dbe978 100644
--- a/include/linux/damon.h
+++ b/include/linux/damon.h
@@ -997,6 +997,8 @@ int damon_set_region_biggest_system_ram_default(struct damon_target *t,
 				unsigned long addr_unit,
 				unsigned long min_region_sz);
 
+int damon_validate_attrs(struct damon_attrs *attrs);
+
 #endif	/* CONFIG_DAMON */
 
 #endif	/* _DAMON_H */
diff --git a/mm/damon/core.c b/mm/damon/core.c
index 4dae03a37f9a..a459cdcaaed2 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -751,6 +751,27 @@ static bool damon_valid_intervals_goal(struct damon_attrs *attrs)
 	return true;
 }
 
+/**
+ * damon_validate_attrs() - Check if the given DAMON attributes are valid.
+ * @attrs:		attributes to be checked
+ *
+ * Return: 0 if valid, negative error code otherwise.
+ */
+int damon_validate_attrs(struct damon_attrs *attrs)
+{
+	if (!damon_valid_intervals_goal(attrs))
+		return -EINVAL;
+	if (attrs->min_nr_regions < 3)
+		return -EINVAL;
+	if (attrs->min_nr_regions > attrs->max_nr_regions)
+		return -EINVAL;
+	if (attrs->sample_interval > attrs->aggr_interval)
+		return -EINVAL;
+
+	return 0;
+}
+EXPORT_SYMBOL(damon_validate_attrs);
+
 /**
  * damon_set_attrs() - Set attributes for the monitoring.
  * @ctx:		monitoring context
@@ -778,16 +799,11 @@ int damon_set_attrs(struct damon_ctx *ctx, struct damon_attrs *attrs)
 	struct damos *s;
 	bool aggregating = ctx->passed_sample_intervals <
 		ctx->next_aggregation_sis;
+	int err;
 
-	if (!damon_valid_intervals_goal(attrs))
-		return -EINVAL;
-
-	if (attrs->min_nr_regions < 3)
-		return -EINVAL;
-	if (attrs->min_nr_regions > attrs->max_nr_regions)
-		return -EINVAL;
-	if (attrs->sample_interval > attrs->aggr_interval)
-		return -EINVAL;
+	err = damon_validate_attrs(attrs);
+	if (err)
+		return err;
 
 	/* calls from core-external doesn't set this. */
 	if (!attrs->aggr_samples)
diff --git a/mm/damon/lru_sort.c b/mm/damon/lru_sort.c
index 554559d72976..883860721d3b 100644
--- a/mm/damon/lru_sort.c
+++ b/mm/damon/lru_sort.c
@@ -39,7 +39,6 @@ static bool enabled __read_mostly;
  * the re-reading, DAMON_LRU_SORT will be disabled.
  */
 static bool commit_inputs __read_mostly;
-module_param(commit_inputs, bool, 0600);
 
 /*
  * Desired active to [in]active memory ratio in bp (1/10,000).
@@ -361,6 +360,39 @@ static int damon_lru_sort_handle_commit_inputs(void)
 	return err;
 }
 
+static int damon_lru_sort_commit_inputs_store(const char *val,
+		const struct kernel_param *kp)
+{
+	struct damon_attrs attrs;
+	bool yes;
+	int err;
+
+	err = kstrtobool(val, &yes);
+	if (err)
+		return err;
+
+	if (commit_inputs == yes)
+		return 0;
+
+	if (!yes)
+		return 0;
+
+	attrs = damon_lru_sort_mon_attrs;
+	err = damon_validate_attrs(&attrs);
+	if (err)
+		return err;
+
+	commit_inputs = yes;
+	return err;
+}
+
+static const struct kernel_param_ops commit_inputs_param_ops = {
+	.set = damon_lru_sort_commit_inputs_store,
+	.get = param_get_bool,
+};
+
+module_param_cb(commit_inputs, &commit_inputs_param_ops, &commit_inputs, 0600);
+
 static int damon_lru_sort_damon_call_fn(void *arg)
 {
 	struct damon_ctx *c = arg;
diff --git a/mm/damon/reclaim.c b/mm/damon/reclaim.c
index 86da14778658..6ea44b02cc76 100644
--- a/mm/damon/reclaim.c
+++ b/mm/damon/reclaim.c
@@ -39,7 +39,6 @@ static bool enabled __read_mostly;
  * re-reading, DAMON_RECLAIM will be disabled.
  */
 static bool commit_inputs __read_mostly;
-module_param(commit_inputs, bool, 0600);
 
 /*
  * Time threshold for cold memory regions identification in microseconds.
@@ -267,6 +266,39 @@ static int damon_reclaim_handle_commit_inputs(void)
 	return err;
 }
 
+static int damon_reclaim_commit_inputs_store(const char *val,
+		const struct kernel_param *kp)
+{
+	struct damon_attrs attrs;
+	bool yes;
+	int err;
+
+	err = kstrtobool(val, &yes);
+	if (err)
+		return err;
+
+	if (commit_inputs == yes)
+		return 0;
+
+	if (!yes)
+		return 0;
+
+	attrs = damon_reclaim_mon_attrs;
+	err = damon_validate_attrs(&attrs);
+	if (err)
+		return err;
+
+	commit_inputs = yes;
+	return err;
+}
+
+static const struct kernel_param_ops commit_inputs_param_ops = {
+	.set = damon_reclaim_commit_inputs_store,
+	.get = param_get_bool,
+};
+
+module_param_cb(commit_inputs, &commit_inputs_param_ops, &commit_inputs, 0600);
+
 static int damon_reclaim_damon_call_fn(void *arg)
 {
 	struct damon_ctx *c = arg;
-- 
2.53.0



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

* (Sashiko) Re: [RFC v2] mm/damon: add synchronous validation for commit_inputs
  2026-03-21 14:09 [RFC v2] mm/damon: add synchronous validation for commit_inputs Liew Rui Yan
@ 2026-03-21 17:06 ` SeongJae Park
  2026-03-21 17:40   ` SeongJae Park
  2026-03-21 17:32 ` SeongJae Park
  1 sibling, 1 reply; 6+ messages in thread
From: SeongJae Park @ 2026-03-21 17:06 UTC (permalink / raw)
  To: Liew Rui Yan; +Cc: SeongJae Park, damon, linux-mm

Forwarding Sashiko review for doing the discussion on the mailing list.

# review url: https://sashiko.dev/#/patchset/20260321140926.22163-1-aethernet65535@gmail.com
# start of sashiko.dev inline review
commit 05e8b12e4fe302bfaf2b30ee9c0d38b036fc1c5e
Author: Liew Rui Yan <aethernet65535@gmail.com>
Subject: mm/damon: add synchronous validation for commit_inputs

This commit adds synchronous validation to the sysfs commit_inputs parameter
for the DAMON lru_sort and reclaim modules. It aims to return immediate
feedback to userspace if the provided monitoring attributes are invalid,
rather than failing silently in the background.

Link: https://lore.kernel.org/20260318153731.97470-1-aethernet65535@gmail.com

> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index db6c67e52d2b..22136486e745 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
> @@ -751,6 +751,27 @@ static bool damon_valid_intervals_goal(struct damon_attrs *attrs)
>  	return true;
>  }
>  
> +/**
> + * damon_validate_attrs() - Check if the given DAMON attributes are valid.
> + * @attrs:		attributes to be checked
> + *
> + * Return: 0 if valid, negative error code otherwise.
> + */
> +int damon_validate_attrs(struct damon_attrs *attrs)
> +{
> +	if (!damon_valid_intervals_goal(attrs))
> +		return -EINVAL;
> +	if (attrs->min_nr_regions < 3)
> +		return -EINVAL;
> +	if (attrs->min_nr_regions > attrs->max_nr_regions)
> +		return -EINVAL;
> +	if (attrs->sample_interval > attrs->aggr_interval)
> +		return -EINVAL;

Does this validation fully prevent silent asynchronous failures?

If a user writes 0 to sample_interval or aggr_interval, the check 
attrs->sample_interval > attrs->aggr_interval will pass (since 0 > 0 is
false), returning success to the shell.

Since the daemon's actual application logic requires non-zero intervals,
will this still fail silently in the background for zero values?

> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(damon_validate_attrs);

Is this EXPORT_SYMBOL() necessary? 

Since both DAMON_RECLAIM and DAMON_LRU_SORT are built into the kernel and
cannot be compiled as loadable modules, does this need to be exported to the
global symbol table? None of the other DAMON core functions in this file
appear to be exported for this reason.

[ ... ]
> diff --git a/mm/damon/lru_sort.c b/mm/damon/lru_sort.c
> index 554559d72976..883860721d3b 100644
> --- a/mm/damon/lru_sort.c
> +++ b/mm/damon/lru_sort.c
[ ... ]
> @@ -361,6 +360,39 @@ static int damon_lru_sort_handle_commit_inputs(void)
>  	return err;
>  }
>  
> +static int damon_lru_sort_commit_inputs_store(const char *val,
> +		const struct kernel_param *kp)
> +{
> +	struct damon_attrs attrs;
> +	bool yes;
> +	int err;
> +
> +	err = kstrtobool(val, &yes);
> +	if (err)
> +		return err;
> +
> +	if (commit_inputs == yes)
> +		return 0;
> +
> +	if (!yes)
> +		return 0;

If a user writes 'N' to the sysfs parameter, yes evaluates to false and this
returns success without updating commit_inputs.

If commit_inputs was previously 'Y' (waiting for kdamond to process it),
does this prevent administrators from aborting a pending parameter commit?

> +
> +	attrs = damon_lru_sort_mon_attrs;
> +	err = damon_validate_attrs(&attrs);

Will this synchronous validation reject valid configurations if
autotune_monitoring_intervals is enabled?

The actual parameter application logic in damon_lru_sort_apply_parameters()
overwrites sample_interval and aggr_interval with hardcoded defaults when
autotuning is on. 

Because this validation happens before those overrides are applied, will a
user who leaves sample_interval > aggr_interval receive a false -EINVAL error
even though the configuration would be safely overridden?

> +	if (err)
> +		return err;
> +
> +	commit_inputs = yes;
> +	return err;
> +}
[ ... ]
> diff --git a/mm/damon/reclaim.c b/mm/damon/reclaim.c
> index 86da14778658..6ea44b02cc76 100644
> --- a/mm/damon/reclaim.c
> +++ b/mm/damon/reclaim.c
[ ... ]
> @@ -267,6 +266,39 @@ static int damon_reclaim_handle_commit_inputs(void)
>  	return err;
>  }
>  
> +static int damon_reclaim_commit_inputs_store(const char *val,
> +		const struct kernel_param *kp)
> +{
> +	struct damon_attrs attrs;
> +	bool yes;
> +	int err;
> +
> +	err = kstrtobool(val, &yes);
> +	if (err)
> +		return err;
> +
> +	if (commit_inputs == yes)
> +		return 0;
> +
> +	if (!yes)
> +		return 0;

Similar to the lru_sort module, does returning 0 here without clearing
commit_inputs prevent users from aborting a pending commit by writing 'N'?

> +
> +	attrs = damon_reclaim_mon_attrs;
> +	err = damon_validate_attrs(&attrs);
> +	if (err)
> +		return err;
> +
> +	commit_inputs = yes;
> +	return err;
> +}
[ ... ]


# end of sashiko.dev inline review
# review url: https://sashiko.dev/#/patchset/20260321140926.22163-1-aethernet65535@gmail.com


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

* Re: [RFC v2] mm/damon: add synchronous validation for commit_inputs
  2026-03-21 14:09 [RFC v2] mm/damon: add synchronous validation for commit_inputs Liew Rui Yan
  2026-03-21 17:06 ` (Sashiko) " SeongJae Park
@ 2026-03-21 17:32 ` SeongJae Park
  2026-03-22  6:06   ` Liew Rui Yan
  1 sibling, 1 reply; 6+ messages in thread
From: SeongJae Park @ 2026-03-21 17:32 UTC (permalink / raw)
  To: Liew Rui Yan; +Cc: SeongJae Park, damon, linux-mm

Hello Liew,

On Sat, 21 Mar 2026 22:09:26 +0800 Liew Rui Yan <aethernet65535@gmail.com> wrote:

> Problem
> =======
> Writing invalid parameters to sysfs followed by 'commit_inputs=Y' fails
> silently (no error returned to shell), because the validation happens
> aynchronously in the kdamond.
> 
> Solution
> ========
> To fix this, I proposed adding synchronous validation in
> damon_lru_sort_commit_inputs_store() and
> damon_reclaim_commit_inputs_store() before setting the 'commit_inputs'
> flag. This allow users to receive immediate feedback (e.g., -EINVAL) if
> the parameters are invalid.
> 
> Changes
> =======
> 1. Added damon_lru_sort_commit_inputs_store() and
>    damon_reclaim_commit_inputs_store() to handle parameter validation
>    before setting 'commit_inputs'.

Nice idea.  I think we could do even applying parameters in the callback.  It
could be something similar to what DAMON_SYSFS is doing.  The code path for
doing that in DAMON_SYSFS is, state_store() -> damon_sysfs_handle_cmd() ->
damon_sysfs_damon_call() -> damon_call() -> damon_sysfs_commit_input().  In the
damon_sysfs_commit_input(), you can show how DAMON_SYSFS validates input.  You
can also refer to commit 4c9ea539ad59 ("mm/damon/sysfs: validate user inputs
from damon_sysfs_commit_input()") for more details.

> 2. Added damon_validate_attrs() to centralize validation logic.

I think damon_commit_ctx() is a better way for validation.

> 
> This change is motivated from another discussion [1].
> 
> [1] https://lore.kernel.org/20260318153731.97470-1-aethernet65535@gmail.com
> 
> Signed-off-by: Liew Rui Yan <aethernet65535@gmail.com>

Because this is an RFC and I have a high level comment above, I'm skipping
detailed review of the code.


Thanks,
SJ

[...]


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

* Re: (Sashiko) Re: [RFC v2] mm/damon: add synchronous validation for commit_inputs
  2026-03-21 17:06 ` (Sashiko) " SeongJae Park
@ 2026-03-21 17:40   ` SeongJae Park
  0 siblings, 0 replies; 6+ messages in thread
From: SeongJae Park @ 2026-03-21 17:40 UTC (permalink / raw)
  To: SeongJae Park; +Cc: Liew Rui Yan, damon, linux-mm

On Sat, 21 Mar 2026 10:06:22 -0700 SeongJae Park <sj@kernel.org> wrote:

> Forwarding Sashiko review for doing the discussion on the mailing list.
> 
> # review url: https://sashiko.dev/#/patchset/20260321140926.22163-1-aethernet65535@gmail.com
> # start of sashiko.dev inline review
> commit 05e8b12e4fe302bfaf2b30ee9c0d38b036fc1c5e
> Author: Liew Rui Yan <aethernet65535@gmail.com>
> Subject: mm/damon: add synchronous validation for commit_inputs
> 
> This commit adds synchronous validation to the sysfs commit_inputs parameter
> for the DAMON lru_sort and reclaim modules. It aims to return immediate
> feedback to userspace if the provided monitoring attributes are invalid,
> rather than failing silently in the background.
> 
> Link: https://lore.kernel.org/20260318153731.97470-1-aethernet65535@gmail.com
> 
> > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > index db6c67e52d2b..22136486e745 100644
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
> > @@ -751,6 +751,27 @@ static bool damon_valid_intervals_goal(struct damon_attrs *attrs)
> >  	return true;
> >  }
> >  
> > +/**
> > + * damon_validate_attrs() - Check if the given DAMON attributes are valid.
> > + * @attrs:		attributes to be checked
> > + *
> > + * Return: 0 if valid, negative error code otherwise.
> > + */
> > +int damon_validate_attrs(struct damon_attrs *attrs)
> > +{
> > +	if (!damon_valid_intervals_goal(attrs))
> > +		return -EINVAL;
> > +	if (attrs->min_nr_regions < 3)
> > +		return -EINVAL;
> > +	if (attrs->min_nr_regions > attrs->max_nr_regions)
> > +		return -EINVAL;
> > +	if (attrs->sample_interval > attrs->aggr_interval)
> > +		return -EINVAL;
> 
> Does this validation fully prevent silent asynchronous failures?
> 
> If a user writes 0 to sample_interval or aggr_interval, the check 
> attrs->sample_interval > attrs->aggr_interval will pass (since 0 > 0 is
> false), returning success to the shell.
> 
> Since the daemon's actual application logic requires non-zero intervals,
> will this still fail silently in the background for zero values?

Zero sample_interval and aggr_interval are valid DAMON parameters.  DAMON logic
is written to allow that.  If not, that's a bug or wrong design.  Please let me
know if you find that.  So this should be fine.

> 
> > +
> > +	return 0;
> > +}
> > +EXPORT_SYMBOL(damon_validate_attrs);
> 
> Is this EXPORT_SYMBOL() necessary? 
> 
> Since both DAMON_RECLAIM and DAMON_LRU_SORT are built into the kernel and
> cannot be compiled as loadable modules, does this need to be exported to the
> global symbol table? None of the other DAMON core functions in this file
> appear to be exported for this reason.

I don't think it is necessary.  I also suggested to just not expose this
function to in-kernel DAMON core API callers in the high level review.

> 
> [ ... ]
> > diff --git a/mm/damon/lru_sort.c b/mm/damon/lru_sort.c
> > index 554559d72976..883860721d3b 100644
> > --- a/mm/damon/lru_sort.c
> > +++ b/mm/damon/lru_sort.c
> [ ... ]
> > @@ -361,6 +360,39 @@ static int damon_lru_sort_handle_commit_inputs(void)
> >  	return err;
> >  }
> >  
> > +static int damon_lru_sort_commit_inputs_store(const char *val,
> > +		const struct kernel_param *kp)
> > +{
> > +	struct damon_attrs attrs;
> > +	bool yes;
> > +	int err;
> > +
> > +	err = kstrtobool(val, &yes);
> > +	if (err)
> > +		return err;
> > +
> > +	if (commit_inputs == yes)
> > +		return 0;
> > +
> > +	if (!yes)
> > +		return 0;
> 
> If a user writes 'N' to the sysfs parameter, yes evaluates to false and this
> returns success without updating commit_inputs.
> 
> If commit_inputs was previously 'Y' (waiting for kdamond to process it),
> does this prevent administrators from aborting a pending parameter commit?
> 
> > +
> > +	attrs = damon_lru_sort_mon_attrs;
> > +	err = damon_validate_attrs(&attrs);
> 
> Will this synchronous validation reject valid configurations if
> autotune_monitoring_intervals is enabled?
> 
> The actual parameter application logic in damon_lru_sort_apply_parameters()
> overwrites sample_interval and aggr_interval with hardcoded defaults when
> autotuning is on. 
> 
> Because this validation happens before those overrides are applied, will a
> user who leaves sample_interval > aggr_interval receive a false -EINVAL error
> even though the configuration would be safely overridden?

I haven't think thoroughly about this.  But I think synchronously doing not
only validation but also commit, as I suggested on the high level review will
solve these complex problems.

> 
> > +	if (err)
> > +		return err;
> > +
> > +	commit_inputs = yes;
> > +	return err;
> > +}
> [ ... ]
> > diff --git a/mm/damon/reclaim.c b/mm/damon/reclaim.c
> > index 86da14778658..6ea44b02cc76 100644
> > --- a/mm/damon/reclaim.c
> > +++ b/mm/damon/reclaim.c
> [ ... ]
> > @@ -267,6 +266,39 @@ static int damon_reclaim_handle_commit_inputs(void)
> >  	return err;
> >  }
> >  
> > +static int damon_reclaim_commit_inputs_store(const char *val,
> > +		const struct kernel_param *kp)
> > +{
> > +	struct damon_attrs attrs;
> > +	bool yes;
> > +	int err;
> > +
> > +	err = kstrtobool(val, &yes);
> > +	if (err)
> > +		return err;
> > +
> > +	if (commit_inputs == yes)
> > +		return 0;
> > +
> > +	if (!yes)
> > +		return 0;
> 
> Similar to the lru_sort module, does returning 0 here without clearing
> commit_inputs prevent users from aborting a pending commit by writing 'N'?

Again, I believe doing both validation and commit synchronously will eliminate
these complexity.

> 
> > +
> > +	attrs = damon_reclaim_mon_attrs;
> > +	err = damon_validate_attrs(&attrs);
> > +	if (err)
> > +		return err;
> > +
> > +	commit_inputs = yes;
> > +	return err;
> > +}
> [ ... ]
> 
> 
> # end of sashiko.dev inline review
> # review url: https://sashiko.dev/#/patchset/20260321140926.22163-1-aethernet65535@gmail.com

FYI,

hkml [1] generated a draft of this mail using below command:

    hkml patch sashiko_dev --for_forwarding 20260321140926.22163-1-aethernet65535@gmail.com

[1] https://github.com/sjp38/hackermail


Thanks,
SJ


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

* Re: [RFC v2] mm/damon: add synchronous validation for commit_inputs
  2026-03-21 17:32 ` SeongJae Park
@ 2026-03-22  6:06   ` Liew Rui Yan
  2026-03-22 15:37     ` SeongJae Park
  0 siblings, 1 reply; 6+ messages in thread
From: Liew Rui Yan @ 2026-03-22  6:06 UTC (permalink / raw)
  To: sj; +Cc: aethernet65535, damon, linux-mm

Hi SeongJae,

I tried implementing the synchronous commit using damon_call() as
suggested, similar to how damon_sysfs_commit_input() works. This
successfully returns errors to userspace immediately instead of failing
silently.

However, I observed a side effect during testing:
Since damon_call() waits for the kdamond thread to process the request,
the latency of writing to 'commit_inputs' depends on the kdamond's
wake-up interval (controlled by damos_watermarks.interval).

In my test with DAMON_LRU_SORT:
- With '.interval=5s', the write latency can be up to ~5 seconds.
- When I temporarily increase '.interval=50s' for testing, the latency
  increased proportionally.

I understand this is expected behavior for synchronous communication
with a sleeping kernel thread. However, since 'commit_inputs' is a
control interface rather than a hot path, I wanted to comfirm:

Is this level of latency acceptable for the 'commit_inputs' parameter?
Or should we consider waking up the kdamond thread immediately upon
receiving a damon_call() request to reduce the worst-case latency?

For reference, DAMON_SYSFS seems to have similar latency
charactheristics when using damon_call().

Thank you for you high-level comments and the suggestion! :>

Best regards,
Rui Yan


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

* Re: [RFC v2] mm/damon: add synchronous validation for commit_inputs
  2026-03-22  6:06   ` Liew Rui Yan
@ 2026-03-22 15:37     ` SeongJae Park
  0 siblings, 0 replies; 6+ messages in thread
From: SeongJae Park @ 2026-03-22 15:37 UTC (permalink / raw)
  To: Liew Rui Yan; +Cc: SeongJae Park, damon, linux-mm

On Sun, 22 Mar 2026 14:06:30 +0800 Liew Rui Yan <aethernet65535@gmail.com> wrote:

> Hi SeongJae,
> 
> I tried implementing the synchronous commit using damon_call() as
> suggested, similar to how damon_sysfs_commit_input() works. This
> successfully returns errors to userspace immediately instead of failing
> silently.

Nice.

> 
> However, I observed a side effect during testing:
> Since damon_call() waits for the kdamond thread to process the request,
> the latency of writing to 'commit_inputs' depends on the kdamond's
> wake-up interval (controlled by damos_watermarks.interval).
> 
> In my test with DAMON_LRU_SORT:
> - With '.interval=5s', the write latency can be up to ~5 seconds.
> - When I temporarily increase '.interval=50s' for testing, the latency
>   increased proportionally.
> 
> I understand this is expected behavior for synchronous communication
> with a sleeping kernel thread. However, since 'commit_inputs' is a
> control interface rather than a hot path, I wanted to comfirm:
> 
> Is this level of latency acceptable for the 'commit_inputs' parameter?
> Or should we consider waking up the kdamond thread immediately upon
> receiving a damon_call() request to reduce the worst-case latency?

I believe this level of latency is acceptable.  The special-purpose DAMON
modules are designed for long term use with minimum control.  So I expect
commit_inputs to be used only occasionally in real use case.

Of course, making it faster would be nice, as long as the required change is
very simple.  I have no good ideea about making it really simple, though.
Nonetheless, I think it is not too late to do that after someone starts
complaining, or we find a really good idea.

> 
> For reference, DAMON_SYSFS seems to have similar latency
> charactheristics when using damon_call().

You are right.  And we got no complain about it so far, so I believe that
latency for DAMON_RECLAIM and DAMON_LRU_SORT should be fine.

> 
> Thank you for you high-level comments and the suggestion! :>

My pleasure :)


Thanks,
SJ

[...]


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

end of thread, other threads:[~2026-03-22 15:38 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-21 14:09 [RFC v2] mm/damon: add synchronous validation for commit_inputs Liew Rui Yan
2026-03-21 17:06 ` (Sashiko) " SeongJae Park
2026-03-21 17:40   ` SeongJae Park
2026-03-21 17:32 ` SeongJae Park
2026-03-22  6:06   ` Liew Rui Yan
2026-03-22 15:37     ` SeongJae Park

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