Re: (sashiko review) [RFC PATCH v5 02/11] mm/damon/core: merge quota-sliced regions back

SeongJae Park
2026-04-11T16:33:53Z

On Fri, 10 Apr 2026 16:55:26 -0700 SeongJae Park <sj@kernel.org> wrote:
[...]
> > Should the merge condition verify that the contiguous slices share identical
> > state before merging, perhaps by checking if (orig_region->age != r->age)?
>
> Good finding. I was thinking the scheme will anyway be applied, once the
> quota-split is happened. But I was wrong, since DAMOS core filter is applied
> after the split operation. Also, age is not reset if the action is DAMOS_STAT.
>
> I will address this in the next revision, like below.
>
> '''
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
> @@ -2439,35 +2439,32 @@ static void damon_do_apply_schemes(struct damon_ctx *c,
>
> static void damos_apply_target(struct damon_ctx *c, struct damon_target *t)
> {
> - struct damon_region *r, *orig_region = NULL;
> - unsigned long orig_end_addr;
> + struct damon_region *r;
>
> damon_for_each_region(r, t) {
> + struct damon_region *prev_r;
> +
> + damon_do_apply_schemes(c, t, r);
> /*
> - * damon_do_apply_schemes() could split the region for the
> + * damon_do_apply_scheems() could split the region for the
> * quota. Keeping the new slices is an overhead. Merge back
> - * the slices into the original region if there is no reason to
> - * keep those.
> + * the slices into the previous region if it doesn't lose any
> + * information.
> */
> - if (!orig_region || orig_end_addr <= r->ar.start) {
> - orig_region = r;
> - orig_end_addr = r->ar.end;
> - }
> - damon_do_apply_schemes(c, t, r);
> - if (r == orig_region)
> + if (damon_first_region(t) == r)
> continue;
> - /*
> - * If no scheme was applied to the sliced region, the age of
> - * the slice ain't be reset. Don't merge that back.
> - * Otherwise, the monitored information of the region is lost.
> - */
> - if (r->age) {
> - orig_region = NULL;
> + prev_r = damon_prev_region(r);
> + if (prev_r->ar.end != r->ar.start)
> continue;
> - }
> - orig_region->ar.end = r->ar.end;
> + if (prev_r->age != r->age)
> + continue;
> + if (prev_r->last_nr_accesses != r->last_nr_accesses)
> + continue;
> + if (prev_r->nr_accesses != r->nr_accesses)
> + continue;
> + prev_r->ar.end = r->ar.end;
> damon_destroy_region(r, t);
> - r = orig_region;
> + r = prev_r;
> }
> }

This could result in too aggressive merging, that can make the number of
regions lower than the min_nr_regions. Actually this makes one of damo tests
fail. I will fix this by using damon_region_sz_limit().

Thanks,
SJ …