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

[SJ Park <sj@kernel.org>]

On Sat, 07 Nov 2026 21:58:17 +0000 sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 1 potential
> issue to consider:
> - [Medium] mm/damon/core: dangling charge_target_from ABA reuse in
>   damos_adjust_quota()
> --
> 
> commit 6429d6d07b9e92711137f889d5332896002d4342
> 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 damos_adjust_quota()
> to clear quota->charge_target_from and quota->charge_addr_from if the
> target was removed or has no regions left after a runtime parameter commit.
> 
> Link: https://lore.kernel.org/20261020023958.108276-1-sj@kernel.org
> 
> > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > index 046c07b86a6e7..badd0628e5eb2 100644
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
> > @@ -1686,6 +1686,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 damos_adjust_quota(struct damon_ctx *c, struct damos *s)
> >  {
> >  	struct damos_quota *quota = &s->quota;
> > @@ -1714,6 +1734,8 @@ static void damos_adjust_quota(struct damon_ctx *c, struct damos *s)
> >  		damos_set_effective_quota(quota);
> >  	}
> >  
> > +	damos_fix_stale_charge_pointers(c, s);
> > +
> >  	if (!c->ops.get_scheme_score)
> >  		return;
> 
> [Severity: Medium]
> Can damos_fix_stale_charge_pointers() match a different target if
> quota->charge_target_from is left dangling until damos_adjust_quota()
> runs?
> 
> When damon_commit_targets() removes a target via damon_destroy_target(),
> it frees dst_target without clearing quota->charge_target_from, and
> damon_commit_ctx() then calls damon_set_attrs() ->
> damos_set_next_apply_sis(), deferring damos_adjust_quota() to a later
> interval.  If a newly committed target reuses the freed slab address
> before then (or if damon_commit_target() overwrites dst_target in place
> with a different target), t == quota->charge_target_from matches the new
> target and leaves the stale charge_target_from and charge_addr_from in
> place whenever damon_nr_regions(t) > 0, causing damos_skip_charged_region()
> to skip targets and regions up to the old address.
> 
> Clearing charge_target_from and charge_addr_from directly in
> damon_commit_targets() when a target is destroyed or updated would avoid
> comparing a dangling pointer later.

Good finding, but this is not a blocker of this patch.  Occasionally starting
quota charging from unexpected target is no real problem for real DAMOS usages.

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


Thanks,
SJ