[PATCH] mm/damon/core: eliminate hot-path integer division in damon_max_nr_accesses()

[PATCH] mm/damon/core: eliminate hot-path integer division in damon_max_nr_accesses()

[Josh Law]

Hardware integer division is slow. The function damon_max_nr_accesses(),
which is called very frequently, performs an integer division.
However, the struct damon_attrs already caches this exact ratio in the
internal field aggr_samples. We can eliminate the hardware division in
the hot path by simply returning aggr_samples.

Signed-off-by: Josh Law <objecting@objecting.org>
---
 include/linux/damon.h       |  3 +--
 mm/damon/core.c             |  1 +
 mm/damon/tests/core-kunit.h | 16 ++++++++++++----
 3 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/include/linux/damon.h b/include/linux/damon.h
index 6bd71546f7b2..438fe6f3eab4 100644
--- a/include/linux/damon.h
+++ b/include/linux/damon.h
@@ -960,8 +960,7 @@ static inline bool damon_target_has_pid(const struct damon_ctx *ctx)
 static inline unsigned int damon_max_nr_accesses(const struct damon_attrs *attrs)
 {
 	/* {aggr,sample}_interval are unsigned long, hence could overflow */
-	return min(attrs->aggr_interval / attrs->sample_interval,
-			(unsigned long)UINT_MAX);
+	return min_t(unsigned long, attrs->aggr_samples, UINT_MAX);
 }
 
 
diff --git a/mm/damon/core.c b/mm/damon/core.c
index b0ab0ee6eab9..59b709f04975 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -582,6 +582,7 @@ struct damon_ctx *damon_new_ctx(void)
 	ctx->attrs.sample_interval = 5 * 1000;
 	ctx->attrs.aggr_interval = 100 * 1000;
 	ctx->attrs.ops_update_interval = 60 * 1000 * 1000;
+	ctx->attrs.aggr_samples = 20;
 
 	ctx->passed_sample_intervals = 0;
 	/* These will be set from kdamond_init_ctx() */
diff --git a/mm/damon/tests/core-kunit.h b/mm/damon/tests/core-kunit.h
index e86d4f4fe261..56d03ef6a5a4 100644
--- a/mm/damon/tests/core-kunit.h
+++ b/mm/damon/tests/core-kunit.h
@@ -416,6 +416,8 @@ static void damon_test_nr_accesses_to_accesses_bp(struct kunit *test)
 		.aggr_interval = ((unsigned long)UINT_MAX + 1) * 10
 	};
 
