Re: [PATCH 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:
- [High] Scaling of `probe_hits` during parameter update produces mathematically incorrect results and silent integer truncation due to `unsigned char` type constraints.
--

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

Users can dynamically update subsystem parameters at runtime. This patch adds
logic to appropriately scale probe_hits and last_probe_hits based on the
updated aggregation and sample intervals to maintain accurate monitoring
snapshots.

> 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: High]
Does this code silently truncate the scaled results?

damon_nr_samples_for_new_attrs() returns an unsigned int, but both
last_probe_hits[] and probe_hits[] in struct damon_region are unsigned char
arrays. If the user updates the parameters such that the new ratio of
aggr_interval to sample_interval scales the sample count above 255, the
assignment to unsigned char will silently wrap.

For example, if a hit count is scaled up to 300, the stored value would
become 44. Could this produce mathematically invalid scaling and corrupt the
relative hit proportions that the monitoring results rely on?

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

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