Linux-mm Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] mm/damon/stat: avoid overflow in estimated_memory_bandwidth calculation
@ 2026-11-01 18:00 Wang Guangke
  2026-11-01 19:13 ` SJ Park
  0 siblings, 1 reply; 2+ messages in thread
From: Wang Guangke @ 2026-11-01 18:00 UTC (permalink / raw)
  To: sj, akpm; +Cc: damon, linux-mm, linux-kernel, Wang Guangke

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.

Fix it by splitting the division into quotient and remainder,
multiplying each part separately.

Fixes: fabdd1e911da ("mm/damon/stat: calculate and expose estimated
memory bandwidth")

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);
+	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);
 }
 
 static int damon_stat_idletime(const struct damon_region *r)
-- 
2.47.3



^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH] mm/damon/stat: avoid overflow in estimated_memory_bandwidth calculation
  2026-11-01 18:00 [PATCH] mm/damon/stat: avoid overflow in estimated_memory_bandwidth calculation Wang Guangke
@ 2026-11-01 19:13 ` SJ Park
  0 siblings, 0 replies; 2+ messages in thread
From: SJ Park @ 2026-11-01 19:13 UTC (permalink / raw)
  To: Wang Guangke; +Cc: SJ Park, akpm, damon, linux-mm, linux-kernel

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


^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-11-01 19:13 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-11-01 18:00 [PATCH] mm/damon/stat: avoid overflow in estimated_memory_bandwidth calculation Wang Guangke
2026-11-01 19:13 ` SJ Park

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox