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

[SJ Park <sj@kernel.org>]

On Tue, 08 Sep 2026 14:07:20 +0000 sashiko-bot@kernel.org wrote:

> 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?

I'm working [1] on this issue.

[1] https://lore.kernel.org/20260907165529.98874-1-sj@kernel.org

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


Thanks,
SJ