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

[sashiko-bot@kernel.org]

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.

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