DAMON development mailing list
 help / color / mirror / Atom feed
* [PATCH] samples/damon/prcl: add module parameter description for target_pid
@ 2026-07-16  9:14 Enze Li
  2026-07-16 13:54 ` SJ Park
  0 siblings, 1 reply; 4+ messages in thread
From: Enze Li @ 2026-07-16  9:14 UTC (permalink / raw)
  To: sj, damon; +Cc: linux-mm, lienze, enze.li

Document 'target_pid' via MODULE_PARM_DESC so its meaning is visible
through kernel command line usage and sysfs interface.

Signed-off-by: Enze Li <lienze@kylinos.cn>
---
 samples/damon/prcl.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/samples/damon/prcl.c b/samples/damon/prcl.c
index 842099bd6228..2592fc53e9d7 100644
--- a/samples/damon/prcl.c
+++ b/samples/damon/prcl.c
@@ -18,6 +18,7 @@
 
 static int target_pid __read_mostly;
 module_param(target_pid, int, 0600);
+MODULE_PARM_DESC(target_pid, "Target PID of monitored process");
 
 static int damon_sample_prcl_enable_store(
 		const char *val, const struct kernel_param *kp);
-- 
2.54.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] samples/damon/prcl: add module parameter description for target_pid
  2026-07-16  9:14 [PATCH] samples/damon/prcl: add module parameter description for target_pid Enze Li
@ 2026-07-16 13:54 ` SJ Park
  2026-07-17  8:46   ` Enze Li
  0 siblings, 1 reply; 4+ messages in thread
From: SJ Park @ 2026-07-16 13:54 UTC (permalink / raw)
  To: Enze Li; +Cc: SJ Park, damon, linux-mm, enze.li

Hello Enze,


Thank you for this patch!

On Thu, 16 Jul 2026 17:14:44 +0800 Enze Li <lienze@kylinos.cn> wrote:

> Document 'target_pid' via MODULE_PARM_DESC so its meaning is visible
> through kernel command line usage and sysfs interface.

But...  It is a static module.  Is there a way to read this documentation from
the user space?  If there is no such a way, I don't really think this change is
needed.  If we want to document this, we could use comments.

I show 'enabled' parameter has MODULE_PARM_DESC().  If the inconsistency is a
problem, I think it might be better to consistently remove MODULE_PARM_DESC().
It may also be beneficial to comment why we don't have MODULE_PARM_DESC()s on
the module.  What do you think?


Thanks,
SJ

[...]

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] samples/damon/prcl: add module parameter description for target_pid
  2026-07-16 13:54 ` SJ Park
@ 2026-07-17  8:46   ` Enze Li
  2026-07-17 14:23     ` SJ Park
  0 siblings, 1 reply; 4+ messages in thread
From: Enze Li @ 2026-07-17  8:46 UTC (permalink / raw)
  To: SJ Park; +Cc: damon, linux-mm, enze.li


Hi SJ,

Thank you for your review.

On Thu, Jul 16 2026 at 06:54:37 AM -0700, SJ Park wrote:
> Hello Enze,
>
>
> Thank you for this patch!
>
> On Thu, 16 Jul 2026 17:14:44 +0800 Enze Li <lienze@kylinos.cn> wrote:
>
>> Document 'target_pid' via MODULE_PARM_DESC so its meaning is visible
>> through kernel command line usage and sysfs interface.
>
> But...  It is a static module.  Is there a way to read this documentation from
> the user space?  If there is no such a way, I don't really think this change is
> needed.  If we want to document this, we could use comments.

AFAIK, MODULE_PARM_DESC() is for loadable modules.  For builtins, it’s
effectively just a comment.  That said, if DAMON gains loadable module
support later, having these descriptors in place means we won’t need to
touch the code. WDYT?

Thanks,
Enze

<...>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] samples/damon/prcl: add module parameter description for target_pid
  2026-07-17  8:46   ` Enze Li
@ 2026-07-17 14:23     ` SJ Park
  0 siblings, 0 replies; 4+ messages in thread
From: SJ Park @ 2026-07-17 14:23 UTC (permalink / raw)
  To: Enze Li; +Cc: SJ Park, damon, linux-mm, enze.li

On Fri, 17 Jul 2026 16:46:26 +0800 Enze Li <lienze@kylinos.cn> wrote:

> 
> Hi SJ,
> 
> Thank you for your review.
> 
> On Thu, Jul 16 2026 at 06:54:37 AM -0700, SJ Park wrote:
> > Hello Enze,
> >
> >
> > Thank you for this patch!
> >
> > On Thu, 16 Jul 2026 17:14:44 +0800 Enze Li <lienze@kylinos.cn> wrote:
> >
> >> Document 'target_pid' via MODULE_PARM_DESC so its meaning is visible
> >> through kernel command line usage and sysfs interface.
> >
> > But...  It is a static module.  Is there a way to read this documentation from
> > the user space?  If there is no such a way, I don't really think this change is
> > needed.  If we want to document this, we could use comments.
> 
> AFAIK, MODULE_PARM_DESC() is for loadable modules.  For builtins, it’s
> effectively just a comment.

Thank you for clarifying, Enze.

> That said, if DAMON gains loadable module
> support later, having these descriptors in place means we won’t need to
> touch the code. WDYT?

I think it is not too late to add MODULE_PARM_DESC() when we add loadable
modules support.  As of now, no clear needs for loadable module support have
reported, though.


Thank,
SJ

[...]

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-07-17 14:23 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-16  9:14 [PATCH] samples/damon/prcl: add module parameter description for target_pid Enze Li
2026-07-16 13:54 ` SJ Park
2026-07-17  8:46   ` Enze Li
2026-07-17 14:23     ` SJ Park

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox