* [PATCH] mm/damon/core: reject empty access pattern intervals on scheme commit
@ 2026-11-03 10:54 Xiaolei Wang (Kaihong)
2026-11-03 11:29 ` sashiko-bot
` (2 more replies)
0 siblings, 3 replies; 4+ 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 related [flat|nested] 4+ 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:29 ` sashiko-bot
2026-11-03 11:44 ` Aveline Noir
2026-11-03 14:53 ` SJ Park
2 siblings, 0 replies; 4+ messages in thread
From: sashiko-bot @ 2026-11-03 11:29 UTC (permalink / raw)
To: Xiaolei Wang (Kaihong), sj, akpm, david, ljs, error27
Cc: damon, linux-mm, Liam R. Howlett, Michal Hocko, Mike Rapoport,
Suren Baghdasaryan, Vlastimil Babka
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] mm/damon: damos_validate_scheme() breaks DAMON sysfs.py selftest
--
commit f1effb4d8401c0508f7cb04b14b9b55d0d6abd17
Author: Xiaolei Wang (Kaihong) <xiaoleiwangxiaolei@gmail.com>
mm/damon/core: reject empty access pattern intervals on scheme commit
This commit introduces damos_validate_scheme() and calls it at the start of
damos_commit() to reject DAMOS schemes with reversed access pattern bounds
(min > max) or non-monotonic watermark thresholds with -EINVAL.
> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index d36823f4d0928..9b2b2a63a2576 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
> @@ -1561,10 +1561,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;
[Severity: Medium]
Does this validation break the existing DAMON sysfs kselftest in
tools/testing/selftests/damon/sysfs.py?
In tools/testing/selftests/damon/sysfs.py, main() constructs a non-default
scheme for testing online parameter commitment with min_sz_region = 4096 and
max_sz_region = 1024:
tools/testing/selftests/damon/sysfs.py:main()
schemes=[_damon_sysfs.Damos(
action='pageout',
access_pattern=_damon_sysfs.DamosAccessPattern(
size=[4096, 2**10],
nr_accesses=[3, 317],
age=[5,71]),
When main() subsequently calls kdamonds.kdamonds[0].commit() and
assert_ctxs_committed(kdamonds), writing commit to the kdamond state sysfs
file invokes:
state_store()
-> damon_sysfs_handle_cmd()
-> damon_sysfs_commit_input()
-> damon_commit_ctx()
-> __damon_commit_ctx()
-> damon_commit_schemes()
-> damos_commit()
-> damos_validate_scheme()
Because p->min_sz_region (4096) > p->max_sz_region (1024),
damos_validate_scheme() returns -EINVAL, causing kd.commit() in
assert_ctxs_committed() to fail and exit with status 1:
tools/testing/selftests/damon/sysfs.py:assert_ctxs_committed()
if kd in kdamonds_paused_for_dump:
err = kd.commit()
if err is not None:
print('pause fail (%s)' % err)
kdamonds.stop()
exit(1)
Should tools/testing/selftests/damon/sysfs.py also be updated to use a
non-empty size range (for example, size=[4096, 2**20]) so the selftest
continues to pass?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261103105418.362459-1-xiaoleiwangxiaolei@gmail.com?part=1
^ permalink raw reply [flat|nested] 4+ 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:29 ` sashiko-bot
@ 2026-11-03 11:44 ` Aveline Noir
2026-11-03 14:53 ` SJ Park
2 siblings, 0 replies; 4+ 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] 4+ 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:29 ` sashiko-bot
2026-11-03 11:44 ` Aveline Noir
@ 2026-11-03 14:53 ` SJ Park
2 siblings, 0 replies; 4+ messages in thread
From: SJ Park @ 2026-11-03 14:53 UTC (permalink / raw)
To: Xiaolei Wang (Kaihong)
Cc: SJ Park, akpm, david, ljs, error27, damon, linux-mm, linux-kernel,
stable, Liam R. Howlett, Michal Hocko, Mike Rapoport,
Suren Baghdasaryan, Vlastimil Babka
On Tue, 3 Nov 2026 18:54:18 +0800 "Xiaolei Wang (Kaihong)" <xiaoleiwangxiaolei@gmail.com> wrote:
> 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.
The documentation is not explicitly mentioning about the inputs (min > max and
non-monotonic watermarks). But I was thinking it is not really difficult to
infer this behavior based on the existing document.
If my understanding of the documentation is not wrong, I find no problem at
accepting the inputs and just behave as documented. It might look silly, but
who knows the users have a creative reason to do that? As long as it is not
causing a harm, just supporting it is better for both users and code
maintainability, in my humble opinion.
Pleae let me know if I am missing something.
Thanks,
SJ
[...]
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-11-03 14:53 UTC | newest]
Thread overview: 4+ 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:29 ` sashiko-bot
2026-11-03 11:44 ` Aveline Noir
2026-11-03 14:53 ` SJ Park
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox