[PATCH] mm/damon: reject zero sampling interval

[PATCH] mm/damon: reject zero sampling interval

[Karthikeyan KS]

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.1

Re: [PATCH] mm/damon: reject zero sampling interval

[Sang-Heon Jeon]

Hi,

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

Re: [PATCH] mm/damon: reject zero sampling interval

[Karthikeyan KS]

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

Re: [PATCH] mm/damon: reject zero sampling interval

[SeongJae Park]

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

[...]

[PATCH v2] mm/damon: document that a zero sample_interval is accepted

[Karthikeyan KS]

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.1

Re: [PATCH v2] mm/damon: document that a zero sample_interval is accepted

[SJ Park]

Hello 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

[...]

Re: [PATCH v2] mm/damon: document that a zero sample_interval is accepted

[Karthikeyan KS]

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