Re: [RFC PATCH 04/19] mm/damon/core: commit probes

[SeongJae Park <sj@kernel.org>]

Thank you for reviewing, Asier!

On Mon, 27 Apr 2026 16:21:19 +0300 Gutierrez Asier <gutierrez.asier@huawei-partners.com> wrote:

> 
> 
> On 4/26/2026 11:52 PM, SeongJae Park wrote:
> > Update damon_commit_ctx() to commit installed data probes, too.
> > 
> > Signed-off-by: SeongJae Park <sj@kernel.org>
> > ---
[...]
> > +static int damon_commit_filters(struct damon_probe *dst,
> > +		struct damon_probe *src)
> > +{
> > +	struct damon_filter *dst_filter, *next, *src_filter, *new_filter;
> > +	int i = 0, j = 0;
> > +
> > +	damon_for_each_filter_safe(dst_filter, next, dst) {
> > +		src_filter = damon_nth_filter(i++, src);
> > +		if (src_filter)
> > +			damon_commit_filter(dst_filter, src_filter);
> > +		else
> > +			damon_destroy_filter(dst_filter);
> > +	}
> > +
> > +	damon_for_each_filter_safe(src_filter, next, src) {
> > +		if (j++ < i)
> > +			continue;
> > +
> > +		new_filter = damon_new_filter(src_filter->type,
> > +				src_filter->matching, src_filter->allow);
> > +		if (!new_filter)
> > +			return -ENOMEM;
> We should bail out here. If we return an error, we have already
> committed some of the filters already. If there is an error, we
> should remove any fill that was already added.

The cleanup will be done by the caller.  That is, damon_commit_ctx() caller
should destroy the ctx when the return value is imposing a commit failure.

> > +		damon_add_filter(dst, new_filter);
> > +	}
> > +	return 0;
> > +}
> > +
> > +static int damon_commit_probes(struct damon_ctx *dst, struct damon_ctx *src)
> > +{
> > +	struct damon_probe *dst_probe, *next, *src_probe, *new_probe;
> > +	int i = 0, j = 0, err;
> > +
> > +	damon_for_each_probe_safe(dst_probe, next, dst) {
> > +		src_probe = damon_nth_probe(i++, src);
> > +		if (src_probe) {
> > +			err = damon_commit_filters(dst_probe, src_probe);
> > +			if (err)
> > +				return err;
> > +		} else {
> > +			damon_destroy_probe(dst_probe);
> > +		}
> > +	}
> > +
> > +	damon_for_each_probe_safe(src_probe, next, src) {
> > +		if (j++ < i)
> > +			continue;
> > +
> > +		new_probe = damon_new_probe();
> > +		if (!new_probe)
> > +			return -ENOMEM;
> The same as before, we should bail out.

Ditto.


Thanks,
SJ

[...]