Re: [RFC PATCH 1/4] mm/damon/core: add DAMOS_FILTER_TYPE_AGE_MS

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] mm/damon/core: 32-bit overflow calculating age_ms in damos_filter_match()
--

commit 8785e74923668744015c21ff7266b94722501bbc
Author: SJ Park <sj@kernel.org>

mm/damon/core: add DAMOS_FILTER_TYPE_AGE_MS

This commit adds a new DAMOS core filter type, DAMOS_FILTER_TYPE_AGE_MS,
allowing schemes to filter target regions by age in milliseconds using
damos_filter->range_min and damos_filter->range_max.

> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index d36823f4d0928..d631863b034db 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c

[ ... ]

> @@ -2552,7 +2554,7 @@ static bool damos_filter_match(struct damon_ctx *ctx, struct damon_target *t,
>  	bool matched = false;
>  	struct damon_target *ti;
>  	int target_idx = 0;
> -	unsigned long start, end, wsum;
> +	unsigned long start, end, wsum, age_ms;
>  
>  	*err = 0;
>  	switch (filter->type) {
> @@ -2594,6 +2596,11 @@ static bool damos_filter_match(struct damon_ctx *ctx, struct damon_target *t,
>  		matched = filter->range_min <= wsum &&
>  			wsum <= filter->range_max;
>  		break;
> +	case DAMOS_FILTER_TYPE_AGE_MS:
> +		age_ms = r->age * ctx->attrs.aggr_interval / USEC_PER_MSEC;
> +		matched = filter->range_min <= age_ms &&
> +			age_ms <= filter->range_max;
> +		break;

[Severity: Medium]
Can the multiplication r->age * ctx->attrs.aggr_interval in
damos_filter_match() overflow on 32-bit architectures?

Since r->age is unsigned int and ctx->attrs.aggr_interval is unsigned long
(in microseconds), the multiplication is evaluated in 32-bit unsigned long
on 32-bit platforms before dividing by USEC_PER_MSEC. Once a region's age
exceeds ULONG_MAX microseconds (about 4,295 seconds, or r->age >= 42,950 at
the default 100,000 us aggr_interval), the intermediate product wraps around
and misclassifies old regions with a small age_ms value.

By comparison, damon_hot_score() in mm/damon/ops-common.c casts r->age to
u64 and uses div_u64() to avoid this 32-bit overflow:

	age_in_sec = div_u64((u64)r->age * c->attrs.aggr_interval,
			     USEC_PER_SEC);

Should damos_filter_match() similarly use div_u64((u64)r->age *
ctx->attrs.aggr_interval, USEC_PER_MSEC)?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261104153645.82945-1-sj@kernel.org?part=1