mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH 1/2] rvgen: validate the monitor type
@ 2026-11-12 10:20 Chaosheng Qu
  2026-11-12 11:23 ` Gabriele Monaco
  0 siblings, 1 reply; 2+ messages in thread
From: Chaosheng Qu @ 2026-11-12 10:20 UTC (permalink / raw)
  To: Steven Rostedt, Gabriele Monaco; +Cc: linux-trace-kernel, linux-kernel

From ef3ed8fae481c0ed3dd16c881aae455be9e92f2e Mon Sep 17 00:00:00 2001
From: Chaosheng Qu <chaosheng.qu.linux@gmail.com>
Date: Thu, 12 Nov 2026 14:26:10 +0800
Subject: [PATCH 1/2] rvgen: validate the monitor type

The monitor type is pasted into the generated monitor as RV_MON_<TYPE>.
Nothing checks it, so a typo yields a monitor that cannot build and the
tool still exits 0:

    $ rvgen monitor -c da -s test_da.dot -t bogus_type -n bm
    $ grep RV_MON_TYPE bm/bm.c
    #define RV_MON_TYPE RV_MON_BOGUS_TYPE

include/linux/rv.h defines only RV_MON_GLOBAL, RV_MON_PER_CPU,
RV_MON_PER_TASK and RV_MON_PER_OBJ, so that macro never resolves.

The accepted set is already in Monitor.monitor_types, which the help text
prints.  Pass it to argparse as choices.

Signed-off-by: Chaosheng Qu <chaosheng.qu.linux@gmail.com>
Assisted-by: LLM
---
 tools/verification/rvgen/__main__.py | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/verification/rvgen/__main__.py b/tools/verification/rvgen/__main__.py
index 246b43fa2..6053db5e5 100644
--- a/tools/verification/rvgen/__main__.py
+++ b/tools/verification/rvgen/__main__.py
@@ -38,6 +38,7 @@ if __name__ == '__main__':
     monitor_parser.add_argument('-s', "--spec", dest="spec", required=True,
                                 help="Monitor specification file")
     monitor_parser.add_argument('-t', "--monitor_type", dest="monitor_type", required=True,
+                                choices=list(Monitor.monitor_types.keys()),
                                 help=f"Available options: {', '.join(Monitor.monitor_types.keys())}")
 
     container_parser = subparsers.add_parser("container", parents=[parent_parser])
-- 
2.43.0



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

* Re: [PATCH 1/2] rvgen: validate the monitor type
  2026-11-12 10:20 [PATCH 1/2] rvgen: validate the monitor type Chaosheng Qu
@ 2026-11-12 11:23 ` Gabriele Monaco
  0 siblings, 0 replies; 2+ messages in thread
From: Gabriele Monaco @ 2026-11-12 11:23 UTC (permalink / raw)
  To: Chaosheng Qu, Steven Rostedt; +Cc: linux-trace-kernel, linux-kernel

On Thu, 2026-11-12 at 18:20 +0800, Chaosheng Qu wrote:
> From ef3ed8fae481c0ed3dd16c881aae455be9e92f2e Mon Sep 17 00:00:00 2001
> From: Chaosheng Qu <chaosheng.qu.linux@gmail.com>
> Date: Thu, 12 Nov 2026 14:26:10 +0800
> Subject: [PATCH 1/2] rvgen: validate the monitor type
> 
> The monitor type is pasted into the generated monitor as RV_MON_<TYPE>.
> Nothing checks it, so a typo yields a monitor that cannot build and the
> tool still exits 0:
> 
>     $ rvgen monitor -c da -s test_da.dot -t bogus_type -n bm
>     $ grep RV_MON_TYPE bm/bm.c
>     #define RV_MON_TYPE RV_MON_BOGUS_TYPE
> 
> include/linux/rv.h defines only RV_MON_GLOBAL, RV_MON_PER_CPU,
> RV_MON_PER_TASK and RV_MON_PER_OBJ, so that macro never resolves.
> 
> The accepted set is already in Monitor.monitor_types, which the help text
> prints.  Pass it to argparse as choices.
> 
> Signed-off-by: Chaosheng Qu <chaosheng.qu.linux@gmail.com>
> Assisted-by: LLM

Looks neat, thanks!

Some minor issues on the submission itself. Whenever you send a multi-patch
series make sure the patches are part of the same mail thread, this happens
automatically if you use git workflow (format-patch, send-email) [1].

All tools expect that so it's much easier for everyone to see and apply the
entire series.

You'd normally also add a cover letter, but for a simple series like yours where
1 adds the feature and 2 tests it, I'd say, you can skip it.

Essentially if you want to do it manually, the In-Reply-To of your 2/2 should be
the Message-ID of 1/2 (or of 0/2, the cover letter), but I strongly advise you
to use git tools.

Also, please use consistent commit tags, so both your patches should have
verification/rvgen: instead of whatever you put. If in doubt, use git log --
oneline <modified files> to see.

Finally capitalise after the tag (e.g. uppercase Validate).

TL;DR please send a V2 where both patches are in the same thread and the commit
messages are:
verification/rvgen: Validate the monitor type
verification/rvgen: Cover an invalid monitor type

Thanks,
Gabriele

[1] - https://www.kernel.org/doc/html/latest/process/submitting-patches.html

> ---
>  tools/verification/rvgen/__main__.py | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/tools/verification/rvgen/__main__.py
> b/tools/verification/rvgen/__main__.py
> index 246b43fa2..6053db5e5 100644
> --- a/tools/verification/rvgen/__main__.py
> +++ b/tools/verification/rvgen/__main__.py
> @@ -38,6 +38,7 @@ if __name__ == '__main__':
>      monitor_parser.add_argument('-s', "--spec", dest="spec", required=True,
>                                  help="Monitor specification file")
>      monitor_parser.add_argument('-t', "--monitor_type", dest="monitor_type",
> required=True,
> +                                choices=list(Monitor.monitor_types.keys()),
>                                  help=f"Available options: {',
> '.join(Monitor.monitor_types.keys())}")
>  
>      container_parser = subparsers.add_parser("container",
> parents=[parent_parser])


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

end of thread, other threads:[~2026-11-12 11:23 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-11-12 10:20 [PATCH 1/2] rvgen: validate the monitor type Chaosheng Qu
2026-11-12 11:23 ` Gabriele Monaco

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

all inboxes | Powered by JetHome®