Re: [RFC PATCH v1.2 6/7] mm/damon/core: update probe hits for new parameter commit

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Silent integer truncation causes scaled `probe_hits` to wrap around, corrupting monitoring data when updating DAMON attributes.
--

--- Patch [6]: [RFC PATCH v1.2 6/7] mm/damon/core: update probe hits for new parameter commit ---
commit 3c348527fa362f1ef3768392d433781b92010ba7
Author: SJ Park <sj@kernel.org>

mm/damon/core: update probe hits for new parameter commit

This commit ensures that DAMON probe hits are appropriately scaled when a user
dynamically updates the sampling or aggregation intervals at runtime. It
applies the existing scaling logic used for other metrics to both probe_hits
and last_probe_hits.

> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index 800ff6b6d2473..15d31285515d6 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
> @@ -897,10 +897,32 @@ static unsigned int damon_nr_samples_for_new_attrs(unsigned int nr,
>  			damon_sample_count_to_bp(nr, old_attrs), new_attrs);
>  }
>  
> +static void damon_update_probe_hits(struct damon_region *r,
> +		struct damon_attrs *old_attrs, struct damon_attrs *new_attrs,
> +		bool aggregating, struct damon_ctx *ctx)
> +{
> +	struct damon_probe *p;
> +	int i = 0;
> +
> +	damon_for_each_probe(p, ctx) {
> +		r->last_probe_hits[i] = damon_nr_samples_for_new_attrs(
> +				r->last_probe_hits[i], old_attrs, new_attrs);
> +		if (!aggregating)
> +			r->probe_hits[i] = damon_nr_samples_for_new_attrs(
> +					r->probe_hits[i], old_attrs,
> +					new_attrs);

[Severity: Medium]
Will this silently truncate the scaled probe hits if they exceed 255?

The function damon_nr_samples_for_new_attrs() returns an unsigned int, but
both last_probe_hits and probe_hits are defined as unsigned char arrays in
struct damon_region. 

If a user dynamically updates intervals via sysfs such that the new
aggr_samples is significantly larger than the old one, the scaled value could
exceed the 8-bit maximum and wrap around modulo 256 instead of saturating.

Should these values be clamped to the maximum value of an unsigned char to
prevent monitoring data corruption when the scaled count overflows?

> +		else
> +			r->probe_hits[i] = 0;
> +		i++;
> +	}
> +}

[ ... ]

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