Re: [PATCH 05/17] mm/damon/core: introduce damon_prep struct

[SJ Park <sj@kernel.org>]

On Tue, 01 Sep 2026 04:46:01 +0000 sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Low] Missing exported cleanup API for `struct damon_prep` leads to broken abstraction on error paths.
> --
> 
> commit 046989ba790c1b12be9fe4a07f46c502a62eeb7b
> Author: SJ Park <sj@kernel.org>
> 
> mm/damon/core: introduce damon_prep struct
> 
> This commit introduces the damon_prep data structure to specify preparatory
> actions for DAMON probes. It also adds DAMON_PREP_SET_PGIDLE as the initial
> preparation action for access monitoring.
> 
> > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > index 644daf5a16560..0b396c2456d05 100644
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
> 
> [ ... ]
> 
> > @@ -111,6 +111,28 @@ int damon_select_ops(struct damon_ctx *ctx, enum damon_ops_id id)
> >  	return err;
> >  }
> >  
> > +struct damon_prep *damon_new_prep(enum damon_prep_action action)
> > +{
> > +	struct damon_prep *prep;
> > +
> > +	prep = kmalloc_obj(*prep);
> > +	if (!prep)
> > +		return NULL;
> > +	prep->action = action;
> > +	INIT_LIST_HEAD(&prep->list);
> > +	return prep;
> > +}
> 
> [Severity: Low]
> Is there a missing exported cleanup API for struct damon_prep?
> 
> Since damon_new_prep() is exposed for external modules to allocate a new
> prep structure, do they need a corresponding exported destructor (like
> damon_destroy_prep()) to clean it up if an error occurs before it is
> attached to a probe via damon_add_prep()?
> 
> Currently, damon_free_prep() is static, which forces any external caller to
> manually call kfree() on error paths, potentially breaking the subsystem's
> abstraction.

The cleanup API is not necessary.  Callers will be able to free up preps with
the context at once using damon_destroy_ctx().

> 
> > +
> > +void damon_add_prep(struct damon_probe *p, struct damon_prep *prep)
> > +{
> > +	list_add_tail(&prep->list, &p->preps);
> > +}
> > +
> > +static void damon_free_prep(struct damon_prep *p)
> > +{
> > +	kfree(p);
> > +}
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260901043417.2165-1-sj@kernel.org?part=5


Thanks,
SJ