Re: [RFC PATCH v1.1 3/3] mm/damon/sysfs-schemes: set probe hits as pseudo moving sums

[SJ Park <sj@kernel.org>]

On Thu, 02 Jul 2026 17:32:07 +0000 sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] damon_probe_hits_mvsum() returns an artificially inflated moving sum when left_window_bp == 10000 after an attribute update.
> --
> 
> commit aa52db4de056d7c850beaa9ef06ec63a3befcbf2
> Author: SJ Park <sj@kernel.org>
> 
> mm/damon/sysfs-schemes: set probe hits as pseudo moving sums
> 
> This commit updates the DAMON sysfs interface to expose probe hits using
> a pseudo moving sum instead of incomplete counters. It uses
> damon_probe_hits_mvsum() to calculate the value, similar to the logic
> used for nr_accesses.
> 
> > diff --git a/mm/damon/sysfs-schemes.c b/mm/damon/sysfs-schemes.c
> > index dbf2b0515d582..32f495a96b17a 100644
> > --- a/mm/damon/sysfs-schemes.c
> > +++ b/mm/damon/sysfs-schemes.c
> > @@ -110,7 +110,8 @@ static int damos_sysfs_probes_add_dirs(struct damos_sysfs_probes *probes,
> >  		struct damos_sysfs_probe *sys_probe;
> >  		int err;
> >  
> > -		sys_probe = damos_sysfs_probe_alloc(region->probe_hits[i]);
> > +		sys_probe = damos_sysfs_probe_alloc(
> > +				damon_probe_hits_mvsum(i, region, ctx));
> 
> [Severity: Medium]
> Can this result in an artificially inflated moving sum being exposed to
> userspace?
> 
> Looking at damon_probe_hits_mvsum() in mm/damon/core.c:
> 
> mm/damon/core.c:damon_probe_hits_mvsum() {
>     ...
> 	return damon_mvsum(r->probe_hits[probe_idx],
> 			r->last_probe_hits[probe_idx], left_window_bp);
> }
> 
> When left_window_bp == 10000 after an attribute update, this calculates
> current + last.
> 
> Because r->probe_hits is not cleared during attribute updates, the
> un-cleared partial hits might be erroneously added to the previous
> interval's hits. This function seems to lack the safeguard for this
> condition that is present in damon_nr_accesses_mvsum().
> 
> If users query the stats immediately after an update, could this
> silently overflow the unsigned char and expose incorrect statistics?

This is good thing to improve it for the perfect.  But the perfect is the enemy
of good ;)

probe_hits was very unreliable before this patch, and this patch makes it
reliable but still having some rooms to improve.

Not a blocker for this patch, in my opinion.


Thanks,
SJ

[...]