[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <20250820221633.87243-1-sj@kernel.org>
Date: Wed, 20 Aug 2025 15:16:33 -0700
From: SeongJae Park <sj@...nel.org>
To: Quanmin Yan <yanquanmin1@...wei.com>
Cc: SeongJae Park <sj@...nel.org>,
akpm@...ux-foundation.org,
damon@...ts.linux.dev,
linux-kernel@...r.kernel.org,
linux-mm@...ck.org,
wangkefeng.wang@...wei.com,
zuoze1@...wei.com,
Andrew Paniakin <apanyaki@...zon.com>
Subject: Re: [RFC PATCH mm-next v2 12/12] mm/damon/core: prevent unnecessary overflow in damos_set_effective_quota()
+ Andrew
On Wed, 20 Aug 2025 16:06:22 +0800 Quanmin Yan <yanquanmin1@...wei.com> wrote:
> On 32-bit systems, the throughput calculation in function
> damos_set_effective_quota() is prone to unnecessary
> multiplication overflow. Using mult_frac() to fix it.
Andrew Paniakin also recently found and privately reported this issue, on 64
bit systems. This can also happen on 64-bit systems, once the charged size
exceeds ~17 TiB. On systems running for long time in production, this issue
can actually happen.
More specifically, when a DAMOS scheme having the time quota runs for long
time, throughput calculation can overflow and set esz too small. As a result,
speed of the scheme get unexpectedly slow.
Quanmin, could you please add the above paragraph on the commit message? Also,
I think this fix deserves Cc: stable@ and has no reason to be a part of this
patch series. Could you please add appripriate tags like below and post again
separately?
Fixes: 1cd243030059 ("mm/damon/schemes: implement time quota")
Reported-by: Andrew Paniakin <apanyaki@...zon.com>
Closes: N/A # privately reported
Cc: <stable@...r.kernel.org> # 5.16.x
>
> Signed-off-by: Quanmin Yan <yanquanmin1@...wei.com>
> ---
> mm/damon/core.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index 980e271e42e9..38b5f842ef30 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
> @@ -2102,8 +2102,8 @@ static void damos_set_effective_quota(struct damos_quota *quota)
>
> if (quota->ms) {
> if (quota->total_charged_ns)
> - throughput = quota->total_charged_sz * 1000000 /
> - quota->total_charged_ns;
> + throughput = mult_frac(quota->total_charged_sz, 1000000,
> + quota->total_charged_ns);
> else
> throughput = PAGE_SIZE * 1024;
> esz = min(throughput * quota->ms, esz);
> --
> 2.43.0
Thanks,
SJ
Powered by blists - more mailing lists