Re: [RFC PATCH v1.1 05/16] mm/damon/core: implement damon_probe_hits_wsum()

[SJ Park <sj@kernel.org>]

On Mon, 06 Jul 2026 14:41:34 +0000 sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
> 
> New issues:
> - [High] The calculation of the weighted sum in `damon_probe_hits_wsum()` is vulnerable to integer overflow, resulting in a silent wrap-around.
> - [Low] Misspelled words in the kernel-doc comment for `damon_probe_hits_wsum`.
> 
> Pre-existing issues:
> - [High] `r->probe_hits[i]` is an 8-bit `unsigned char` but serves as a sample counter across an aggregation interval, making it highly susceptible to wrap-around.
> --
> 
> commit 627b12cd883dbd832e93cc6b4f4e3373c791b6d0
> Author: SJ Park <sj@kernel.org>
> 
> mm/damon/core: implement damon_probe_hits_wsum()
> 
> This commit implements damon_probe_hits_wsum() to calculate the weighted
> sum of probe hits for a DAMON region. This weighted sum is intended to be
> used by users as well as DAMON internal logic like region merging.
> 
> [ ... ]
> 
> > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > index dbd1f86a0832f..42f27849af722 100644
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
> > @@ -390,6 +390,30 @@ static bool damon_is_last_region(struct damon_region *r,
> >  	return list_is_last(&r->list, &t->regions_list);
> >  }
> >  
> > +/**
> > + * damon_probe_hits_wsum() - Returns probe hits weighted sum of a region.
> > + * @r:		region to get the weigted sum of.
> > + * @last:	if the request is for last-window agggregated probe hits.
> 
> [Severity: Low]
> This isn't a bug, but there are spelling errors in this kernel-doc block.
> 
> "weigted" should be "weighted" and "agggregated" should be "aggregated".

Good finding, I will fix these in the next spin.

> 
> > + * @ctx:	context of &r.
> > + *
> > + * Return: the weighted sum of probe hits of the region.
> > + */
> > +unsigned int damon_probe_hits_wsum(struct damon_region *r, bool last,
> > +		struct damon_ctx *ctx)
> > +{
> > +	struct damon_probe *probe;
> > +	unsigned int sum = 0;
> > +	int i = 0;
> > +
> > +	damon_for_each_probe(probe, ctx) {
> > +		if (last)
> > +			sum += r->last_probe_hits[i++] * probe->weight;
> > +		else
> > +			sum += r->probe_hits[i++] * probe->weight;
> 
> [Severity: High]
> Can the accumulation into sum silently overflow?
> 
> If a large probe weight is configured via sysfs (e.g., > 16,843,009), the
> multiplication of this 32-bit unsigned int weight by r->probe_hits[i] could
> wrap around when accumulated into sum.
> 
> This might corrupt the DAMON region merge and split logic that relies on this
> score.

Yes, it can.  But it causes only poor monitoring results for wrong user setup.
Maybe we can improve it, but not necessarily right now.  No blocker.

> 
> [Severity: High]
> This is a pre-existing issue, but can r->probe_hits[i] wrap around during long
> aggregation intervals?
> 
> The probe_hits array is defined as an 8-bit unsigned char. If the user
> configures aggr_interval and sample_interval such that their ratio exceeds 255
> (for example, 1s aggregation and 1ms sample), the counter will silently wrap
> around to 0 without any bounds checking.
> 
> This could severely degrade the accuracy of DAMON's monitoring, and is
> exacerbated here by relying on the potentially corrupted counter for the new
> weighted sum logic.

Known low priority issue.  No blocker.

> 
> > +	}
> > +	return sum;
> > +}
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260706141912.88445-1-sj@kernel.org?part=5

Thanks,
SJ