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

Gutierrez Asier
2026-04-28T13:13:07Z

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@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.
>

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