[RFC PATCH] mm/damon/lru_sort: add synchronous validation for commit_inputs

[RFC PATCH] mm/damon/lru_sort: add synchronous validation for commit_inputs

[Liew Rui Yan]

Hi SeongJae,

I would like to ask some question about the fourth task in discussion [1].

Please let me state first that the following changes will also apply to
the DAMON_RECLAIM module.

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() 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() to handle parameter
   validation before setting 'commit_inputs'.
2. Added damon_valid_attrs() to centralize validation logic.
3. Extracted validation checks from damon_lru_sort_apply_parameters()
   (currently copied from core.c).

Questions
=========
- Design:
  Is this synchronous validation approach acceptable? Or do you prefer
  keeping it purely asynchronous?

- Code Structure:
  I noticed damon_valid_intervals_goal() and similar validation logic
  exist in core.c but are static. To avoid duplication, should I:
  a) Export a generic validation function in damon.h or internal.h?
  b) Keep the copy in lru_sort.c?

- Concurrency:
  Are there any race conditions I should be aware of when reading
  'damon_lru_sort_mon_attrs' in the store function? Should I use
  READ_ONCE() or add explicit locking?

Thanks for your time reviewing this! :>

Best regards,
Rui Yan

[1] https://lore.kernel.org/20260319151528.86490-1-sj@kernel.org

Signed-off-by: Liew Rui Yan <aethernet65535@gmail.com>
---
 mm/damon/lru_sort.c | 64 ++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 63 insertions(+), 1 deletion(-)

diff --git a/mm/damon/lru_sort.c b/mm/damon/lru_sort.c
index 554559d72976..0e5f9d29e294 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,69 @@ static int damon_lru_sort_handle_commit_inputs(void)
 	return err;
 }
 
+/* Source: mm/damon/core.c */
+static bool damon_valid_intervals_goal(struct damon_attrs *attrs)
+{
+	struct damon_intervals_goal *goal = &attrs->intervals_goal;
+
+	/* tuning is disabled */
+	if (!goal->aggrs)
+		return true;
+	if (goal->min_sample_us > goal->max_sample_us)
+		return false;
+	if (attrs->sample_interval < goal->min_sample_us ||
+			goal->max_sample_us < attrs->sample_interval)
+		return false;
+	return true;
+}
+
+static int damon_valid_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;
+}
+
+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_valid_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;
-- 
2.53.0

Re: [RFC PATCH] mm/damon/lru_sort: add synchronous validation for commit_inputs

[SeongJae Park]

Hello Liew,

I saw the v2 [1] of this patch.  So I will skip reviewing this one.  Let me
know if I'm missing something.

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


Thanks,
SJ

[...]