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

[Liew Rui Yan <aethernet65535@gmail.com>]

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