Re: [PATCH v2] 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] A preexisting bug causes DAMOS scheme application to permanently stall for all subsequent targets if `quota->charge_target_from` points to a target that becomes invalid or loses all its regions.
--

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

mm/damon/core: fix unconditionally skip last region

This patch fixes an issue where the last region of a tracked target could
be unconditionally skipped and reset without being processed when a scheme
quota is exhausted. By modifying the reset logic, it ensures the last region
is appropriately handled rather than always skipped.

> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index 644daf5a16560..fb76b530bbc68 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
> @@ -2342,36 +2342,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 does this logic cause all
subsequent targets to permanently stall if the target currently stored in
quota->charge_target_from becomes invalid?

If the monitored process dies before the next window, kdamond_apply_schemes()
appears to bypass it without clearing the saved target:

mm/damon/core.c:kdamond_apply_schemes() {
    ...
		if (c->ops.target_valid && c->ops.target_valid(t) == false)
			continue;
		damos_apply_target(c, t, max_region_sz);
    ...
}

Because the target is skipped entirely, we never call damos_apply_target()
and never reach the clearing mechanism inside damos_skip_charged_region().

When this happens, wouldn't all other valid targets continue to evaluate
(t != quota->charge_target_from) as true here, permanently skipping scheme
applications?

It appears a local user could reliably trigger this by setting a restrictive
quota on a memory-intensive process they own, waiting for the quota to
exhaust, and then terminating the process.

[ ... ]

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