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

[SJ Park <sj@kernel.org>]

On Wed, 08 Jul 2026 14:08:07 +0000 sashiko-bot@kernel.org wrote:

> 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?

Known low priority issue.

> 
> > +		else
> > +			r->probe_hits[i] = 0;
> > +		i++;
> > +	}
> > +}
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260708135359.122587-1-sj@kernel.org?part=6


Thanks,
SJ