Re: [PATCH 6.6.y v3 2/2] mm/damon/core: fix unconditionally skip last region

[SJ Park <sj@kernel.org>]

On Fri, 09 Oct 2026 14:05:09 +0000 sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] mm/damon: dangling charge_target_from stalls schemes on target deletion
> - [High] mm/damon: missing charge_target_from reset when target has no regions
> --
> 
> commit 34b3a9a3bd49604e23a1fa56349694c2bc0db52b
> Author: Liew Rui Yan <aethernet65535@gmail.com>
> 
> mm/damon/core: fix unconditionally skip last region
> 
> This commit modifies the quota charge skipping logic to only reset
> charge_target_from and charge_addr_from when the last region is actually
> reached. It addresses a minor issue where the last region of a tracked target
> could be unconditionally skipped and reset even when it had not been processed
> yet.
> 
> Link: https://lore.kernel.org/20260908134739.96919-1-sj@kernel.org
> 
> > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > index c86cac855b9b0..c459eae4bbaeb 100644
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
> > @@ -884,36 +884,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]
> Does this logic stall the scheme indefinitely if charge_target_from points to a
> destroyed target?
> 
> If a target is removed during a sysfs commit, damon_sysfs_set_targets()
> dynamically destroys it:
> 
> mm/damon/sysfs.c:damon_sysfs_set_targets() {
> [ ... ]
> 		} else {
> 			if (damon_target_has_pid(ctx))
> 				put_pid(t->pid);
> 			damon_destroy_target(t);
> 		}
> [ ... ]
> }
> 
> However, damon_sysfs_set_schemes() merely updates existing schemes in-place and
> fails to clear the quota->charge_target_from pointer if it points to the deleted
> target.
> 
> Because this saved target was deleted and removed from the active target list,
> would this condition evaluate to true for all remaining targets, permanently
> skipping all regions and never reaching the logic to clear the dangling pointer?
> 
> [Severity: High]
> Does this also cause a stall if a target loses all of its regions?
> 
> If quota->charge_target_from points to a target that loses all its regions (for
> example, if all VMAs are unmapped and the task dies), damon_for_each_region()
> will execute zero times for that target:
> 
> mm/damon/core.c:kdamond_apply_schemes() {
> [ ... ]
> 	damon_for_each_target(t, c) {
> 		damon_for_each_region(r, t)
> 			damon_do_apply_schemes(c, t, r);
> 	}
> [ ... ]
> }
> 
> Would this bypass the invocation of damos_skip_charged_region() entirely for the
> charged target, preventing charge_target_from from being cleared, and causing
> all subsequent targets to be skipped indefinitely because they do not match the
> uncleared charge_target_from pointer?

Pre-existing issue that also exist in the mainline.  Hence no blocker of this
backport.  I will separately work on the issue.

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


Thanks,
SJ