mm/damon/core.c | 2 ++ mm/damon/tests/core-kunit.h | 13 +++++++++++++ 2 files changed, 15 insertions(+)
damon_set_attrs() already rejects inverted intervals (sample_interval >
aggr_interval) and invalid region counts, but it accepts
sample_interval == 0.
A zero sampling interval is not a valid monitoring period. Sysfs "on"
and "commit" both install attrs via damon_set_attrs(), so kdamond then
calls kdamond_usleep(0) and busy-spins.
Return -EINVAL if sample_interval is zero.
Signed-off-by: Karthikeyan KS <karthiproffesional@gmail.com>
---
Hello,
I recently started reading DAMON and just found this one. This
change only tightens damon_set_attrs() validation, in the same place
as the existing sample > aggr and min_nr_regions checks. It is not a
crash, UAF, or race.
sample_us=0 is accepted today, so kdamond_usleep(0) busy-spins (about
one core). Sysfs "on" and "commit" both go through damon_set_attrs();
one check covers both.
Checked on v7.3-rc3 (238650ef6c7c):
unpatched: echo on with sample_us=0 succeeds; kdamond uses ~97% of
one core. echo commit of 0 onto a running kdamond also
succeeds and it starts spinning.
patched: both on and commit return -EINVAL. Valid sample_us=5000
still starts. After a rejected commit the same pid stays
on and does not spin.
aggr_us=0 with a positive sample_us is already -EINVAL from the
ordering check.
Thanks,
mm/damon/core.c | 2 ++
mm/damon/tests/core-kunit.h | 13 +++++++++++++
2 files changed, 15 insertions(+)
diff --git a/mm/damon/core.c b/mm/damon/core.c
index 644daf5a1656..376c3e2cbe00 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -1060,6 +1060,8 @@ int damon_set_attrs(struct damon_ctx *ctx, struct damon_attrs *attrs)
return -EINVAL;
if (attrs->min_nr_regions > attrs->max_nr_regions)
return -EINVAL;
+ if (!attrs->sample_interval)
+ return -EINVAL;
if (attrs->sample_interval > attrs->aggr_interval)
return -EINVAL;
diff --git a/mm/damon/tests/core-kunit.h b/mm/damon/tests/core-kunit.h
index 4a536d41cdb2..ec8ef71956c8 100644
--- a/mm/damon/tests/core-kunit.h
+++ b/mm/damon/tests/core-kunit.h
@@ -658,6 +658,19 @@ static void damon_test_set_attrs(struct kunit *test)
invalid_attrs.aggr_interval = 4999;
KUNIT_EXPECT_EQ(test, damon_set_attrs(c, &invalid_attrs), -EINVAL);
+ invalid_attrs = valid_attrs;
+ invalid_attrs.sample_interval = 0;
+ KUNIT_EXPECT_EQ(test, damon_set_attrs(c, &invalid_attrs), -EINVAL);
+
+ invalid_attrs = valid_attrs;
+ invalid_attrs.aggr_interval = 0;
+ KUNIT_EXPECT_EQ(test, damon_set_attrs(c, &invalid_attrs), -EINVAL);
+
+ invalid_attrs = valid_attrs;
+ invalid_attrs.sample_interval = 0;
+ invalid_attrs.aggr_interval = 0;
+ KUNIT_EXPECT_EQ(test, damon_set_attrs(c, &invalid_attrs), -EINVAL);
+
damon_destroy_ctx(c);
}
--
2.34.1Hi, On Fri, Sep 18, 2026 at 12:14 AM Karthikeyan KS <karthiproffesional@gmail.com> wrote: > > damon_set_attrs() already rejects inverted intervals (sample_interval > > aggr_interval) and invalid region counts, but it accepts > sample_interval == 0. > > A zero sampling interval is not a valid monitoring period. Sysfs "on" > and "commit" both install attrs via damon_set_attrs(), so kdamond then > calls kdamond_usleep(0) and busy-spins. > > Return -EINVAL if sample_interval is zero. I think SJ already commented on a similar patch. please refer to [1] If this happens a lot, maybe should we document or add a comment about this? [1] https://lore.kernel.org/all/20260722094304.3132750-1-dayou5941@163.com/ > Signed-off-by: Karthikeyan KS <karthiproffesional@gmail.com> > --- > > Hello, > > I recently started reading DAMON and just found this one. This > change only tightens damon_set_attrs() validation, in the same place > as the existing sample > aggr and min_nr_regions checks. It is not a > crash, UAF, or race. > > sample_us=0 is accepted today, so kdamond_usleep(0) busy-spins (about > one core). Sysfs "on" and "commit" both go through damon_set_attrs(); > one check covers both. > > Checked on v7.3-rc3 (238650ef6c7c): > > unpatched: echo on with sample_us=0 succeeds; kdamond uses ~97% of > one core. echo commit of 0 onto a running kdamond also > succeeds and it starts spinning. > > patched: both on and commit return -EINVAL. Valid sample_us=5000 > still starts. After a rejected commit the same pid stays > on and does not spin. > > aggr_us=0 with a positive sample_us is already -EINVAL from the > ordering check. > > Thanks, > > mm/damon/core.c | 2 ++ > mm/damon/tests/core-kunit.h | 13 +++++++++++++ > 2 files changed, 15 insertions(+) > > diff --git a/mm/damon/core.c b/mm/damon/core.c > index 644daf5a1656..376c3e2cbe00 100644 > --- a/mm/damon/core.c > +++ b/mm/damon/core.c > @@ -1060,6 +1060,8 @@ int damon_set_attrs(struct damon_ctx *ctx, struct damon_attrs *attrs) > return -EINVAL; > if (attrs->min_nr_regions > attrs->max_nr_regions) > return -EINVAL; > + if (!attrs->sample_interval) > + return -EINVAL; > if (attrs->sample_interval > attrs->aggr_interval) > return -EINVAL; > > diff --git a/mm/damon/tests/core-kunit.h b/mm/damon/tests/core-kunit.h > index 4a536d41cdb2..ec8ef71956c8 100644 > --- a/mm/damon/tests/core-kunit.h > +++ b/mm/damon/tests/core-kunit.h > @@ -658,6 +658,19 @@ static void damon_test_set_attrs(struct kunit *test) > invalid_attrs.aggr_interval = 4999; > KUNIT_EXPECT_EQ(test, damon_set_attrs(c, &invalid_attrs), -EINVAL); > > + invalid_attrs = valid_attrs; > + invalid_attrs.sample_interval = 0; > + KUNIT_EXPECT_EQ(test, damon_set_attrs(c, &invalid_attrs), -EINVAL); > + > + invalid_attrs = valid_attrs; > + invalid_attrs.aggr_interval = 0; > + KUNIT_EXPECT_EQ(test, damon_set_attrs(c, &invalid_attrs), -EINVAL); > + > + invalid_attrs = valid_attrs; > + invalid_attrs.sample_interval = 0; > + invalid_attrs.aggr_interval = 0; > + KUNIT_EXPECT_EQ(test, damon_set_attrs(c, &invalid_attrs), -EINVAL); > + > damon_destroy_ctx(c); > } > > -- > 2.34.1 > Best regards, Sang-Heon Jeon
Hi Sang-Heon, Hi SJ, > I think SJ already commented on a similar patch. please refer to [1] > > [1] https://lore.kernel.org/all/20260722094304.3132750-1-dayou5941@163.com/ Thanks, missed that thread. Same objection applies to mine. > Can't we keep supporting zero sample interval and fix the warning > or the real bug instead? I measured a few floors for kdamond_usleep(): floor kdamond CPU (3s) vs sample_us=5000 1us 84% 28x 100us 42% 14x 1000us 12% ~3x 5000us ~2% 1x (matches control) 5000us is damon_new_ctx()'s own default, and at that floor the spin is just gone. Would you rather we just document sample_us=0 as accepted/expected behavior, or clamp it to that default inside damon_set_attrs()? Thanks, Karthikeyan KS
Hi Karthikeyan and Sang-Heon, On Thu, 17 Sep 2026 18:00:13 +0000 Karthikeyan KS <karthiproffesional@gmail.com> wrote: > Hi Sang-Heon, Hi SJ, > > > I think SJ already commented on a similar patch. please refer to [1] > > > > [1] https://lore.kernel.org/all/20260722094304.3132750-1-dayou5941@163.com/ Thank you for pointing this out, Sang-Heon! > > Thanks, missed that thread. Same objection applies to mine. > > > Can't we keep supporting zero sample interval and fix the warning > > or the real bug instead? > > I measured a few floors for kdamond_usleep(): > > floor kdamond CPU (3s) vs sample_us=5000 > 1us 84% 28x > 100us 42% 14x > 1000us 12% ~3x > 5000us ~2% 1x (matches control) > > 5000us is damon_new_ctx()'s own default, and at that floor the spin > is just gone. > > Would you rather we just document sample_us=0 as accepted/expected > behavior, or clamp it to that default inside damon_set_attrs()? I'd prefer adding a comment saying zero sampling interval is accepted. I think damon_attr kernel-doc comment is a good place for adding that. Thanks, SJ [...]
damon_set_attrs() accepts sample_interval == 0. This was reported as
a bug in v1 of this patch (rejecting it in damon_set_attrs()). A
similar patch was already declined for the same reason: a zero
interval is intentionally supported [1].
Document the behavior instead of changing it.
[1] https://lore.kernel.org/all/20260722094304.3132750-1-dayou5941@163.com/
Signed-off-by: Karthikeyan KS <karthiproffesional@gmail.com>
---
Hi SJ,
> I'd prefer adding a comment saying zero sampling interval is
> accepted. I think damon_attr kernel-doc comment is a good place
> for adding that.
Adding comments instead of code changes. Let me know if you'd
rather the wording live somewhere else, or be phrased differently.
Changes from v1:
- Dropped the damon_set_attrs() rejection and the KUnit cases that
tested it.
- Added a kernel-doc note on struct damon_attrs's @sample_interval
instead.
Thanks,
Karthikeyan KS
include/linux/damon.h | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/include/linux/damon.h b/include/linux/damon.h
index 0c8b7ddef9ab..1a7ef416ec46 100644
--- a/include/linux/damon.h
+++ b/include/linux/damon.h
@@ -787,7 +787,8 @@ struct damon_probe {
/**
* struct damon_attrs - Monitoring attributes for accuracy/overhead control.
*
- * @sample_interval: The time between access samplings.
+ * @sample_interval: The time between access samplings. Zero is
+ * accepted.
* @aggr_interval: The time between monitor results aggregations.
* @ops_update_interval: The time between monitoring operations updates.
* @intervals_goal: Intervals auto-tuning goal.
--
2.34.1Hello Karthikeyan, From the next time, please don't post a new version of a patch as a reply to the thread. Post it as a new thread instead. Also please add a links to the previous revisions in the changelog [1]. On Fri, 18 Sep 2026 13:13:01 +0000 Karthikeyan KS <karthiproffesional@gmail.com> wrote: > damon_set_attrs() accepts sample_interval == 0. This was reported as > a bug in v1 of this patch (rejecting it in damon_set_attrs()). A > similar patch was already declined for the same reason: a zero > interval is intentionally supported [1]. > > Document the behavior instead of changing it. > > [1] https://lore.kernel.org/all/20260722094304.3132750-1-dayou5941@163.com/ The patch itself looks good to me. > > Signed-off-by: Karthikeyan KS <karthiproffesional@gmail.com> Reviewed-by: SJ Park <sj@kernel.org> This patch is applied to damon/next [2] tree. If this patch is not added to mm.git in short term (~1 week?), I will ask mm.git maintainer (Andrew Morton) to pick this. So, no action from your side is needed for now. If it seems I also forgot doing that or you cannot wait for my action, please feel free to directly ask that to Andrew. > --- > > Hi SJ, > > > I'd prefer adding a comment saying zero sampling interval is > > accepted. I think damon_attr kernel-doc comment is a good place > > for adding that. > > Adding comments instead of code changes. Let me know if you'd > rather the wording live somewhere else, or be phrased differently. > > Changes from v1: > - Dropped the damon_set_attrs() rejection and the KUnit cases that > tested it. > - Added a kernel-doc note on struct damon_attrs's @sample_interval > instead. Thank you for accepting my humble suggestion. [1] https://docs.kernel.org/process/submitting-patches.html#commentary [2] https://origin.kernel.org/doc/html/latest/mm/damon/maintainer-profile.html#scm-trees Thanks, SJ [...]
Hi SJ, hi Sang-Heon, Thanks -- appreciate the review, and the pointer to the July thread. Will post as a new thread with a link to the previous version next time. Thanks, Karthikeyan KS
© 2016 - 2026 Red Hat, Inc.