The current behaviour of rvgen when running with the -a option is to
append the necessary lines at the end of the configuration for Kconfig,
Makefile and tracepoints.
This is not always the desired behaviour in case of nested monitors:
while tracepoints are not affected by nesting and the Makefile's only
requirement is that the parent monitor is built before its children, in
the Kconfig it is better to have children defined right after their
parent, otherwise the result has wrong indentation:
[*] foo_parent monitor
[*] foo_child1 monitor
[*] foo_child2 monitor
[*] bar_parent monitor
[*] bar_child1 monitor
[*] bar_child2 monitor
[*] foo_child3 monitor
[*] foo_child4 monitor
Adapt rvgen to look for a different marker for nested monitors in the
Kconfig file and append the line right after the last sibling, instead
of the last monitor.
Also add the marker when creating a new parent monitor.
Signed-off-by: Gabriele Monaco <gmonaco@redhat.com>
---
kernel/trace/rv/Kconfig | 5 +++++
tools/verification/rvgen/rvgen/container.py | 13 +++++++++++++
tools/verification/rvgen/rvgen/generator.py | 13 ++++++++-----
3 files changed, 26 insertions(+), 5 deletions(-)
diff --git a/kernel/trace/rv/Kconfig b/kernel/trace/rv/Kconfig
index c11bf7e61ebf0..26017378f79b8 100644
--- a/kernel/trace/rv/Kconfig
+++ b/kernel/trace/rv/Kconfig
@@ -43,6 +43,7 @@ config RV_PER_TASK_MONITORS
source "kernel/trace/rv/monitors/wip/Kconfig"
source "kernel/trace/rv/monitors/wwnr/Kconfig"
+
source "kernel/trace/rv/monitors/sched/Kconfig"
source "kernel/trace/rv/monitors/tss/Kconfig"
source "kernel/trace/rv/monitors/sco/Kconfig"
@@ -50,9 +51,13 @@ source "kernel/trace/rv/monitors/snroc/Kconfig"
source "kernel/trace/rv/monitors/scpd/Kconfig"
source "kernel/trace/rv/monitors/snep/Kconfig"
source "kernel/trace/rv/monitors/sncid/Kconfig"
+# Add new sched monitors here
+
source "kernel/trace/rv/monitors/rtapp/Kconfig"
source "kernel/trace/rv/monitors/pagefault/Kconfig"
source "kernel/trace/rv/monitors/sleep/Kconfig"
+# Add new rtapp monitors here
+
# Add new monitors here
config RV_REACTORS
diff --git a/tools/verification/rvgen/rvgen/container.py b/tools/verification/rvgen/rvgen/container.py
index 47d8ab2ad3ec4..fee493f89fde6 100644
--- a/tools/verification/rvgen/rvgen/container.py
+++ b/tools/verification/rvgen/rvgen/container.py
@@ -20,3 +20,16 @@ class Container(generator.RVGenerator):
main_h = self.main_h
main_h = main_h.replace("%%MODEL_NAME%%", self.name)
return main_h
+
+ def fill_kconfig_tooltip(self):
+ """Override to produce a marker for this container in the Kconfig"""
+ container_marker = f"# Add new {self.name} monitors here\n"
+ if self.auto_patch:
+ self._patch_file("Kconfig",
+ "# Add new monitors here", "")
+ result = super().fill_kconfig_tooltip()
+ if self.auto_patch:
+ self._patch_file("Kconfig",
+ "# Add new monitors here", container_marker)
+ return result
+ return result + container_marker
diff --git a/tools/verification/rvgen/rvgen/generator.py b/tools/verification/rvgen/rvgen/generator.py
index 19d0078a38032..ac97c4505ff00 100644
--- a/tools/verification/rvgen/rvgen/generator.py
+++ b/tools/verification/rvgen/rvgen/generator.py
@@ -137,7 +137,8 @@ class RVGenerator:
kconfig = kconfig.replace("%%MONITOR_DEPS%%", monitor_deps)
return kconfig
- def __patch_file(self, file, marker, line):
+ def _patch_file(self, file, marker, line):
+ assert(self.auto_patch)
file_to_patch = os.path.join(self.rv_dir, file)
content = self._read_file(file_to_patch)
content = content.replace(marker, line + "\n" + marker)
@@ -146,7 +147,7 @@ class RVGenerator:
def fill_tracepoint_tooltip(self):
monitor_class_type = self.fill_monitor_class_type()
if self.auto_patch:
- self.__patch_file("rv_trace.h",
+ self._patch_file("rv_trace.h",
"// Add new monitors based on CONFIG_%s here" % monitor_class_type,
"#include <monitors/%s/%s_trace.h>" % (self.name, self.name))
return " - Patching %s/rv_trace.h, double check the result" % self.rv_dir
@@ -158,8 +159,10 @@ Add this line where other tracepoints are included and %s is defined:
def fill_kconfig_tooltip(self):
if self.auto_patch:
- self.__patch_file("Kconfig",
- "# Add new monitors here",
+ # monitors with a container should stay together in the Kconfig
+ self._patch_file("Kconfig",
+ "# Add new %smonitors here" %
+ (self.parent + " " if self.parent else ""),
"source \"kernel/trace/rv/monitors/%s/Kconfig\"" % (self.name))
return " - Patching %s/Kconfig, double check the result" % self.rv_dir
@@ -172,7 +175,7 @@ source \"kernel/trace/rv/monitors/%s/Kconfig\"
name = self.name
name_up = name.upper()
if self.auto_patch:
- self.__patch_file("Makefile",
+ self._patch_file("Makefile",
"# Add new monitors here",
"obj-$(CONFIG_RV_MON_%s) += monitors/%s/%s.o" % (name_up, name, name))
return " - Patching %s/Makefile, double check the result" % self.rv_dir
--
2.50.1On Tue, Jul 15, 2025 at 09:14:25AM +0200, Gabriele Monaco wrote:
> The current behaviour of rvgen when running with the -a option is to
> append the necessary lines at the end of the configuration for Kconfig,
> Makefile and tracepoints.
> This is not always the desired behaviour in case of nested monitors:
> while tracepoints are not affected by nesting and the Makefile's only
> requirement is that the parent monitor is built before its children, in
> the Kconfig it is better to have children defined right after their
> parent, otherwise the result has wrong indentation:
>
> [*] foo_parent monitor
> [*] foo_child1 monitor
> [*] foo_child2 monitor
> [*] bar_parent monitor
> [*] bar_child1 monitor
> [*] bar_child2 monitor
> [*] foo_child3 monitor
> [*] foo_child4 monitor
>
> Adapt rvgen to look for a different marker for nested monitors in the
> Kconfig file and append the line right after the last sibling, instead
> of the last monitor.
> Also add the marker when creating a new parent monitor.
>
> Signed-off-by: Gabriele Monaco <gmonaco@redhat.com>
> ---
> kernel/trace/rv/Kconfig | 5 +++++
> tools/verification/rvgen/rvgen/container.py | 13 +++++++++++++
> tools/verification/rvgen/rvgen/generator.py | 13 ++++++++-----
> 3 files changed, 26 insertions(+), 5 deletions(-)
>
> diff --git a/kernel/trace/rv/Kconfig b/kernel/trace/rv/Kconfig
> index c11bf7e61ebf0..26017378f79b8 100644
> --- a/kernel/trace/rv/Kconfig
> +++ b/kernel/trace/rv/Kconfig
> @@ -43,6 +43,7 @@ config RV_PER_TASK_MONITORS
>
> source "kernel/trace/rv/monitors/wip/Kconfig"
> source "kernel/trace/rv/monitors/wwnr/Kconfig"
> +
> source "kernel/trace/rv/monitors/sched/Kconfig"
> source "kernel/trace/rv/monitors/tss/Kconfig"
> source "kernel/trace/rv/monitors/sco/Kconfig"
> @@ -50,9 +51,13 @@ source "kernel/trace/rv/monitors/snroc/Kconfig"
> source "kernel/trace/rv/monitors/scpd/Kconfig"
> source "kernel/trace/rv/monitors/snep/Kconfig"
> source "kernel/trace/rv/monitors/sncid/Kconfig"
> +# Add new sched monitors here
> +
> source "kernel/trace/rv/monitors/rtapp/Kconfig"
> source "kernel/trace/rv/monitors/pagefault/Kconfig"
> source "kernel/trace/rv/monitors/sleep/Kconfig"
> +# Add new rtapp monitors here
> +
> # Add new monitors here
>
> config RV_REACTORS
> diff --git a/tools/verification/rvgen/rvgen/container.py b/tools/verification/rvgen/rvgen/container.py
> index 47d8ab2ad3ec4..fee493f89fde6 100644
> --- a/tools/verification/rvgen/rvgen/container.py
> +++ b/tools/verification/rvgen/rvgen/container.py
> @@ -20,3 +20,16 @@ class Container(generator.RVGenerator):
> main_h = self.main_h
> main_h = main_h.replace("%%MODEL_NAME%%", self.name)
> return main_h
> +
> + def fill_kconfig_tooltip(self):
> + """Override to produce a marker for this container in the Kconfig"""
> + container_marker = f"# Add new {self.name} monitors here\n"
> + if self.auto_patch:
> + self._patch_file("Kconfig",
> + "# Add new monitors here", "")
Isn't this one excessive? I gave it a try, but I had double blank line:
python3 tools/verification/rvgen -a container -n hello
61 source "kernel/trace/rv/monitors/sleep/Kconfig"
62 # Add new rtapp monitors here
63
+ 64
+ 65 source "kernel/trace/rv/monitors/hello/Kconfig"
+ 66 # Add new hello monitors here
+ 67
68 # Add new monitors here
69
70 config RV_REACTORS
> + result = super().fill_kconfig_tooltip()
> + if self.auto_patch:
> + self._patch_file("Kconfig",
> + "# Add new monitors here", container_marker)
> + return result
> + return result + container_marker
> diff --git a/tools/verification/rvgen/rvgen/generator.py b/tools/verification/rvgen/rvgen/generator.py
> index 19d0078a38032..ac97c4505ff00 100644
> --- a/tools/verification/rvgen/rvgen/generator.py
> +++ b/tools/verification/rvgen/rvgen/generator.py
> @@ -137,7 +137,8 @@ class RVGenerator:
> kconfig = kconfig.replace("%%MONITOR_DEPS%%", monitor_deps)
> return kconfig
>
> - def __patch_file(self, file, marker, line):
> + def _patch_file(self, file, marker, line):
> + assert(self.auto_patch)
> file_to_patch = os.path.join(self.rv_dir, file)
> content = self._read_file(file_to_patch)
> content = content.replace(marker, line + "\n" + marker)
> @@ -146,7 +147,7 @@ class RVGenerator:
> def fill_tracepoint_tooltip(self):
> monitor_class_type = self.fill_monitor_class_type()
> if self.auto_patch:
> - self.__patch_file("rv_trace.h",
> + self._patch_file("rv_trace.h",
> "// Add new monitors based on CONFIG_%s here" % monitor_class_type,
> "#include <monitors/%s/%s_trace.h>" % (self.name, self.name))
> return " - Patching %s/rv_trace.h, double check the result" % self.rv_dir
> @@ -158,8 +159,10 @@ Add this line where other tracepoints are included and %s is defined:
>
> def fill_kconfig_tooltip(self):
> if self.auto_patch:
> - self.__patch_file("Kconfig",
> - "# Add new monitors here",
> + # monitors with a container should stay together in the Kconfig
> + self._patch_file("Kconfig",
> + "# Add new %smonitors here" %
> + (self.parent + " " if self.parent else ""),
This one must be kept in sync with container_marker in
Container.fill_kconfig_tooltip(). I think this is hard to maintain later
on.
How about we keep in centralized with a helper function, something like:
def container_marker(container: str) -> str:
return f"# Add new {container} monitors here\n"
Nam
On Tue, 2025-07-15 at 16:48 +0200, Nam Cao wrote:
> On Tue, Jul 15, 2025 at 09:14:25AM +0200, Gabriele Monaco wrote:
> > The current behaviour of rvgen when running with the -a option is
> > to
> > append the necessary lines at the end of the configuration for
> > Kconfig,
> > Makefile and tracepoints.
> > This is not always the desired behaviour in case of nested
> > monitors:
> > while tracepoints are not affected by nesting and the Makefile's
> > only
> > requirement is that the parent monitor is built before its
> > children, in
> > the Kconfig it is better to have children defined right after their
> > parent, otherwise the result has wrong indentation:
> >
> > [*] foo_parent monitor
> > [*] foo_child1 monitor
> > [*] foo_child2 monitor
> > [*] bar_parent monitor
> > [*] bar_child1 monitor
> > [*] bar_child2 monitor
> > [*] foo_child3 monitor
> > [*] foo_child4 monitor
> >
> > Adapt rvgen to look for a different marker for nested monitors in
> > the
> > Kconfig file and append the line right after the last sibling,
> > instead
> > of the last monitor.
> > Also add the marker when creating a new parent monitor.
> >
> > Signed-off-by: Gabriele Monaco <gmonaco@redhat.com>
> > ---
> > kernel/trace/rv/Kconfig | 5 +++++
> > tools/verification/rvgen/rvgen/container.py | 13 +++++++++++++
> > tools/verification/rvgen/rvgen/generator.py | 13 ++++++++-----
> > 3 files changed, 26 insertions(+), 5 deletions(-)
> >
> > diff --git a/kernel/trace/rv/Kconfig b/kernel/trace/rv/Kconfig
> > index c11bf7e61ebf0..26017378f79b8 100644
> > --- a/kernel/trace/rv/Kconfig
> > +++ b/kernel/trace/rv/Kconfig
> > @@ -43,6 +43,7 @@ config RV_PER_TASK_MONITORS
> >
> > source "kernel/trace/rv/monitors/wip/Kconfig"
> > source "kernel/trace/rv/monitors/wwnr/Kconfig"
> > +
> > source "kernel/trace/rv/monitors/sched/Kconfig"
> > source "kernel/trace/rv/monitors/tss/Kconfig"
> > source "kernel/trace/rv/monitors/sco/Kconfig"
> > @@ -50,9 +51,13 @@ source "kernel/trace/rv/monitors/snroc/Kconfig"
> > source "kernel/trace/rv/monitors/scpd/Kconfig"
> > source "kernel/trace/rv/monitors/snep/Kconfig"
> > source "kernel/trace/rv/monitors/sncid/Kconfig"
> > +# Add new sched monitors here
> > +
> > source "kernel/trace/rv/monitors/rtapp/Kconfig"
> > source "kernel/trace/rv/monitors/pagefault/Kconfig"
> > source "kernel/trace/rv/monitors/sleep/Kconfig"
> > +# Add new rtapp monitors here
> > +
> > # Add new monitors here
> >
> > config RV_REACTORS
> > diff --git a/tools/verification/rvgen/rvgen/container.py
> > b/tools/verification/rvgen/rvgen/container.py
> > index 47d8ab2ad3ec4..fee493f89fde6 100644
> > --- a/tools/verification/rvgen/rvgen/container.py
> > +++ b/tools/verification/rvgen/rvgen/container.py
> > @@ -20,3 +20,16 @@ class Container(generator.RVGenerator):
> > main_h = self.main_h
> > main_h = main_h.replace("%%MODEL_NAME%%", self.name)
> > return main_h
> > +
> > + def fill_kconfig_tooltip(self):
> > + """Override to produce a marker for this container in the
> > Kconfig"""
> > + container_marker = f"# Add new {self.name} monitors
> > here\n"
> > + if self.auto_patch:
> > + self._patch_file("Kconfig",
> > + "# Add new monitors here", "")
>
> Isn't this one excessive? I gave it a try, but I had double blank
> line:
>
> python3 tools/verification/rvgen -a container -n hello
>
> 61 source "kernel/trace/rv/monitors/sleep/Kconfig"
> 62 # Add new rtapp monitors here
> 63
> + 64
> + 65 source "kernel/trace/rv/monitors/hello/Kconfig"
> + 66 # Add new hello monitors here
> + 67
> 68 # Add new monitors here
> 69
> 70 config RV_REACTORS
>
Yeah indeed it is, thanks for finding out, it somehow slipped my
tests..
> > + result = super().fill_kconfig_tooltip()
> > + if self.auto_patch:
> > + self._patch_file("Kconfig",
> > + "# Add new monitors here",
> > container_marker)
> > + return result
> > + return result + container_marker
> > diff --git a/tools/verification/rvgen/rvgen/generator.py
> > b/tools/verification/rvgen/rvgen/generator.py
> > index 19d0078a38032..ac97c4505ff00 100644
> > --- a/tools/verification/rvgen/rvgen/generator.py
> > +++ b/tools/verification/rvgen/rvgen/generator.py
> > @@ -137,7 +137,8 @@ class RVGenerator:
> > kconfig = kconfig.replace("%%MONITOR_DEPS%%",
> > monitor_deps)
> > return kconfig
> >
> > - def __patch_file(self, file, marker, line):
> > + def _patch_file(self, file, marker, line):
> > + assert(self.auto_patch)
> > file_to_patch = os.path.join(self.rv_dir, file)
> > content = self._read_file(file_to_patch)
> > content = content.replace(marker, line + "\n" + marker)
> > @@ -146,7 +147,7 @@ class RVGenerator:
> > def fill_tracepoint_tooltip(self):
> > monitor_class_type = self.fill_monitor_class_type()
> > if self.auto_patch:
> > - self.__patch_file("rv_trace.h",
> > + self._patch_file("rv_trace.h",
> > "// Add new monitors based on
> > CONFIG_%s here" % monitor_class_type,
> > "#include <monitors/%s/%s_trace.h>" %
> > (self.name, self.name))
> > return " - Patching %s/rv_trace.h, double check the
> > result" % self.rv_dir
> > @@ -158,8 +159,10 @@ Add this line where other tracepoints are
> > included and %s is defined:
> >
> > def fill_kconfig_tooltip(self):
> > if self.auto_patch:
> > - self.__patch_file("Kconfig",
> > - "# Add new monitors here",
> > + # monitors with a container should stay together in
> > the Kconfig
> > + self._patch_file("Kconfig",
> > + "# Add new %smonitors here" %
> > + (self.parent + " " if self.parent
> > else ""),
>
> This one must be kept in sync with container_marker in
> Container.fill_kconfig_tooltip(). I think this is hard to maintain
> later
> on.
>
> How about we keep in centralized with a helper function, something
> like:
>
> def container_marker(container: str) -> str:
> return f"# Add new {container} monitors here\n"
>
Good point, will do.
Thanks,
Gabriele
© 2016 - 2025 Red Hat, Inc.