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

[SJ Park <sj@kernel.org>]

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?

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

Could you clarify why you do not Cc stable@?

> 
> 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'.

>  }
>  
>  static int damon_stat_idletime(const struct damon_region *r)
> -- 
> 2.47.3
> 
> 


Thanks,
SJ