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

[sashiko-bot@kernel.org]

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?

>  		if (!sys_probe) {
>  			damos_sysfs_probes_rm_dirs(probes);
>  			return -ENOMEM;

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