Re: [PATCH] mm/damon: modified damon_call_control from static to kmalloc

[SeongJae Park <sj@kernel.org>]

On Thu, 11 Dec 2025 12:29:59 +0900 JaeJoon Jung <rgbi3307@gmail.com> wrote:

> On Wed, 10 Dec 2025 at 11:54, SeongJae Park <sj@kernel.org> wrote:
> >
> > On Tue, 9 Dec 2025 21:20:42 +0900 JaeJoon Jung <rgbi3307@gmail.com> wrote:
[...]
> I suggested removing the dealloc_on_cancel condition from the patch.
> When using the damon_call_control structure, if you only use kmalloc()/kfree()
> when canceled without dealloc_on_cancel.  However, currently, in other modules,
> since it is static struct damon_call_control { ... }, kfree() is not allowed,
> so dealloc_on_cancel condition is additionally required.
> Because kmalloc()/kfree() and static are mixed together,
> dealloc_on_cancel condition is required.
> As you pointed out in the previous email, kmalloc()/kfree() has the burden of
> memory allocation and deallocation, and static has the disadvantage of
> increasing
> the code size.  Both have their pros and cons.
> Among these, I proposed unifying them into kmalloc()/kfree() methods and
> removing the dealloc_on_cancel

Thank you for kindly clarifying this, JeaJoon!

> 
> >
> > Meanwhile I find the feature might look complicated, or not well documented.
> > Specifically, dealloc_on_cancel should be set on only dynamic-allocated
> > damon_call_control object, but that is not well documented on the kernel-doc
> > comments.  Are you saying removing it is an advantage because it makes reading
> > code easier?  If that's the case, how about improving the documentation?
> >
> 
> dealloc_on_cancel is only used in mm/damon/sysfs.c.  Here, since kmalloc
> the damon_call_control structure, the dealloc_on_cancel=true condition is
> required.  If you want to keep the current code as is and only change the
> comment, it would be clearer to comment it out like this:
> 
> De-allocate when canceled.
> --> To perform kfree() if allocated with kmalloc() when canceled.

Sounds good.  Would you mind sending a patch for that?


Thanks,
SJ

[...]