mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] mm/damon/core: reject empty access pattern intervals on scheme commit
@ 2026-11-03 10:54 Xiaolei Wang (Kaihong)
  2026-11-03 11:44 ` Aveline Noir
  0 siblings, 1 reply; 2+ messages in thread
From: Xiaolei Wang (Kaihong) @ 2026-11-03 10:54 UTC (permalink / raw)
  To: sj, akpm, david, ljs, error27, xiaoleiwangxiaolei
  Cc: damon, linux-mm, linux-kernel, stable, Liam R. Howlett,
	Michal Hocko, Mike Rapoport, Suren Baghdasaryan, Vlastimil Babka

DAMOS schemes accept access pattern bounds with min > max (e.g.
min_sz_region > max_sz_region).  No input path validates the bounds:
damon_new_scheme() stores the pattern as is, and the sysfs ul_range
stores are plain kstrtoul().

At runtime __damos_valid_target() evaluates "min <= x <= max" for each
of the three pattern dimensions.  A reversed interval is an empty set,
so every region fails the check and the scheme silently never matches
anything: nr_tried stays zero forever while the kdamond keeps running
normally.

With such a reversed interval, damo starts normally (rc=0, kdamond
state=on), but nr_tried and nr_applied stay zero forever and the target
RSS does not drop.

The same holds for the watermarks thresholds, which must be monotonic
(low <= mid <= high).

Validate the access pattern and watermarks at the beginning of
damos_commit(), through which both new schemes (damo start) and online
updates (damo tune) pass.

Fixes: 1f366e421c8f ("mm/damon/core: implement DAMON-based Operation Schemes (DAMOS)")
Cc: stable@vger.kernel.org
Cc: Liam R. Howlett <liam@infradead.org>
Cc: Michal Hocko <mhocko@suse.com>
Cc: Mike Rapoport <rppt@kernel.org>
Cc: Suren Baghdasaryan <surenb@google.com>
Cc: Vlastimil Babka <vbabka@kernel.org>
Signed-off-by: Xiaolei Wang (Kaihong) <xiaoleiwangxiaolei@gmail.com>
---
 mm/damon/core.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/mm/damon/core.c b/mm/damon/core.c
index eb1fcdabc007..865627bda240 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -1566,10 +1566,29 @@ static struct damos *damon_nth_scheme(int n, struct damon_ctx *ctx)
 	return NULL;
 }
 
+static int damos_validate_scheme(struct damos *s)
+{
+	struct damos_access_pattern *p = &s->pattern;
+	struct damos_watermarks *wmarks = &s->wmarks;
+
+	if (p->min_sz_region > p->max_sz_region ||
+	    p->min_nr_accesses > p->max_nr_accesses ||
+	    p->min_age_region > p->max_age_region)
+		return -EINVAL;
+	if (wmarks->metric != DAMOS_WMARK_NONE &&
+	    (wmarks->low > wmarks->mid || wmarks->mid > wmarks->high))
+		return -EINVAL;
+	return 0;
+}
+
 static int damos_commit(struct damos *dst, struct damos *src)
 {
 	int err;
 
+	err = damos_validate_scheme(src);
+	if (err)
+		return err;
+
 	dst->pattern = src->pattern;
 	dst->action = src->action;
 	dst->apply_interval_us = src->apply_interval_us;
-- 
2.53.0


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

* Re: [PATCH] mm/damon/core: reject empty access pattern intervals on scheme commit
  2026-11-03 10:54 [PATCH] mm/damon/core: reject empty access pattern intervals on scheme commit Xiaolei Wang (Kaihong)
@ 2026-11-03 11:44 ` Aveline Noir
  0 siblings, 0 replies; 2+ messages in thread
From: Aveline Noir @ 2026-11-03 11:44 UTC (permalink / raw)
  To: Xiaolei Wang (Kaihong)
  Cc: akpm, damon, david, error27, Liam R. Howlett, linux-kernel,
	linux-mm, ljs, Michal Hocko, Mike Rapoport, sashiko-bot,
	sashiko-reviews, sj, stable, Suren Baghdasaryan, Vlastimil Babka

Hi Xiaolei,

On Tue, 03 Nov 2026 18:54:18 +0800 Xiaolei Wang (Kaihong) wrote:

> At runtime __damos_valid_target() evaluates "min <= x <= max" for each
> of the three pattern dimensions.  A reversed interval is an empty set,
> so every region fails the check and the scheme silently never matches
> anything: nr_tried stays zero forever while the kdamond keeps running
> normally.

My understanding is that DAMON just does what the user asked for here,
so this looks more like an input validation improvement than a bug fix.
Since previously accepted input would now fail with -EINVAL, maybe drop
the Fixes: and Cc: stable tags, and mention the behavior change in the
changelog?

> The same holds for the watermarks thresholds, which must be monotonic
> (low <= mid <= high).

Not for all cases, I think.  With mid > high (and low <= high), the
scheme still works; mid just has no effect.  The subject would also
need to mention watermarks.

> +	if (p->min_sz_region > p->max_sz_region ||
> +	    p->min_nr_accesses > p->max_nr_accesses ||
> +	    p->min_age_region > p->max_age_region)
> +		return -EINVAL;

As Sashiko pointed out, this would break the DAMON sysfs.py selftest,
which uses size=[4096, 2**10].  Maybe update it together, and also add
a kunit case and a note in the DAMON usage document?

Thanks,
Aveline Noir

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

end of thread, other threads:[~2026-11-03 11:56 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-11-03 10:54 [PATCH] mm/damon/core: reject empty access pattern intervals on scheme commit Xiaolei Wang (Kaihong)
2026-11-03 11:44 ` Aveline Noir

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

all inboxes | Powered by JetHome®