Re: [RFC PATCH v2.2 04/28] mm/damon/core: commit probes

[SeongJae Park <sj@kernel.org>]

On Fri, 15 May 2026 01:05:28 +0000 sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] In `damon_commit_probes()`, a newly allocated `new_probe` is added to the destination context before its filters are committed, leaving a partially constructed object on failure.
> --
> 
> commit 2686f1a22f9199eacddb5ee3b2612cd585a68b00
> Author: SeongJae Park <sj@kernel.org>
> 
> mm/damon/core: commit probes
> 
> This commit updates damon_commit_ctx() to additionally commit any installed
> data probes to the destination context.
> 
> > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > index 44943d654dcce..15f2795a1156f 100644
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
> [ ... ]
> > @@ -1378,6 +1413,72 @@ static int damon_commit_targets(
> [ ... ]
> > +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(src_probe, next, src) {
> > +		if (j++ < i)
> > +			continue;
> > +
> > +		new_probe = damon_new_probe();
> > +		if (!new_probe)
> > +			return -ENOMEM;
> > +		damon_add_probe(dst, new_probe);
> > +		err = damon_commit_filters(new_probe, src_probe);
> > +		if (err)
> > +			return err;
> 
> Does this leave a partially constructed probe in the destination context on
> failure?
> 
> If damon_commit_filters() returns an error, the function returns immediately,
> but new_probe has already been added to dst via damon_add_probe().

Yes, it leaves the partially constructed probe.  And the caller will destroy it
when the error is returned.  If the destination is a running DAMON context,
kdamond_fn() stops, using 'maybe_corrupted' field.

> 
> This differs from the pattern in damon_commit_targets(), where new objects
> are only added to the destination list upon success, and are destroyed if
> the commit step fails.

No, the function also returns error immediately leaving partially committed
targets on the context.


Thanks,
SJ

[...]