Re: [RFC PATCH v2 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: 64-to-32-bit truncation of age_ms in damos_filter_match()
--

commit 2459402d4dc794f740b6a5720a69fe4a1a61ae62
Author: SJ Park <sj@kernel.org>

mm/damon/core: add DAMOS_FILTER_TYPE_AGE_MS

This commit introduces DAMOS_FILTER_TYPE_AGE_MS in the DAMON API and core
layer to allow filtering DAMOS target regions by age in milliseconds. It
converts the region age from aggregation intervals to milliseconds using
div_u64() and compares the result against damos_filter->range_{min,max}.

> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index d36823f4d0928..0a056949604f9 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,12 @@ 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 = div_u64((u64)r->age * ctx->attrs.aggr_interval,
> +				USEC_PER_MSEC);
> +		matched = filter->range_min <= age_ms &&
> +			age_ms <= filter->range_max;
> +		break;

[Severity: Medium]
In damos_filter_match(), age_ms is declared as unsigned long. On 32-bit
architectures, unsigned long is 32 bits wide, so storing the u64 return value
of div_u64() into age_ms truncates the upper 32 bits whenever the region age
in milliseconds exceeds ULONG_MAX (about 49.7 days, or for example
r->age >= 42,949,673 at the default 100,000 us aggr_interval, and
proportionally earlier when aggr_interval is larger).

When that happens during kdamond_apply_schemes() -> damon_do_apply_schemes()
-> damos_apply_scheme() -> damos_filter_out() -> damos_core_filter_out() ->
damos_filter_match(), age_ms wraps modulo 2^32 and can misclassify old
regions as young regions or fail to match old-region filters.

By comparison, damon_hot_score() in mm/damon/ops-common.c stores the result
of div_u64() in a u64 variable (as fixed in commit 0284dff76e87 ("mm/damon/
ops-common: fix 64-bit division in damon_hot_score()")):

mm/damon/ops-common.c:damon_hot_score() {
	...
	u64 age_in_sec;
	...
	age_in_sec = div_u64((u64)r->age * c->attrs.aggr_interval,
			     USEC_PER_SEC);
	...
}

Should age_ms be declared as u64 (or saturated to ULONG_MAX before storing into
age_ms so filters with range_max == ULONG_MAX on 32-bit continue to match) to
avoid truncating the quotient on 32-bit systems?

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