Re: [PATCH v3] mm/damon/core: fix unconditionally skip last region

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Quota tracking pointer `charge_target_from` can get permanently stuck if its corresponding target becomes invalid, freezing scheme application for all subsequent targets indefinitely.
--

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

mm/damon/core: fix unconditionally skip last region

This commit modifies damos_skip_charged_region() to only reset the quota's
charge_target_from and charge_addr_from when the last region is reached.
It ensures that the last region of a tracked target is only skipped if it
was actually applied or cannot be split.

> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index ce8c6f99106ed..89d1c77417450 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
> @@ -2452,36 +2452,39 @@ static bool damos_skip_charged_region(struct damon_target *t,
>  {
>  	struct damos_quota *quota = &s->quota;
>  	unsigned long sz_to_skip;
> +	bool skip = false;
>  
>  	/* Skip previously charged regions */
>  	if (quota->charge_target_from) {
>  		if (t != quota->charge_target_from)
>  			return true;

[Severity: High]
This isn't a bug introduced by this patch, but is it possible for
quota->charge_target_from to remain stuck indefinitely if its corresponding
target becomes invalid?

Looking at kdamond_apply_schemes() in mm/damon/core.c, if a target becomes
invalid, it is skipped and damos_apply_target() is never called for it:

    damon_for_each_target(t, c) {
        if (c->ops.target_valid && c->ops.target_valid(t) == false)
            continue;
        damos_apply_target(c, t, max_region_sz);
    }

If the previous quota window ended with quota->charge_target_from pointing
to this now-invalid target, wouldn't the 'return true' here in
damos_skip_charged_region() cause all regions of all subsequent valid targets
to be skipped?

Because the invalid target is never processed, t will never equal
quota->charge_target_from again.

Does this mean the pointer is never cleared, freezing scheme application for
all targets?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260908134739.96919-1-sj@kernel.org?part=1