* [PATCH] mm/damon/stat: avoid 32-bit overflow in idle percentiles
@ 2026-10-15 14:52 Aveline Noir
2026-10-15 15:05 ` sashiko-bot
2026-10-16 0:21 ` SJ Park
0 siblings, 2 replies; 3+ messages in thread
From: Aveline Noir @ 2026-10-15 14:52 UTC (permalink / raw)
To: SJ Park; +Cc: Andrew Morton, damon, linux-mm, linux-kernel
On 32-bit, accounted_bytes * 100 can wrap before division by total_sz.
Some entries in memory_idle_ms_percentiles then keep their old values
instead of being updated.
For example, a 256 MiB i386 QEMU guest reported p0=19200 ms, but p50 and
p100 were both zero.
Cast accounted_bytes to u64 before multiplying and use div64_ul() for
the division. Compute the percentage once per region.
Link: https://lore.kernel.org/r/CANOyQmHad1vamnZT12M5yPrJ69==Pt1U0+MGw6u0MuNtr-DRCw@mail.gmail.com
Signed-off-by: Aveline Noir <jm5905938@gmail.com>
---
This is against mm-new ad9f2d463666. With the fix, the original test
reports p0=p50=p100=19200 ms. The i386 kernel build, x86_64 W=1 stat.o
build and external arithmetic checks passed.
I also ran 38 before/after VM tests and 20 same-image controls. Throughput
was close overall, but median kdamond CPU rose by 0.0053 and 0.0221
percentage points of one guest CPU for stream and sparse, respectively.
Some pairs had 5.91% lower throughput or 15.49% higher scan P99. So I can't
yet tell whether these differences come from the patch.
mm/damon/stat.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/mm/damon/stat.c b/mm/damon/stat.c
index b05b68f73e10..eccd372810d0 100644
--- a/mm/damon/stat.c
+++ b/mm/damon/stat.c
@@ -114,7 +114,7 @@ static void damon_stat_set_idletime_percentiles(struct damon_ctx *c)
{
struct damon_region **sorted_regions, *region;
int nr_regions;
- unsigned long total_sz, accounted_bytes = 0;
+ unsigned long total_sz, accounted_bytes = 0, percentile;
int err, i, next_percentile = 0;
err = damon_stat_sort_regions(c, &sorted_regions, &nr_regions,
@@ -124,7 +124,8 @@ static void damon_stat_set_idletime_percentiles(struct damon_ctx *c)
for (i = 0; i < nr_regions; i++) {
region = sorted_regions[i];
accounted_bytes += region->ar.end - region->ar.start;
- while (next_percentile <= accounted_bytes * 100 / total_sz)
+ percentile = div64_ul((u64)accounted_bytes * 100, total_sz);
+ while (next_percentile <= percentile)
memory_idle_ms_percentiles[next_percentile++] =
damon_stat_idletime(region) *
(long)c->attrs.aggr_interval / USEC_PER_MSEC;
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] mm/damon/stat: avoid 32-bit overflow in idle percentiles
2026-10-15 14:52 [PATCH] mm/damon/stat: avoid 32-bit overflow in idle percentiles Aveline Noir
@ 2026-10-15 15:05 ` sashiko-bot
2026-10-16 0:21 ` SJ Park
1 sibling, 0 replies; 3+ messages in thread
From: sashiko-bot @ 2026-10-15 15:05 UTC (permalink / raw)
To: Aveline Noir; +Cc: damon
> On 32-bit, accounted_bytes * 100 can wrap before division by total_sz.
> Some entries in memory_idle_ms_percentiles then keep their old values
> instead of being updated.
>
> For example, a 256 MiB i386 QEMU guest reported p0=19200 ms, but p50 and
> p100 were both zero.
>
> Cast accounted_bytes to u64 before multiplying and use div64_ul() for
> the division. Compute the percentage once per region.
>
> Link: https://lore.kernel.org/r/CANOyQmHad1vamnZT12M5yPrJ69==Pt1U0+MGw6u0MuNtr-DRCw@mail.gmail.com
> Signed-off-by: Aveline Noir <jm5905938@gmail.com>
Sashiko has reviewed this patch and found no issues. It looks great!
--
Sashiko AI review · https://sashiko.dev/#/patchset/179207592479.3.11595453180868898703@gmail.com?part=1
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] mm/damon/stat: avoid 32-bit overflow in idle percentiles
2026-10-15 14:52 [PATCH] mm/damon/stat: avoid 32-bit overflow in idle percentiles Aveline Noir
2026-10-15 15:05 ` sashiko-bot
@ 2026-10-16 0:21 ` SJ Park
1 sibling, 0 replies; 3+ messages in thread
From: SJ Park @ 2026-10-16 0:21 UTC (permalink / raw)
To: Aveline Noir; +Cc: SJ Park, Andrew Morton, damon, linux-mm, linux-kernel
Hi Aveline,
This patch is sent as a followup of your previous bug report, right? Unless it
is very urgent, please make sure the previous discussion is completed before
making a followup action like sending this patch. As a rule of thumb for
DAMON, I'd suggest waiting for ~10 hours after the last comment on the thread.
On Thu, 15 Oct 2026 22:52:04 +0800 Aveline Noir <jm5905938@gmail.com> wrote:
> On 32-bit, accounted_bytes * 100 can wrap before division by total_sz.
> Some entries in memory_idle_ms_percentiles then keep their old values
> instead of being updated.
>
> For example, a 256 MiB i386 QEMU guest reported p0=19200 ms, but p50 and
> p100 were both zero.
>
> Cast accounted_bytes to u64 before multiplying and use div64_ul() for
> the division. Compute the percentage once per region.
>
> Link: https://lore.kernel.org/r/CANOyQmHad1vamnZT12M5yPrJ69==Pt1U0+MGw6u0MuNtr-DRCw@mail.gmail.com
> Signed-off-by: Aveline Noir <jm5905938@gmail.com>
> ---
> This is against mm-new ad9f2d463666. With the fix, the original test
> reports p0=p50=p100=19200 ms. The i386 kernel build, x86_64 W=1 stat.o
> build and external arithmetic checks passed.
>
> I also ran 38 before/after VM tests and 20 same-image controls. Throughput
What is 38?
> was close overall, but median kdamond CPU rose by 0.0053 and 0.0221
> percentage points of one guest CPU for stream and sparse, respectively.
> Some pairs had 5.91% lower throughput or 15.49% higher scan P99. So I can't
> yet tell whether these differences come from the patch.
Do you have a plan to run more tests to be able to tell that?
>
>
> mm/damon/stat.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/mm/damon/stat.c b/mm/damon/stat.c
> index b05b68f73e10..eccd372810d0 100644
> --- a/mm/damon/stat.c
> +++ b/mm/damon/stat.c
> @@ -114,7 +114,7 @@ static void damon_stat_set_idletime_percentiles(struct damon_ctx *c)
> {
> struct damon_region **sorted_regions, *region;
> int nr_regions;
> - unsigned long total_sz, accounted_bytes = 0;
> + unsigned long total_sz, accounted_bytes = 0, percentile;
> int err, i, next_percentile = 0;
>
> err = damon_stat_sort_regions(c, &sorted_regions, &nr_regions,
> @@ -124,7 +124,8 @@ static void damon_stat_set_idletime_percentiles(struct damon_ctx *c)
> for (i = 0; i < nr_regions; i++) {
> region = sorted_regions[i];
> accounted_bytes += region->ar.end - region->ar.start;
> - while (next_percentile <= accounted_bytes * 100 / total_sz)
> + percentile = div64_ul((u64)accounted_bytes * 100, total_sz);
> + while (next_percentile <= percentile)
Any reason to use div64_ul() instead of div_u64()?
Thanks,
SJ
[...]
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-10-16 0:21 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-15 14:52 [PATCH] mm/damon/stat: avoid 32-bit overflow in idle percentiles Aveline Noir
2026-10-15 15:05 ` sashiko-bot
2026-10-16 0:21 ` SJ Park
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox