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

[JaeJoon Jung <rgbi3307@gmail.com>]

Hello, SeongJae,

Thank you for your detailed feedback.
My patch has the advantage of removing the use of
the damon_call_control.dealloc_on_cancel variable.
Other than that, I agree with you pointed out.
It's not a patch that significantly improves functionality,
so I'm fine with doing whatever you decide.

Thanks,
JaeJoon

On Sun, 7 Dec 2025 at 11:36, SeongJae Park <sj@kernel.org> wrote:
>
> On Sun,  7 Dec 2025 07:47:23 +0900 JaeJoon Jung <rgbi3307@gmail.com> wrote:
>
> > The damon_call_control structure is used by all damon modules.
> > In mm/damon/sysfs.c, it is dynamically allocated with kmalloc(),
>
> It is only for refresh_ms handling damon_call_control.
> damon_sysfs_damon_call() doesn't use kmalloc().
>
> > and in
> > the remaining modules, it is statically coded.
>
> I try not to use dynamic allocation when possible and *reasonable*, since doing
> so eliminates the chances of bugs from dynamic allocation handling mistakes.
>
> > There are many daemon
> > modules and they are designed to be used only when needed.  It is more
> > efficient to use kmalloc dynamically whenever needed as in damon/sysfs.c.
>
> I agree that is more efficient in terms of kernel size.  Nevertheless, I don't
> think it is a significant improvement, since it is only single data structure
> per the DAMON module.  The total number of DAMON modules is only five.  Two
> among those are samples.
>
> > Using damon_call_control dynamically and consistently has the advantage of
> > eliminating the dealloc_on_cancel member variable condition and reducing
> > code size.
>
> Disadvantage of this change is that it is adding four more dynamic allocations
> that we need to cautiously maintain.  It is not significantly increasing the
> maintenance overhead.  But, the advantage (kernel size reduction) also seems
> only modest, if I'm not wrong.
>
> So I'm sadly have to say I don't think this change is really needed.
>
> Please let me know if I'm missing something, though.
>
>
> Thanks,
> SJ
>
> [...]