[PATCH] mm/damon/stat: avoid 32-bit overflow in idle percentiles

[PATCH] mm/damon/stat: avoid 32-bit overflow in idle percentiles

[Aveline Noir]

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;

Re: [PATCH] mm/damon/stat: avoid 32-bit overflow in idle percentiles

[SJ Park]

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

[...]

Re: [PATCH] mm/damon/stat: avoid 32-bit overflow in idle percentiles

[Aveline Noir]

Hi SJ,

On Thu, Oct 15, 2026 at 05:21:29PM -0700, SJ Park wrote:
> 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.

The patch was already prepared and additional tests were underway when
your reply to the bug report arrived, so I went ahead and sent it.
I'll leave more time for discussion before sending follow-up patches.

> What is 38?

38 individual VM runs, forming 19 before/after pairs. The 20 same-image
runs formed 10 control pairs.

> Do you have a plan to run more tests to be able to tell that?

I've run longer VM tests, but even same-kernel runs vary substantially,
so the overall patch effect remains unclear. In a fixed-input 32-bit
userspace test with 10 regions over 256 MiB, the median increase was
227 ns/call (+42%). The old code updated 15 entries, versus 101 with the
fix. I haven't separated the extra updates from arithmetic cost.
Maybe we could record region counts and sizes alongside VM timings to
check whether both versions are doing comparable work.

> Any reason to use div64_ul() instead of div_u64()?

total_sz is unsigned long. div_u64() would truncate its divisor on 64-bit
systems if it exceeds U32_MAX. div64_ul() preserves that width and maps
to div_u64() on 32-bit.

Thanks,
Aveline Noir

Re: [PATCH] mm/damon/stat: avoid 32-bit overflow in idle percentiles

[SJ Park]

On Fri, 16 Oct 2026 17:22:58 +0800 Aveline Noir <jm5905938@gmail.com> wrote:

> Hi SJ,
> 
> On Thu, Oct 15, 2026 at 05:21:29PM -0700, SJ Park wrote:
> > 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.
> 
> The patch was already prepared and additional tests were underway when
> your reply to the bug report arrived, so I went ahead and sent it.
> I'll leave more time for discussion before sending follow-up patches.
> 
> > What is 38?
> 
> 38 individual VM runs, forming 19 before/after pairs. The 20 same-image
> runs formed 10 control pairs.
> 
> > Do you have a plan to run more tests to be able to tell that?
> 
> I've run longer VM tests, but even same-kernel runs vary substantially,
> so the overall patch effect remains unclear. In a fixed-input 32-bit
> userspace test with 10 regions over 256 MiB, the median increase was
> 227 ns/call (+42%). The old code updated 15 entries, versus 101 with the
> fix. I haven't separated the extra updates from arithmetic cost.
> Maybe we could record region counts and sizes alongside VM timings to
> check whether both versions are doing comparable work.
> 
> > Any reason to use div64_ul() instead of div_u64()?
> 
> total_sz is unsigned long. div_u64() would truncate its divisor on 64-bit
> systems if it exceeds U32_MAX. div64_ul() preserves that width and maps
> to div_u64() on 32-bit.

All answers make sense.  Thank you.


Thanks,
SJ

[...]

Re: [PATCH] mm/damon/stat: avoid 32-bit overflow in idle percentiles

[SJ Park]

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

From the next time, please give context about what the link is.

> Signed-off-by: Aveline Noir <jm5905938@gmail.com>

Reviewed-by: SJ Park <sj@kernel.org>

I applied  this patch to damon/next [1] tree, after slightly modifying the
commit message for the abovely mentioned small thing.  We are now quite close
to next merge window.  It may better to focus on making mm.git more stabilized
and therefore ready for the next merge window, rather than adding more changes
that are not really urgent.  I understand this series is not really urgent.
Hence, Andrew might not add this patch until next -rc1 release.  In the case, I
will request adding this to mm.git after next -rc1 release.  So, no action from
your side is needed for now.  Let me know if you think this is really  urgent.

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

The above empty space before 'diff --git' line was making 'git am' of this
patch fails.  I manually fixed it.  Please do not add such empty space or
something that could make 'git am' fails, from the next time.

[1] https://origin.kernel.org/doc/html/latest/mm/damon/maintainer-profile.html#scm-trees


Thanks,
SJ

[...]