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

[Gutierrez Asier]

On 4/28/2026 3:39 AM, SeongJae Park wrote:
> Thank you for reviewing, Asier!
> 
> On Mon, 27 Apr 2026 16:21:19 +0300 Gutierrez Asier <gutierrez.asier@xxxxxxxxxxxxxxxxxxx> 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@xxxxxxxxxx>
>>> ---
> [...]
>>> +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.
> 

Acked. I reviewed all the callers for damon_commit_ctx, and you are right,
they all destroy ctx in case of an error.

>>> +		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
> 
> [...]
> 

-- 
Asier Gutierrez
Huawei