Re: [RFC PATCH 6.1.y 2/2] mm/damon/core: remove unreachable damos charge_{target,addr}_from

[SJ Park <sj@kernel.org>]

On Sat, 07 Nov 2026 23:02:33 +0000 sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 1 potential
> issue to consider:
> - [Medium] mm/damon/core: stale charge_target_from ABA reuse when scheme
>   is inactive
> --
> 
> commit 853b3e89da132f2ce03750bcf6dfc1f62a70d261
> Author: SJ Park <sj@kernel.org>
> 
> mm/damon/core: remove unreachable damos charge_{target,addr}_from
> 
> This commit adds damos_fix_stale_charge_pointers() to clear
> quota->charge_target_from and quota->charge_addr_from when the referenced
> target has been removed or no longer has any regions.
> 
> Link: https://lore.kernel.org/20261020023958.108276-1-sj@kernel.org
> 
> > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > index 9206737d05ae7..01f93146b92fe 100644
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
> > @@ -860,6 +860,26 @@ static void damos_set_effective_quota(struct damos_quota *quota)
> >  	quota->esz = esz;
> >  }
> >  
> > +static void damos_fix_stale_charge_pointers(struct damon_ctx *c,
> > +		struct damos *s)
> > +{
> > +	struct damos_quota *quota;
> > +	struct damon_target *t;
> > +
> > +	quota = &s->quota;
> > +	if (!quota->charge_target_from)
> > +		return;
> > +	damon_for_each_target(t, c) {
> > +		if (t == quota->charge_target_from) {
> > +			if (damon_nr_regions(t) > 0)
> > +				return;
> > +			break;
> > +		}
> > +	}
> > +	quota->charge_target_from = NULL;
> > +	quota->charge_addr_from = 0;
> > +}
> > +
> >  static void kdamond_apply_schemes(struct damon_ctx *c)
> >  {
> >  	struct damon_target *t;
> > @@ -890,6 +910,8 @@ static void kdamond_apply_schemes(struct damon_ctx *c)
> >  			damos_set_effective_quota(quota);
> >  		}
> >  
> > +		damos_fix_stale_charge_pointers(c, s);
> > +
> >  		if (!c->ops.get_scheme_score)
> >  			continue;
> >  
> 
> [Severity: Medium]
> Can quota->charge_target_from match a newly allocated target due to slab
> address reuse if the old target was freed while the scheme was inactive?
> 
> kdamond_apply_schemes() skips inactive schemes at the !s->wmarks.activated
> check before reaching damos_fix_stale_charge_pointers(), and
> damon_sysfs_set_targets() frees removed targets via damon_destroy_target()
> without clearing charge_target_from.  If an online commit removes the
> target stored in charge_target_from while the scheme is deactivated by
> watermarks and a later commit allocates a new target at the same address,
> damos_fix_stale_charge_pointers() will see t == quota->charge_target_from
> with regions present and leave the stale charge_target_from and
> charge_addr_from in place.  When the scheme reactivates,
> damon_do_apply_schemes() will skip targets before the new target and
> regions up to the old charge_addr_from.

Good finding but no real issue.  Occasionally starting quota charging from an
unexpected target is no real issue for DAMOS in my opinion.  So, no blocker.

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


Thanks,
SJ