linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: JaeJoon Jung <rgbi3307@gmail.com>
To: SeongJae Park <sj@kernel.org>
Cc: damon@lists.linux.dev, linux-mm@kvack.org
Subject: Re: [PATCH] mm/damon: modified damon_call_control from static to kmalloc
Date: Thu, 11 Dec 2025 16:38:46 +0900	[thread overview]
Message-ID: <CAHOvCC7epLcuK-58kaROWMof9dq3ZCm2WV218KGv7kcDMf7q-Q@mail.gmail.com> (raw)
In-Reply-To: <20251211060413.60534-1-sj@kernel.org>

On Thu, 11 Dec 2025 at 15:04, SeongJae Park <sj@kernel.org> wrote:
>
> 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?

Would you like me to send you the patch with only the comments corrected?

Thanks,
JaeJoon

>
>
> Thanks,
> SJ
>
> [...]


  reply	other threads:[~2025-12-11  7:39 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-12-06 22:47 JaeJoon Jung
2025-12-07  2:36 ` SeongJae Park
2025-12-09 12:20   ` JaeJoon Jung
2025-12-10  2:54     ` SeongJae Park
2025-12-11  3:29       ` JaeJoon Jung
2025-12-11  6:04         ` SeongJae Park
2025-12-11  7:38           ` JaeJoon Jung [this message]
2025-12-11  7:54             ` SeongJae Park
2025-12-11  8:13               ` JaeJoon Jung
2025-12-12  3:46                 ` SeongJae Park
2025-12-12  7:30                   ` JaeJoon Jung

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAHOvCC7epLcuK-58kaROWMof9dq3ZCm2WV218KGv7kcDMf7q-Q@mail.gmail.com \
    --to=rgbi3307@gmail.com \
    --cc=damon@lists.linux.dev \
    --cc=linux-mm@kvack.org \
    --cc=sj@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox