* [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; 4+ 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 [flat|nested] 4+ 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
2026-11-02 10:04 ` Wang Guangke
0 siblings, 1 reply; 4+ 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] 4+ messages in thread
* Re: [PATCH] mm/damon/stat: avoid overflow in estimated_memory_bandwidth calculation
2026-11-01 19:13 ` SJ Park
@ 2026-11-02 10:04 ` Wang Guangke
2026-11-02 14:25 ` SJ Park
0 siblings, 1 reply; 4+ messages in thread
From: Wang Guangke @ 2026-11-02 10:04 UTC (permalink / raw)
To: SJ Park; +Cc: akpm, damon, linux-mm, linux-kernel
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] mm/damon/stat: avoid overflow in estimated_memory_bandwidth calculation
2026-11-02 10:04 ` Wang Guangke
@ 2026-11-02 14:25 ` SJ Park
0 siblings, 0 replies; 4+ messages in thread
From: SJ Park @ 2026-11-02 14:25 UTC (permalink / raw)
To: Wang Guangke; +Cc: SJ Park, akpm, damon, linux-mm, linux-kernel
On Mon, 2 Nov 2026 18:04:23 +0800 Wang Guangke <gkwang@linx-info.com> wrote:
> 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.
Thank you for clarifying.
[...]
> > > + 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'.
Thank you for accepting my humble suggestions. Looking forward to v2.
Thanks,
SJ
[...]
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-11-02 14:25 UTC | newest]
Thread overview: 4+ 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
2026-11-02 10:04 ` Wang Guangke
2026-11-02 14:25 ` SJ Park
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®