+	attrs.aggr_samples = attrs.aggr_interval / attrs.sample_interval;
+
 	/*
 	 * In some cases such as 32bit architectures where UINT_MAX is
 	 * ULONG_MAX, attrs.aggr_interval becomes zero.  Calling
@@ -434,7 +436,8 @@ static void damon_test_nr_accesses_to_accesses_bp(struct kunit *test)
 static void damon_test_update_monitoring_result(struct kunit *test)
 {
 	struct damon_attrs old_attrs = {
-		.sample_interval = 10, .aggr_interval = 1000,};
+		.sample_interval = 10, .aggr_interval = 1000,
+		.aggr_samples = 100,};
 	struct damon_attrs new_attrs;
 	struct damon_region *r = damon_new_region(3, 7);
 
@@ -446,19 +449,24 @@ static void damon_test_update_monitoring_result(struct kunit *test)
 	r->age = 20;
 
 	new_attrs = (struct damon_attrs){
-		.sample_interval = 100, .aggr_interval = 10000,};
+		.sample_interval = 100, .aggr_interval = 10000,
+		.aggr_samples = 100,};
 	damon_update_monitoring_result(r, &old_attrs, &new_attrs, false);
 	KUNIT_EXPECT_EQ(test, r->nr_accesses, 15);
 	KUNIT_EXPECT_EQ(test, r->age, 2);
 
 	new_attrs = (struct damon_attrs){
-		.sample_interval = 1, .aggr_interval = 1000};
+		.sample_interval = 1, .aggr_interval = 1000,
+		.aggr_samples = 1000,
+	};
 	damon_update_monitoring_result(r, &old_attrs, &new_attrs, false);
 	KUNIT_EXPECT_EQ(test, r->nr_accesses, 150);
 	KUNIT_EXPECT_EQ(test, r->age, 2);
 
 	new_attrs = (struct damon_attrs){
-		.sample_interval = 1, .aggr_interval = 100};
+		.sample_interval = 1, .aggr_interval = 100,
+		.aggr_samples = 100,
+	};
 	damon_update_monitoring_result(r, &old_attrs, &new_attrs, false);
 	KUNIT_EXPECT_EQ(test, r->nr_accesses, 150);
 	KUNIT_EXPECT_EQ(test, r->age, 20);
-- 
2.34.1

Re: (sashiko review) [PATCH] mm/damon/core: eliminate hot-path integer division in damon_max_nr_accesses()

[SeongJae Park]

Forwarding Sashiko review for doing discussions via mails.

# review url: https://sashiko.dev/#/patchset/20260324154005.83651-1-objecting@objecting.org
# start of sashiko.dev inline review
commit ff5e227a18cd69435749b0aae0dc2c762c4f0a47
Author: Josh Law <objecting@objecting.org>

mm/damon/core: eliminate hot-path integer division in damon_max_nr_accesses()
    
Hardware integer division is slow. The function damon_max_nr_accesses(),
which is called very frequently, performs an integer division.
However, the struct damon_attrs already caches this exact ratio in the
internal field aggr_samples. We can eliminate the hardware division in
the hot path by simply returning aggr_samples.

> diff --git a/include/linux/damon.h b/include/linux/damon.h
> index d9a3babbafc1..04c8a052fcfb 100644
> --- a/include/linux/damon.h
> +++ b/include/linux/damon.h
> @@ -978,8 +978,7 @@ static inline bool damon_target_has_pid(const struct damon_ctx *ctx)
>  static inline unsigned int damon_max_nr_accesses(const struct damon_attrs *attrs)
>  {
>  	/* {aggr,sample}_interval are unsigned long, hence could overflow */
> -	return min(attrs->aggr_interval / attrs->sample_interval,
> -			(unsigned long)UINT_MAX);
> +	return min_t(unsigned long, attrs->aggr_samples, UINT_MAX);
>  }

If an administrator dynamically updates aggr_interval via sysfs and commits
the inputs, will attrs->aggr_samples be correctly recalculated?

If aggr_samples becomes stale, kdamond_fn() will compute the real aggregation
loop count and allow r->nr_accesses to increment up to the new bounds (for
example, 80 loops). Meanwhile, damon_max_nr_accesses() will return the
stale aggr_samples cache (for example, 20).

When evaluating schemes, damon_hot_score() normalizes r->nr_accesses against
this stale maximum, producing a freq_subscore that severely exceeds
DAMON_MAX_SUBSCORE. This causes the calculated hotness to exceed
DAMOS_MAX_SCORE.

For DAMOS_PAGEOUT actions used by damon_reclaim, damon_cold_score() returns
DAMOS_MAX_SCORE - hotness, yielding a highly negative score.

Could this negative score cause an out-of-bounds write on the
kernel heap when used as an array index?

mm/damon/core.c:damos_walk() {
    ...
    c->regions_score_histogram[score] += damon_sz_region(r);
    ...
}

> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index db6c67e52d2b..afb058f7da5c 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
> @@ -587,6 +587,7 @@ struct damon_ctx *damon_new_ctx(void)
>  	ctx->attrs.sample_interval = 5 * 1000;
>  	ctx->attrs.aggr_interval = 100 * 1000;
>  	ctx->attrs.ops_update_interval = 60 * 1000 * 1000;
> +	ctx->attrs.aggr_samples = 20;

Since this value is cached here, but not updated when intervals change, does
it remain permanently stale during dynamic updates?


# end of sashiko.dev inline review
# review url: https://sashiko.dev/#/patchset/20260324154005.83651-1-objecting@objecting.org
#
# hkml [1] generated a draft of this mail.  It can be regenerated
# using below command:
#
#     hkml patch sashiko_dev --for_forwarding \
#             20260324154005.83651-1-objecting@objecting.org
#
# [1] https://github.com/sjp38/hackermail

Sent using hkml (https://github.com/sjp38/hackermail)

Re: (sashiko review) [PATCH] mm/damon/core: eliminate hot-path integer division in damon_max_nr_accesses()

[Josh Law]

On 25 March 2026 01:16:52 GMT, SeongJae Park <sj@kernel.org> wrote:
>Forwarding Sashiko review for doing discussions via mails.
>
># review url: https://sashiko.dev/#/patchset/20260324154005.83651-1-objecting@objecting.org
># start of sashiko.dev inline review
>commit ff5e227a18cd69435749b0aae0dc2c762c4f0a47
>Author: Josh Law <objecting@objecting.org>
>
>mm/damon/core: eliminate hot-path integer division in damon_max_nr_accesses()
>    
>Hardware integer division is slow. The function damon_max_nr_accesses(),
>which is called very frequently, performs an integer division.
>However, the struct damon_attrs already caches this exact ratio in the
>internal field aggr_samples. We can eliminate the hardware division in
>the hot path by simply returning aggr_samples.
>
>> diff --git a/include/linux/damon.h b/include/linux/damon.h
>> index d9a3babbafc1..04c8a052fcfb 100644
>> --- a/include/linux/damon.h
>> +++ b/include/linux/damon.h
>> @@ -978,8 +978,7 @@ static inline bool damon_target_has_pid(const struct damon_ctx *ctx)
>>  static inline unsigned int damon_max_nr_accesses(const struct damon_attrs *attrs)
>>  {
>>  	/* {aggr,sample}_interval are unsigned long, hence could overflow */
>> -	return min(attrs->aggr_interval / attrs->sample_interval,
>> -			(unsigned long)UINT_MAX);
>> +	return min_t(unsigned long, attrs->aggr_samples, UINT_MAX);
>>  }
>
>If an administrator dynamically updates aggr_interval via sysfs and commits
>the inputs, will attrs->aggr_samples be correctly recalculated?
>
>If aggr_samples becomes stale, kdamond_fn() will compute the real aggregation
>loop count and allow r->nr_accesses to increment up to the new bounds (for
>example, 80 loops). Meanwhile, damon_max_nr_accesses() will return the
>stale aggr_samples cache (for example, 20).
>
>When evaluating schemes, damon_hot_score() normalizes r->nr_accesses against
>this stale maximum, producing a freq_subscore that severely exceeds
>DAMON_MAX_SUBSCORE. This causes the calculated hotness to exceed
>DAMOS_MAX_SCORE.
>
>For DAMOS_PAGEOUT actions used by damon_reclaim, damon_cold_score() returns
>DAMOS_MAX_SCORE - hotness, yielding a highly negative score.
>
>Could this negative score cause an out-of-bounds write on the
>kernel heap when used as an array index?
>
>mm/damon/core.c:damos_walk() {
>    ...
>    c->regions_score_histogram[score] += damon_sz_region(r);
>    ...
>}
>
>> diff --git a/mm/damon/core.c b/mm/damon/core.c
>> index db6c67e52d2b..afb058f7da5c 100644
>> --- a/mm/damon/core.c
>> +++ b/mm/damon/core.c
>> @@ -587,6 +587,7 @@ struct damon_ctx *damon_new_ctx(void)
>>  	ctx->attrs.sample_interval = 5 * 1000;
>>  	ctx->attrs.aggr_interval = 100 * 1000;
>>  	ctx->attrs.ops_update_interval = 60 * 1000 * 1000;
>> +	ctx->attrs.aggr_samples = 20;
>
>Since this value is cached here, but not updated when intervals change, does
>it remain permanently stale during dynamic updates?
>
>
># end of sashiko.dev inline review
># review url: https://sashiko.dev/#/patchset/20260324154005.83651-1-objecting@objecting.org
>#
># hkml [1] generated a draft of this mail.  It can be regenerated
># using below command:
>#
>#     hkml patch sashiko_dev --for_forwarding \
>#             20260324154005.83651-1-objecting@objecting.org
>#
># [1] https://github.com/sjp38/hackermail
>
>Sent using hkml (https://github.com/sjp38/hackermail)



So it's nacked? Or is it just a review

V/R


Josh Law

Re: (sashiko review) [PATCH] mm/damon/core: eliminate hot-path integer division in damon_max_nr_accesses()

[SeongJae Park]

On Wed, 25 Mar 2026 07:36:20 +0000 Josh Law <objecting@objecting.org> wrote:

> 
> 
> On 25 March 2026 01:16:52 GMT, SeongJae Park <sj@kernel.org> wrote:
> >Forwarding Sashiko review for doing discussions via mails.
> >
> ># review url: https://sashiko.dev/#/patchset/20260324154005.83651-1-objecting@objecting.org
> ># start of sashiko.dev inline review
> >commit ff5e227a18cd69435749b0aae0dc2c762c4f0a47
> >Author: Josh Law <objecting@objecting.org>
> >
> >mm/damon/core: eliminate hot-path integer division in damon_max_nr_accesses()
> >    
> >Hardware integer division is slow. The function damon_max_nr_accesses(),
> >which is called very frequently, performs an integer division.
> >However, the struct damon_attrs already caches this exact ratio in the
> >internal field aggr_samples. We can eliminate the hardware division in
> >the hot path by simply returning aggr_samples.
> >
> >> diff --git a/include/linux/damon.h b/include/linux/damon.h
> >> index d9a3babbafc1..04c8a052fcfb 100644
> >> --- a/include/linux/damon.h
> >> +++ b/include/linux/damon.h
> >> @@ -978,8 +978,7 @@ static inline bool damon_target_has_pid(const struct damon_ctx *ctx)
> >>  static inline unsigned int damon_max_nr_accesses(const struct damon_attrs *attrs)
> >>  {
> >>  	/* {aggr,sample}_interval are unsigned long, hence could overflow */
> >> -	return min(attrs->aggr_interval / attrs->sample_interval,
> >> -			(unsigned long)UINT_MAX);
> >> +	return min_t(unsigned long, attrs->aggr_samples, UINT_MAX);
> >>  }
> >
> >If an administrator dynamically updates aggr_interval via sysfs and commits
> >the inputs, will attrs->aggr_samples be correctly recalculated?
> >
> >If aggr_samples becomes stale, kdamond_fn() will compute the real aggregation
> >loop count and allow r->nr_accesses to increment up to the new bounds (for
> >example, 80 loops). Meanwhile, damon_max_nr_accesses() will return the
> >stale aggr_samples cache (for example, 20).
> >
> >When evaluating schemes, damon_hot_score() normalizes r->nr_accesses against
> >this stale maximum, producing a freq_subscore that severely exceeds
> >DAMON_MAX_SUBSCORE. This causes the calculated hotness to exceed
> >DAMOS_MAX_SCORE.
> >
> >For DAMOS_PAGEOUT actions used by damon_reclaim, damon_cold_score() returns
> >DAMOS_MAX_SCORE - hotness, yielding a highly negative score.
> >
> >Could this negative score cause an out-of-bounds write on the
> >kernel heap when used as an array index?
> >
> >mm/damon/core.c:damos_walk() {
> >    ...
> >    c->regions_score_histogram[score] += damon_sz_region(r);
> >    ...
> >}
> >
> >> diff --git a/mm/damon/core.c b/mm/damon/core.c
> >> index db6c67e52d2b..afb058f7da5c 100644
> >> --- a/mm/damon/core.c
> >> +++ b/mm/damon/core.c
> >> @@ -587,6 +587,7 @@ struct damon_ctx *damon_new_ctx(void)
> >>  	ctx->attrs.sample_interval = 5 * 1000;
> >>  	ctx->attrs.aggr_interval = 100 * 1000;
> >>  	ctx->attrs.ops_update_interval = 60 * 1000 * 1000;
> >> +	ctx->attrs.aggr_samples = 20;
> >
> >Since this value is cached here, but not updated when intervals change, does
> >it remain permanently stale during dynamic updates?
> >
> >
> ># end of sashiko.dev inline review
> ># review url: https://sashiko.dev/#/patchset/20260324154005.83651-1-objecting@objecting.org
> >#
> ># hkml [1] generated a draft of this mail.  It can be regenerated
> ># using below command:
> >#
> >#     hkml patch sashiko_dev --for_forwarding \
> >#             20260324154005.83651-1-objecting@objecting.org
> >#
> ># [1] https://github.com/sjp38/hackermail
> >
> >Sent using hkml (https://github.com/sjp38/hackermail)
> 
> 
> 
> So it's nacked? Or is it just a review

Just a review from an AI :)

I treat sashiko review as just another review comments.  I'm forwarding these
in this way for a case that someone is interested in the comments and want to
share some findings from those in a way easy for mailing list based
communication.


Thanks,
SJ

[...]

Re: [PATCH] mm/damon/core: eliminate hot-path integer division in damon_max_nr_accesses()

[SeongJae Park]

On Tue, 24 Mar 2026 15:40:05 +0000 Josh Law <objecting@objecting.org> wrote:

> Hardware integer division is slow. The function damon_max_nr_accesses(),
> which is called very frequently, performs an integer division.
> However, the struct damon_attrs already caches this exact ratio in the
> internal field aggr_samples. We can eliminate the hardware division in
> the hot path by simply returning aggr_samples.
> 
> Signed-off-by: Josh Law <objecting@objecting.org>
> ---
>  include/linux/damon.h       |  3 +--
>  mm/damon/core.c             |  1 +
>  mm/damon/tests/core-kunit.h | 16 ++++++++++++----
>  3 files changed, 14 insertions(+), 6 deletions(-)
> 
> diff --git a/include/linux/damon.h b/include/linux/damon.h
> index 6bd71546f7b2..438fe6f3eab4 100644
> --- a/include/linux/damon.h
> +++ b/include/linux/damon.h
> @@ -960,8 +960,7 @@ static inline bool damon_target_has_pid(const struct damon_ctx *ctx)
>  static inline unsigned int damon_max_nr_accesses(const struct damon_attrs *attrs)
>  {
>  	/* {aggr,sample}_interval are unsigned long, hence could overflow */
> -	return min(attrs->aggr_interval / attrs->sample_interval,
> -			(unsigned long)UINT_MAX);
> +	return min_t(unsigned long, attrs->aggr_samples, UINT_MAX);
>  }
>  
>  
> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index b0ab0ee6eab9..59b709f04975 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
> @@ -582,6 +582,7 @@ struct damon_ctx *damon_new_ctx(void)
>  	ctx->attrs.sample_interval = 5 * 1000;
>  	ctx->attrs.aggr_interval = 100 * 1000;
>  	ctx->attrs.ops_update_interval = 60 * 1000 * 1000;
> +	ctx->attrs.aggr_samples = 20;
>  
>  	ctx->passed_sample_intervals = 0;
>  	/* These will be set from kdamond_init_ctx() */
> diff --git a/mm/damon/tests/core-kunit.h b/mm/damon/tests/core-kunit.h
> index e86d4f4fe261..56d03ef6a5a4 100644
> --- a/mm/damon/tests/core-kunit.h
> +++ b/mm/damon/tests/core-kunit.h
> @@ -416,6 +416,8 @@ static void damon_test_nr_accesses_to_accesses_bp(struct kunit *test)
>  		.aggr_interval = ((unsigned long)UINT_MAX + 1) * 10
>  	};
>  
> +	attrs.aggr_samples = attrs.aggr_interval / attrs.sample_interval;
> +
>  	/*
>  	 * In some cases such as 32bit architectures where UINT_MAX is
>  	 * ULONG_MAX, attrs.aggr_interval becomes zero.  Calling
> @@ -434,7 +436,8 @@ static void damon_test_nr_accesses_to_accesses_bp(struct kunit *test)
>  static void damon_test_update_monitoring_result(struct kunit *test)
>  {
>  	struct damon_attrs old_attrs = {
> -		.sample_interval = 10, .aggr_interval = 1000,};
> +		.sample_interval = 10, .aggr_interval = 1000,
> +		.aggr_samples = 100,};
>  	struct damon_attrs new_attrs;
>  	struct damon_region *r = damon_new_region(3, 7);
>  
> @@ -446,19 +449,24 @@ static void damon_test_update_monitoring_result(struct kunit *test)
>  	r->age = 20;
>  
>  	new_attrs = (struct damon_attrs){
> -		.sample_interval = 100, .aggr_interval = 10000,};
> +		.sample_interval = 100, .aggr_interval = 10000,
> +		.aggr_samples = 100,};
>  	damon_update_monitoring_result(r, &old_attrs, &new_attrs, false);
>  	KUNIT_EXPECT_EQ(test, r->nr_accesses, 15);
>  	KUNIT_EXPECT_EQ(test, r->age, 2);
>  
>  	new_attrs = (struct damon_attrs){
> -		.sample_interval = 1, .aggr_interval = 1000};
> +		.sample_interval = 1, .aggr_interval = 1000,
> +		.aggr_samples = 1000,
> +	};
>  	damon_update_monitoring_result(r, &old_attrs, &new_attrs, false);
>  	KUNIT_EXPECT_EQ(test, r->nr_accesses, 150);
>  	KUNIT_EXPECT_EQ(test, r->age, 2);
>  
>  	new_attrs = (struct damon_attrs){
> -		.sample_interval = 1, .aggr_interval = 100};
> +		.sample_interval = 1, .aggr_interval = 100,
> +		.aggr_samples = 100,
> +	};
>  	damon_update_monitoring_result(r, &old_attrs, &new_attrs, false);
>  	KUNIT_EXPECT_EQ(test, r->nr_accesses, 150);
>  	KUNIT_EXPECT_EQ(test, r->age, 20);
> -- 
> 2.34.1

I think this amount of change is somewhat too much maintenance burden for the
simple optimization.  Apparently aggr_samples was not designed for this use
case.  I think another way should be used, but failing at finding a good
alternative.  And I'm not very sure damon_max_nr_accesses() deserves that much
time to optimize.

So, if you find a better way for optimizing damon_max_nr_accesses()
without adding maintenance burden or evidence of benefit that justifies this
amount of maintenance burden, please feel free to repost a new version.  But,
in my humble and frank opinion, I don't see that much worth in the
optimization.


Thanks,
SJ

[...]

Re: (sashiko review) [PATCH] mm/damon/core: eliminate hot-path integer division in damon_max_nr_accesses()

[SeongJae Park]

On Tue, 24 Mar 2026 18:16:52 -0700 SeongJae Park <sj@kernel.org> wrote:

> Forwarding Sashiko review for doing discussions via mails.
> 
> # review url: https://sashiko.dev/#/patchset/20260324154005.83651-1-objecting@objecting.org
> # start of sashiko.dev inline review
> commit ff5e227a18cd69435749b0aae0dc2c762c4f0a47
> Author: Josh Law <objecting@objecting.org>
> 
> mm/damon/core: eliminate hot-path integer division in damon_max_nr_accesses()
>     
> Hardware integer division is slow. The function damon_max_nr_accesses(),
> which is called very frequently, performs an integer division.
> However, the struct damon_attrs already caches this exact ratio in the
> internal field aggr_samples. We can eliminate the hardware division in
> the hot path by simply returning aggr_samples.
> 
> > diff --git a/include/linux/damon.h b/include/linux/damon.h
> > index d9a3babbafc1..04c8a052fcfb 100644
> > --- a/include/linux/damon.h
> > +++ b/include/linux/damon.h
> > @@ -978,8 +978,7 @@ static inline bool damon_target_has_pid(const struct damon_ctx *ctx)
> >  static inline unsigned int damon_max_nr_accesses(const struct damon_attrs *attrs)
> >  {
> >  	/* {aggr,sample}_interval are unsigned long, hence could overflow */
> > -	return min(attrs->aggr_interval / attrs->sample_interval,
> > -			(unsigned long)UINT_MAX);
> > +	return min_t(unsigned long, attrs->aggr_samples, UINT_MAX);
> >  }
> 
> If an administrator dynamically updates aggr_interval via sysfs and commits
> the inputs, will attrs->aggr_samples be correctly recalculated?
> 
> If aggr_samples becomes stale, kdamond_fn() will compute the real aggregation
> loop count and allow r->nr_accesses to increment up to the new bounds (for
> example, 80 loops). Meanwhile, damon_max_nr_accesses() will return the
> stale aggr_samples cache (for example, 20).
> 
> When evaluating schemes, damon_hot_score() normalizes r->nr_accesses against
> this stale maximum, producing a freq_subscore that severely exceeds
> DAMON_MAX_SUBSCORE. This causes the calculated hotness to exceed
> DAMOS_MAX_SCORE.
> 
> For DAMOS_PAGEOUT actions used by damon_reclaim, damon_cold_score() returns
> DAMOS_MAX_SCORE - hotness, yielding a highly negative score.
> 
> Could this negative score cause an out-of-bounds write on the
> kernel heap when used as an array index?
> 
> mm/damon/core.c:damos_walk() {
>     ...
>     c->regions_score_histogram[score] += damon_sz_region(r);
>     ...
> }

I didn't take deep look here, but sounds like true.  I also had similar
concern.

But this takes too much time for the small optimization, so I suggested another
direction.  So I will not take more time on this sashiko comment.

> 
> > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > index db6c67e52d2b..afb058f7da5c 100644
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
> > @@ -587,6 +587,7 @@ struct damon_ctx *damon_new_ctx(void)
> >  	ctx->attrs.sample_interval = 5 * 1000;
> >  	ctx->attrs.aggr_interval = 100 * 1000;
> >  	ctx->attrs.ops_update_interval = 60 * 1000 * 1000;
> > +	ctx->attrs.aggr_samples = 20;
> 
> Since this value is cached here, but not updated when intervals change, does
> it remain permanently stale during dynamic updates?

Ditto.


Thanks,
SJ

[...]