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

[SJ Park <sj@kernel.org>]

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

[...]