Re: [PATCH] mm/damon/stat: avoid overflow in estimated_memory_bandwidth calculation

[Wang Guangke <gkwang@linx-info.com>]

Hi SJ,
Thanks for your review.

On Sun, Nov 01, 2026 at 11:13:43AM -0800, SJ Park wrote:
> Hello Wang,
> 
> On Mon,  2 Nov 2026 02:00:01 +0800 Wang Guangke <gkwang@linx-info.com> wrote:
> 
> > The calculation of estimated_memory_bandwidth computes
> > access_bytes * USEC_PER_MSEC * MSEC_PER_SEC / aggr_interval directly.
> > On 32-bit systems, the intermediate product
> > access_bytes * USEC_PER_MSEC * MSEC_PER_SEC may overflow before the
> > division is performed.
> 
> Do you find this issue from real usage?  Or, jsut theoretical?  Could you
> please clarify it, and user impacts of the issue?

I found it by code inspection, not from real usage on a 32-bit system.

The parameter estimated_memory_bandwidth of the damon_stat
module is calculated from access_bytes * USEC_PER_MSEC *
MSEC_PER_SEC (= USEC_PER_SEC) / c->attrs.aggr_interval,
and users read it via
/sys/module/damon_stat/parameters/estimated_memory_bandwidth.

The maximum value of a 32-bit unsigned integer is 4294967295. Therefore,
in the product access_bytes * USEC_PER_SEC, once access_bytes exceeds 4294,
the 32-bit value is bound to overflow. Users reading
/sys/module/damon_stat/parameters/estimated_memory_bandwidth
on 32-bit systems will get a wrong value.

To see whether real workloads exceed this threshold, I printed the
intermediate values of the computation via a tracepoint on a 64-bit
system.

on 64-bit system:
    ~ # echo 1 > /sys/kernel/debug/tracing/event/damon/estimated_memory_bandwidth
    ~ # cat /sys/kernel/debug/tracing/trace
    ...
           kdamond.0-78      [003] .....  7507.959114: estimated_memory_bandwidth: access_bytes=5536956416, c->attrs.aggr_interval=200000000

I observed that access_bytes can far exceed 4294.

In addition, I used qemu-system-i386 to emulate a 32-bit system and
performed the same verification:

on 32-bit system:
without-patch:
    ~ # cat /sys/kernel/debug/tracing/trace
    ...
           kdamond.0-45      [000] .....  1809.507372: estimated_memory_bandwidth: access_bytes=15237120, c->attrs.aggr_interval=7910660
    ~ # cat /sys/module/damon_stat/parameters/estimated_memory_bandwidth
    362
    ~ # echo "15237120 * 1000 * 1000 / 7910660" | bc
    1926150

The module parameter value (362) does not match the expected value
(1926150), so an overflow has occurred.

with-patch:
    ~ # cat /sys/kernel/debug/tracing/trace
    ...
           kdamond.0-45      [000] .....   212.682057: estimated_memory_bandwidth: access_bytes=7503872, c->attrs.aggr_interval=3615680
    ~ # cat /sys/module/damon_stat/parameters/estimated_memory_bandwidth
    2075369
    ~ # echo "7503872 * 1000 * 1000 / 3615680" | bc
    2075369

The module parameter value (2075369) matches the expected value.
> 
> > 
> > Fix it by splitting the division into quotient and remainder,
> > multiplying each part separately.
> 
> Thank you for this patch.
> 
> > 
> > Fixes: fabdd1e911da ("mm/damon/stat: calculate and expose estimated
> > memory bandwidth")
> 
> Please do not wrap Fixes: tag line.

Sorry, I will not wrap it in v2.
> 
> Could you clarify why you do not Cc stable@?

Sorry, I missed it. I will Cc stable@ in v2.
> 
> > 
> > Signed-off-by: Wang Guangke <gkwang@linx-info.com>
> > ---
> >  mm/damon/stat.c | 17 +++++++++++------
> >  1 file changed, 11 insertions(+), 6 deletions(-)
> > 
> > diff --git a/mm/damon/stat.c b/mm/damon/stat.c
> > index b05b68f73e10..7cfa3a8d6801 100644
> > --- a/mm/damon/stat.c
> > +++ b/mm/damon/stat.c
> > @@ -32,8 +32,8 @@ static bool enabled __read_mostly = IS_ENABLED(
> >  module_param_cb(enabled, &enabled_param_ops, NULL, 0600);
> >  MODULE_PARM_DESC(enabled, "Enable of disable DAMON_STAT");
> >  
> > -static unsigned long estimated_memory_bandwidth __read_mostly;
> > -module_param(estimated_memory_bandwidth, ulong, 0400);
> > +static u64 estimated_memory_bandwidth __read_mostly;
> > +module_param(estimated_memory_bandwidth, ullong, 0400);
> >  MODULE_PARM_DESC(estimated_memory_bandwidth,
> >  		"Estimated memory bandwidth usage in bytes per second");
> >  
> > @@ -55,15 +55,20 @@ static void damon_stat_set_estimated_memory_bandwidth(struct damon_ctx *c)
> >  {
> >  	struct damon_target *t;
> >  	struct damon_region *r;
> > -	unsigned long access_bytes = 0;
> > +	u64 access_bytes = 0;
> > +	u64 remainder, bytes_per_interval;
> >  
> >  	damon_for_each_target(t, c) {
> >  		damon_for_each_region(r, t)
> > -			access_bytes += (r->ar.end - r->ar.start) *
> > +			access_bytes += (u64)(r->ar.end - r->ar.start) *
> >  				r->nr_accesses;
> >  	}
> > -	estimated_memory_bandwidth = access_bytes * USEC_PER_MSEC *
> > -		MSEC_PER_SEC / c->attrs.aggr_interval;
> > +	bytes_per_interval = div64_u64_rem(access_bytes,
> > +			c->attrs.aggr_interval, &remainder);
> 
> This is not bytes per interval but bytes per microsecond.  Let's rename the
> variable to bytes_per_us.
> 
> > +	estimated_memory_bandwidth =
> > +		bytes_per_interval * USEC_PER_MSEC * MSEC_PER_SEC +
> > +		div64_u64(remainder * USEC_PER_MSEC * MSEC_PER_SEC,
> > +				c->attrs.aggr_interval);
> 
> Let's use USEC_PER_SEC instead of 'USEC_PER_MSEC * MSEC_PER_SEC'.

Thanks, I will rename the variable to bytes_per_us, and use USEC_PER_SEC
instead of 'USEC_PER_MSEC * MSEC_PER_SEC'.
> 
> >  }
> >  
> >  static int damon_stat_idletime(const struct damon_region *r)
> > -- 
> > 2.47.3
> > 
> > 
> 
> 
> Thanks,
> SJ
> 
Thanks,
Wang Guangke