Re: [RFC PATCH] mm/damon: fix damos quota walk-position tracking

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The DAMOS rotating walk cursor (`walk_target_from`) becomes permanently stuck if it points to a target with zero regions, silently disabling the scheme entirely.
--

--- Patch [1]: [RFC PATCH] mm/damon: fix damos quota walk-position tracking ---
commit f919352b7b1f727f53877f4c9877e9c62ea1cfad
Author: Liew Rui Yan <aethernet65535@gmail.com>

mm/damon: fix damos quota walk-position tracking

DAMOS uses charge_target_from/charge_addr_from to remember how far a
quota-limited walk has progressed. This commit treats the fields as a
rotating walk cursor to resolve issues where regions could be skipped or
remain stuck.

> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index 644daf5a16560..bbdb530513a18 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
[ ... ]
> @@ -404,6 +404,52 @@ static bool damon_is_last_region(struct damon_region *r,
>  	return list_is_last(&r->list, &t->regions_list);
>  }
>  
> +static bool damos_completely_walked(struct damos *s, struct damon_target *t)
> +{
> +	/*
> +	 * '>=' instead of '==', because the region set of @t could have been
> +	 * shrunken by damon_set_regions() after the cursor was saved.
> +	 */
> +	return s->quota.walk_target_from == t &&
> +		s->quota.walk_addr_from >= damon_last_region(t)->ar.end;
> +}
[ ... ]
> @@ -2658,18 +2701,31 @@ static void damon_do_apply_schemes(struct damon_ctx *c,
>  		if (damos_quota_is_full(quota, c->min_region_sz))
>  			continue;
>  
> -		if (damos_skip_charged_region(t, r, s, c->min_region_sz))
> -			continue;
> -
>  		if (s->max_nr_snapshots &&
>  				s->max_nr_snapshots <= s->stat.nr_snapshots)
>  			continue;
>  
> +		/*
> +		 * Skip regions before the cursor.  If the skip makes the cursor
> +		 * reach the end of this target, rotate to the next valid target
> +		 * before continuing with the next region/scheme.
> +		 */
> +		if (damos_skip_walked_region(t, r, s, c->min_region_sz)) {
> +			damos_walk_maybe_rotate(c, t, s);

[Severity: High]
Does this code permanently skip all regions if walk_target_from points to a
target with zero regions?

Because cursor rotation only happens here inside damon_do_apply_schemes()
which is called during region iteration, if walk_target_from is a target
without any regions, this code never runs for that target.

When other targets are evaluated, damos_skip_walked_region() will continually
return true since walk_target_from != t. However, damos_walk_maybe_rotate()
won't advance the cursor because damos_completely_walked() requires
walk_target_from == t.

Can we ensure the cursor properly advances past empty targets?

> +			continue;
> +		}
> +
>  		if (damos_valid_target(c, r, s))
>  			damos_apply_scheme(c, t, r, s);
>  
>  		if (damon_is_last_region(r, t))
>  			s->stat.nr_snapshots++;
> +
> +		if (damos_quota_is_set(quota)) {
> +			quota->walk_target_from = t;
> +			quota->walk_addr_from = r->ar.end;
> +			damos_walk_maybe_rotate(c, t, s);
> +		}
>  	}
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260825124616.5129-1-aethernet65535@gmail.com?part=1