Re: [PATCH v2] mm/damon/core: fix false positive in

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The newly added early exit `if (!quota->esz)` in `damon_do_apply_schemes()` unintentionally skips and completely breaks all DAMOS schemes that do not have quotas configured.
- [Medium] The new condition `if (!quota->charged_sz)` in `damos_quota_is_full()` incorrectly skips quota exhaustion checks when a configured quota is smaller than `min_region_sz`, violating per-region quota semantics and leading to statistic inflation and wasted CPU cycles.
--

commit b189466ffcadd150b11edbb63a99f1fb0477b8b5
Author: Liew Rui Yan <aethernet65535@gmail.com>

mm/damon/core: fix false positive in

This commit aims to fix a false positive in damos_quota_is_full() when
goal_tuner is 'temporal' and the goal is achieved, causing quota->esz_bp
to drop to zero. It adds checks for zero charged_sz and zero esz to
prevent unexpectedly increasing qt_exceeds.

> 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]
Does this unconditionally bypass the remaining quota check when no regions
have been charged yet?

If a DAMOS scheme is evaluated with an effective quota (quota->esz) that
is greater than 0 but strictly less than c->min_region_sz, returning false
here skips the final check in this function:

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

Could this cause the scheme to continue evaluating all targets and regions,
only to calculate a zero size later in damos_apply_scheme() due to
insufficient quota? This would artificially inflate s->stat.nr_tried for
every region while wasting CPU cycles applying zero bytes.

>  	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]
Will this newly added check completely skip valid DAMOS schemes that simply
do not have quotas configured?

When a user creates a scheme without quota limits (ms == 0, sz == 0, and
no goals), damos_quota_init() initializes quota->esz to 0. Since
damos_adjust_quota() leaves esz unmodified for un-set quotas, esz remains 0.

Because this check happens before damos_quota_is_full() (which normally
handles non-quota schemes safely by returning false via damos_quota_is_set()),
won't this cause damon_do_apply_schemes() to unconditionally skip all
schemes that lack a quota?

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

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