[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20250715144858.ZS4_m8Qm@linutronix.de>
Date: Tue, 15 Jul 2025 16:48:58 +0200
From: Nam Cao <namcao@...utronix.de>
To: Gabriele Monaco <gmonaco@...hat.com>
Cc: linux-kernel@...r.kernel.org, Steven Rostedt <rostedt@...dmis.org>,
Masami Hiramatsu <mhiramat@...nel.org>,
linux-trace-kernel@...r.kernel.org, Ingo Molnar <mingo@...hat.com>,
Peter Zijlstra <peterz@...radead.org>,
Tomas Glozar <tglozar@...hat.com>, Juri Lelli <jlelli@...hat.com>,
Clark Williams <williams@...hat.com>,
John Kacur <jkacur@...hat.com>
Subject: Re: [PATCH v3 08/17] verification/rvgen: Organise Kconfig entries
for nested monitors
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@...hat.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
Powered by blists - more mailing lists