Re: [PATCH v2.1] mm/damon/core: fix false positive in damos_quota_is_full() when esz is zero

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Schemes without configured quota limits (unlimited quotas) are completely skipped and never applied due to the new `!quota->esz` early exit check.
- [Medium] Incorrect quota evaluation for very small quotas (`0 < esz < min_region_sz`) breaks the early short-circuit optimization, causing wasted CPU cycles, artificially inflated statistics, and broken telemetry.
--

commit 4718f859e447852ff19b6830ab8bf79106ad37d1
Author: Liew Rui Yan <aethernet65535@gmail.com>
Subject: mm/damon/core: fix false positive in damos_quota_is_full() when esz is zero

This patch aims to fix an issue where damos_quota_is_full() returns true when
quota->esz_bp drops to zero, even if no regions have been tried. It attempts
to resolve this by adding early checks for zero quota->charged_sz and
zero quota->esz.

Cc: <stable@vger.kernel.org> # v7.2.x

> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index df22621765911..f8c784df24600 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
> @@ -2655,6 +2655,8 @@ static bool damos_quota_is_full(struct damos_quota *quota,
>  {
>  	if (!damos_quota_is_set(quota))
>  		return false;
> +	if (!quota->charged_sz)
> +		return false;

[Severity: Medium]
Could this cause an evaluation loop that degrades performance when the
configured quota is very small?

If a quota is configured such that 0 < quota->esz < min_region_sz,
quota->charged_sz starts at 0 at the beginning of the quota window.
This new check will return false, bypassing the subsequent small remaining
quota check:

    return quota->esz - quota->charged_sz < min_region_sz;

This falsely signals available quota. Later, when damon_do_apply_schemes()
calls damos_apply_scheme(), the size is calculated as:

    sz = ALIGN_DOWN(quota->esz - quota->charged_sz, c->min_region_sz);

Because esz < min_region_sz and charged_sz is 0, sz evaluates to 0. The code
then jumps to update_stat, which increments nr_tried but does not increase
charged_sz.

Since charged_sz remains 0, the next region will also evaluate to 0 and skip
charging. Won't this loop wastefully over every region in an O(N) fashion,
artificially inflating the nr_tried statistics and breaking the qt_exceeds
telemetry?

>  	if (quota->charged_sz >= quota->esz)
>  		return true;
>  	/*
> @@ -2750,6 +2752,9 @@ static void damon_do_apply_schemes(struct damon_ctx *c,
>  		if (!s->wmarks.activated)
>  			continue;
>  
> +		if (!quota->esz)
> +			continue;
> +

[Severity: High]
Does this unconditional check completely skip schemes that do not have quota
limits configured?

For unlimited schemes (schemes without configured quotas), damos_adjust_quota()
returns early without calculating esz, leaving quota->esz initialized to 0.
With this check, won't unlimited schemes silently fail to apply entirely
because the loop unconditionally continues?

>  		/* Check the quota */
>  		if (damos_quota_is_full(quota, c->min_region_sz))
>  			continue;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260902084445.27170-1-aethernet65535@gmail.com?part=1