* Re: [RFC bpf-next 04/12] bpf: Add struct bpf_tramp_node object
From: Andrii Nakryiko @ 2026-02-05 22:27 UTC (permalink / raw)
To: Jiri Olsa
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, bpf,
linux-trace-kernel, Martin KaFai Lau, Eduard Zingerman, Song Liu,
Yonghong Song, Menglong Dong, Steven Rostedt
In-Reply-To: <aYRbYY8f_wWoiZhk@krava>
On Thu, Feb 5, 2026 at 12:57 AM Jiri Olsa <olsajiri@gmail.com> wrote:
>
> On Wed, Feb 04, 2026 at 11:00:57AM -0800, Andrii Nakryiko wrote:
> > On Tue, Feb 3, 2026 at 1:39 AM Jiri Olsa <jolsa@kernel.org> wrote:
> > >
> > > Adding struct bpf_tramp_node to decouple the link out of the trampoline
> > > attachment info.
> > >
> > > At the moment the object for attaching bpf program to the trampoline is
> > > 'struct bpf_tramp_link':
> > >
> > > struct bpf_tramp_link {
> > > struct bpf_link link;
> > > struct hlist_node tramp_hlist;
> > > u64 cookie;
> > > }
> > >
> > > The link holds the bpf_prog pointer and forces one link - one program
> > > binding logic. In following changes we want to attach program to multiple
> > > trampolines but have just one bpf_link object.
> > >
> > > Splitting struct bpf_tramp_link into:
> > >
> > > struct bpf_tramp_link {
> > > struct bpf_link link;
> > > struct bpf_tramp_node node;
> > > };
> > >
> > > struct bpf_tramp_node {
> > > struct hlist_node tramp_hlist;
> > > struct bpf_prog *prog;
> > > u64 cookie;
> > > };
> >
> > I'm a bit confused here. For singular fentry/fexit attachment we have
> > one trampoline and one program, right? For multi-fentry, we have
> > multiple trampoline, but still one program pointer, no? So why put a
> > prog pointer into tramp_node?.. You do want cookie in tramp_node, yes,
> > but not the program.
>
> yes, but both links:
> - single link 'struct bpf_tramp_link'
> - multi link 'struct bpf_tracing_multi_link'
>
> are using same code to attach that code needs to have a hlist_node to
> link the program to the trampoline and be able to reach the bpf_prog
> (like in invoke_bpf_prog)
>
> current code is passing whole bpf_tramp_link object so it has access
> to both, but multi link needs to keep link to each trampoline (nodes
> below):
>
> struct bpf_tracing_multi_link {
> struct bpf_link link;
> enum bpf_attach_type attach_type;
> int nodes_cnt;
> struct bpf_tracing_multi_node nodes[] __counted_by(nodes_cnt);
> };
>
> and we can't get get from &nodes[x] to bpf_tracing_multi_link.link.prog
>
> it's bit redundant, but not sure what else we can do
invoke_bpf_prog() specifically doesn't have to get prog pointer from
bpf_tramp_link, it can be passed prog as a separate argument and then
bpf_tramp_node with cookie separately as well. I haven't looked at
all other code, but I suspect we can refactor it to accept prog
explicitly and the relevant parts (node+cookie) separately.
Just at the conceptual level, we have single prog and multiple places
to patch (trampolines), so we shouldn't be co-locating in the same
data structure. It feels like a complete hack to duplicate prog just
to make some internal code access it.
>
> > Because then there is also a question what is
> > bpf_link's prog pointing to?...
>
> bpf_link.prog is still keeping the prog, I don't think we can remove that
>
> jirka
>
> >
> >
> > >
> > > where 'struct bpf_tramp_link' defines standard single trampoline link,
> > > and 'struct bpf_tramp_node' is the attachment trampoline object. This
> > > will allow us to define link for multiple trampolines, like:
> > >
> > > struct bpf_tracing_multi_link {
> > > struct bpf_link link;
> > > ...
> > > int nodes_cnt;
> > > struct bpf_tracing_multi_node nodes[] __counted_by(nodes_cnt);
> > > };
> > >
> > > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> > > ---
> > > arch/arm64/net/bpf_jit_comp.c | 58 +++++++++----------
> > > arch/s390/net/bpf_jit_comp.c | 42 +++++++-------
> > > arch/x86/net/bpf_jit_comp.c | 54 ++++++++---------
> > > include/linux/bpf.h | 47 ++++++++-------
> > > kernel/bpf/bpf_struct_ops.c | 24 ++++----
> > > kernel/bpf/syscall.c | 25 ++++----
> > > kernel/bpf/trampoline.c | 102 ++++++++++++++++-----------------
> > > net/bpf/bpf_dummy_struct_ops.c | 11 ++--
> > > 8 files changed, 185 insertions(+), 178 deletions(-)
> > >
> >
> > [...]
^ permalink raw reply
* Re: [PATCH v2 20/20] rv/rvgen: add missing return type annotations
From: Wander Lairson Costa @ 2026-02-05 20:12 UTC (permalink / raw)
To: Gabriele Monaco
Cc: Steven Rostedt, Nam Cao, open list:RUNTIME VERIFICATION (RV),
open list
In-Reply-To: <1edfc4451310645ffe89a60e461682eb49ff6006.camel@redhat.com>
On Thu, Feb 05, 2026 at 08:24:40AM +0100, Gabriele Monaco wrote:
> On Wed, 2026-02-04 at 11:42 -0300, Wander Lairson Costa wrote:
> > Add missing `-> str` return type annotations to code generation
> > methods in RVGenerator. These methods return strings, and the missing
> > hints caused errors with static analysis tools.
> >
> > Signed-off-by: Wander Lairson Costa <wander@redhat.com>
> > ---
> > tools/verification/rvgen/rvgen/generator.py | 16 ++++++++--------
> > 1 file changed, 8 insertions(+), 8 deletions(-)
> >
> > diff --git a/tools/verification/rvgen/rvgen/generator.py
> > b/tools/verification/rvgen/rvgen/generator.py
> > index ef6c9150f50c6..4b984869a7b3d 100644
> > --- a/tools/verification/rvgen/rvgen/generator.py
> > +++ b/tools/verification/rvgen/rvgen/generator.py
> > @@ -73,13 +73,13 @@ class RVGenerator:
> > return f"#include <monitors/{self.parent}/{self.parent}.h>\n"
> > return ""
> >
> > - def fill_tracepoint_handlers_skel(self):
> > + def fill_tracepoint_handlers_skel(self) -> str:
> > return "NotImplemented"
>
> Those are the ones that will raise an exception in a later iteration of this,
> right? I wonder if we really need to touch them now.
> Anyway I'm fine with this if it pleases your tools, thanks.
I was uncertain if I should send now or postpone it, to be fair.
>
> Reviewed-by: Gabriele Monaco <gmonaco@redhat.com>
>
> >
> > - def fill_tracepoint_attach_probe(self):
> > + def fill_tracepoint_attach_probe(self) -> str:
> > return "NotImplemented"
> >
> > - def fill_tracepoint_detach_helper(self):
> > + def fill_tracepoint_detach_helper(self) -> str:
> > return "NotImplemented"
> >
> > def fill_main_c(self):
> > @@ -100,19 +100,19 @@ class RVGenerator:
> >
> > return main_c
> >
> > - def fill_model_h(self):
> > + def fill_model_h(self) -> str:
> > return "NotImplemented"
> >
> > - def fill_monitor_class_type(self):
> > + def fill_monitor_class_type(self) -> str:
> > return "NotImplemented"
> >
> > - def fill_monitor_class(self):
> > + def fill_monitor_class(self) -> str:
> > return "NotImplemented"
> >
> > - def fill_tracepoint_args_skel(self, tp_type):
> > + def fill_tracepoint_args_skel(self, tp_type) -> str:
> > return "NotImplemented"
> >
> > - def fill_monitor_deps(self):
> > + def fill_monitor_deps(self) -> str:
> > buff = []
> > buff.append(" # XXX: add dependencies if there")
> > if self.parent:
>
^ permalink raw reply
* Re: [PATCH v2 07/20] rv/rvgen: fix typos in automata and generator docstring and comments
From: Wander Lairson Costa @ 2026-02-05 20:04 UTC (permalink / raw)
To: Gabriele Monaco
Cc: Steven Rostedt, Nam Cao, open list:RUNTIME VERIFICATION (RV),
open list
In-Reply-To: <90102c96156337943a5319ab25991f95f9f07cf5.camel@redhat.com>
On Thu, Feb 05, 2026 at 08:03:16AM +0100, Gabriele Monaco wrote:
> On Wed, 2026-02-04 at 11:42 -0300, Wander Lairson Costa wrote:
> > Fix two typos in the Automata class documentation that have been
> > present since the initial implementation. Fix the class
> > docstring: "part it" instead of "parses it". Additionally, a
> > comment describing transition labels contained the misspelling
> > "lables" instead of "labels".
> >
> > Fix a typo in the comment describing the insertion of the initial
> > state into the states list: "bein og" should be "beginning of".
> >
> > Fix typo in the module docstring: "Abtract" should be "Abstract".
> >
> > Signed-off-by: Wander Lairson Costa <wander@redhat.com>
>
>
> While you're at it there are a few singular/plural inconsistencies, see below.
Okie dokie. I will update the patch
>
> Other than that it looks good, thanks!
>
> Reviewed-by: Gabriele Monaco <gmonaco@redhat.com>
>
>
> > ---
> > tools/verification/rvgen/rvgen/automata.py | 6 +++---
> > tools/verification/rvgen/rvgen/generator.py | 2 +-
> > 2 files changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/tools/verification/rvgen/rvgen/automata.py
> > b/tools/verification/rvgen/rvgen/automata.py
> > index 1feb5f0c0bc3e..0d7cbd0c634a9 100644
> > --- a/tools/verification/rvgen/rvgen/automata.py
> > +++ b/tools/verification/rvgen/rvgen/automata.py
> > @@ -18,7 +18,7 @@ class AutomataError(Exception):
> > """
> >
> > class Automata:
> > - """Automata class: Reads a dot file and part it as an automata.
> > + """Automata class: Reads a dot file and parses it as an automata.
> >
>
> Automata is plural, the singular is automaton (you can keep the class name
> unchanged):
>
> + """Automata class: Reads a dot file and parses it as an automaton.
>
> > Attributes:
> > dot_file: A dot file with an state_automaton definition.
> > @@ -113,7 +113,7 @@ class Automata:
> > states = sorted(set(states))
> > states.remove(initial_state)
> >
> > - # Insert the initial state at the bein og the states
> > + # Insert the initial state at the beginning of the states
> > states.insert(0, initial_state)
> >
> > if not has_final_states:
> > @@ -134,7 +134,7 @@ class Automata:
> > line = self.__dot_lines[cursor].split()
> > event = line[-2].replace('"','')
> >
> > - # when a transition has more than one lables, they are like
> > this
> > + # when a transition has more than one labels, they are like
> > this
>
> This should be "more than one label" as singular.
>
> > # "local_irq_enable\nhw_local_irq_enable_n"
> > # so split them.
> >
> > diff --git a/tools/verification/rvgen/rvgen/generator.py
> > b/tools/verification/rvgen/rvgen/generator.py
> > index ee75e111feef1..a3fbb1ac74916 100644
> > --- a/tools/verification/rvgen/rvgen/generator.py
> > +++ b/tools/verification/rvgen/rvgen/generator.py
> > @@ -3,7 +3,7 @@
> > #
> > # Copyright (C) 2019-2022 Red Hat, Inc. Daniel Bristot de Oliveira
> > <bristot@kernel.org>
> > #
> > -# Abtract class for generating kernel runtime verification monitors from
> > specification file
> > +# Abstract class for generating kernel runtime verification monitors from
> > specification file
> >
> > import platform
> > import os
>
^ permalink raw reply
* Re: [PATCH v2 19/20] rv/rvgen: fix _fill_states() return type annotation
From: Wander Lairson Costa @ 2026-02-05 20:03 UTC (permalink / raw)
To: Gabriele Monaco
Cc: Steven Rostedt, Nam Cao, open list:RUNTIME VERIFICATION (RV),
open list
In-Reply-To: <7b35a21f1b85e6510b3653ab520920e459160679.camel@redhat.com>
On Thu, Feb 05, 2026 at 08:24:11AM +0100, Gabriele Monaco wrote:
> On Wed, 2026-02-04 at 11:42 -0300, Wander Lairson Costa wrote:
> > The _fill_states() method returns a list of strings, but the type
> > annotation incorrectly specified str. Update the annotation to
> > list[str] to match the actual return value.
> >
> > Signed-off-by: Wander Lairson Costa <wander@redhat.com>
> > ---
>
> Looks good, thanks. I would keep all annotation changes together (i.e. squash
> with the next patch), but if you prefer this way, I'm fine too.
>
I don't have strong feelings either way. Feel free to squash them.
> Reviewed-by: Gabriele Monaco <gmonaco@redhat.com>
>
> > tools/verification/rvgen/rvgen/ltl2k.py | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/tools/verification/rvgen/rvgen/ltl2k.py
> > b/tools/verification/rvgen/rvgen/ltl2k.py
> > index 2c564cc937235..de765b8486bd1 100644
> > --- a/tools/verification/rvgen/rvgen/ltl2k.py
> > +++ b/tools/verification/rvgen/rvgen/ltl2k.py
> > @@ -71,7 +71,7 @@ class ltl2k(generator.Monitor):
> > if not self.name:
> > self.name = Path(file_path).stem
> >
> > - def _fill_states(self) -> str:
> > + def _fill_states(self) -> list[str]:
> > buf = [
> > "enum ltl_buchi_state {",
> > ]
>
^ permalink raw reply
* Re: [PATCH v2 03/20] rv/rvgen: replace % string formatting with f-strings
From: Wander Lairson Costa @ 2026-02-05 19:54 UTC (permalink / raw)
To: Gabriele Monaco
Cc: Steven Rostedt, Nam Cao, open list:RUNTIME VERIFICATION (RV),
open list
In-Reply-To: <e02aeb0856a2d4dbcebea519c3ef1a07ace7a703.camel@redhat.com>
On Thu, Feb 05, 2026 at 12:37:13PM +0100, Gabriele Monaco wrote:
> On Wed, 2026-02-04 at 11:42 -0300, Wander Lairson Costa wrote:
> > Replace all instances of percent-style string formatting with
> > f-strings across the rvgen codebase. This modernizes the string
> > formatting to use Python 3.6+ features, providing clearer and more
> > maintainable code while improving runtime performance.
> >
> > The conversion handles all formatting cases including simple variable
> > substitution, multi-variable formatting, and complex format specifiers.
> > Dynamic width formatting is converted from "%*s" to "{var:>{width}}"
> > using proper alignment syntax. Template strings for generated C code
> > properly escape braces using double-brace syntax to produce literal
> > braces in the output.
> >
> > F-strings provide approximately 2x performance improvement over percent
> > formatting and are the recommended approach in modern Python.
> >
> > Signed-off-by: Wander Lairson Costa <wander@redhat.com>
> > Reviewed-by: Nam Cao <namcao@linutronix.de>
> > Reviewed-by: Gabriele Monaco <gmonaco@redhat.com>
> > ---
>
> There's a new one after the rebase, you may want to add this hunk:
>
> diff --git a/tools/verification/rvgen/rvgen/dot2k.py
> b/tools/verification/rvgen/rvgen/dot2k.py
> index 4bf0db52e626..0d2cdfdf25eb 100644
> --- a/tools/verification/rvgen/rvgen/dot2k.py
> +++ b/tools/verification/rvgen/rvgen/dot2k.py
> @@ -66,7 +66,7 @@ class dot2k(Monitor, Dot2c):
> buff.append(" * Documentation/trace/rv/deterministic_automata.rst")
> buff.append(" */")
> buff.append("")
> - buff.append("#define MONITOR_NAME %s" % (self.name))
> + buff.append(f"#define MONITOR_NAME {self.name}")
> buff.append("")
>
> return buff
Ah, thanks! I will update the patch.
>
> ---
>
> Thanks,
> Gabriele
>
> > tools/verification/rvgen/__main__.py | 6 +--
> > tools/verification/rvgen/rvgen/automata.py | 6 +--
> > tools/verification/rvgen/rvgen/dot2c.py | 38 ++++++-------
> > tools/verification/rvgen/rvgen/dot2k.py | 26 ++++-----
> > tools/verification/rvgen/rvgen/generator.py | 59 ++++++++++-----------
> > tools/verification/rvgen/rvgen/ltl2k.py | 30 +++++------
> > 6 files changed, 81 insertions(+), 84 deletions(-)
> >
> > diff --git a/tools/verification/rvgen/__main__.py
> > b/tools/verification/rvgen/__main__.py
> > index 3bd438f8476ed..50b7d4227fb16 100644
> > --- a/tools/verification/rvgen/__main__.py
> > +++ b/tools/verification/rvgen/__main__.py
> > @@ -45,7 +45,7 @@ if __name__ == '__main__':
> >
> > try:
> > if params.subcmd == "monitor":
> > - print("Opening and parsing the specification file %s" %
> > params.spec)
> > + print(f"Opening and parsing the specification file
> > {params.spec}")
> > if params.monitor_class == "da":
> > monitor = dot2k(params.spec, params.monitor_type,
> > vars(params))
> > elif params.monitor_class == "ltl":
> > @@ -59,11 +59,11 @@ if __name__ == '__main__':
> > print(f"There was an error processing {params.spec}: {e}",
> > file=sys.stderr)
> > sys.exit(1)
> >
> > - print("Writing the monitor into the directory %s" % monitor.name)
> > + print(f"Writing the monitor into the directory {monitor.name}")
> > monitor.print_files()
> > print("Almost done, checklist")
> > if params.subcmd == "monitor":
> > - print(" - Edit the %s/%s.c to add the instrumentation" %
> > (monitor.name, monitor.name))
> > + print(f" - Edit the {monitor.name}/{monitor.name}.c to add the
> > instrumentation")
> > print(monitor.fill_tracepoint_tooltip())
> > print(monitor.fill_makefile_tooltip())
> > print(monitor.fill_kconfig_tooltip())
> > diff --git a/tools/verification/rvgen/rvgen/automata.py
> > b/tools/verification/rvgen/rvgen/automata.py
> > index 6ecd5ccd8f3d3..bd8c04526be3a 100644
> > --- a/tools/verification/rvgen/rvgen/automata.py
> > +++ b/tools/verification/rvgen/rvgen/automata.py
> > @@ -39,11 +39,11 @@ class Automata:
> > basename = ntpath.basename(self.__dot_path)
> > if not basename.endswith(".dot") and not basename.endswith(".gv"):
> > print("not a dot file")
> > - raise AutomataError("not a dot file: %s" % self.__dot_path)
> > + raise AutomataError(f"not a dot file: {self.__dot_path}")
> >
> > model_name = ntpath.splitext(basename)[0]
> > if model_name.__len__() == 0:
> > - raise AutomataError("not a dot file: %s" % self.__dot_path)
> > + raise AutomataError(f"not a dot file: {self.__dot_path}")
> >
> > return model_name
> >
> > @@ -62,7 +62,7 @@ class Automata:
> > line = dot_lines[cursor].split()
> >
> > if (line[0] != "digraph") and (line[1] != "state_automaton"):
> > - raise AutomataError("Not a valid .dot format: %s" %
> > self.__dot_path)
> > + raise AutomataError(f"Not a valid .dot format:
> > {self.__dot_path}")
> > else:
> > cursor += 1
> > return dot_lines
> > diff --git a/tools/verification/rvgen/rvgen/dot2c.py
> > b/tools/verification/rvgen/rvgen/dot2c.py
> > index 74147ae2942f9..6a2ad4fbf7824 100644
> > --- a/tools/verification/rvgen/rvgen/dot2c.py
> > +++ b/tools/verification/rvgen/rvgen/dot2c.py
> > @@ -28,17 +28,17 @@ class Dot2c(Automata):
> >
> > def __get_enum_states_content(self) -> list[str]:
> > buff = []
> > - buff.append("\t%s%s," % (self.initial_state, self.enum_suffix))
> > + buff.append(f"\t{self.initial_state}{self.enum_suffix},")
> > for state in self.states:
> > if state != self.initial_state:
> > - buff.append("\t%s%s," % (state, self.enum_suffix))
> > - buff.append("\tstate_max%s," % (self.enum_suffix))
> > + buff.append(f"\t{state}{self.enum_suffix},")
> > + buff.append(f"\tstate_max{self.enum_suffix},")
> >
> > return buff
> >
> > def format_states_enum(self) -> list[str]:
> > buff = []
> > - buff.append("enum %s {" % self.enum_states_def)
> > + buff.append(f"enum {self.enum_states_def} {{")
> > buff += self.__get_enum_states_content()
> > buff.append("};\n")
> >
> > @@ -47,15 +47,15 @@ class Dot2c(Automata):
> > def __get_enum_events_content(self) -> list[str]:
> > buff = []
> > for event in self.events:
> > - buff.append("\t%s%s," % (event, self.enum_suffix))
> > + buff.append(f"\t{event}{self.enum_suffix},")
> >
> > - buff.append("\tevent_max%s," % self.enum_suffix)
> > + buff.append(f"\tevent_max{self.enum_suffix},")
> >
> > return buff
> >
> > def format_events_enum(self) -> list[str]:
> > buff = []
> > - buff.append("enum %s {" % self.enum_events_def)
> > + buff.append(f"enum {self.enum_events_def} {{")
> > buff += self.__get_enum_events_content()
> > buff.append("};\n")
> >
> > @@ -71,25 +71,25 @@ class Dot2c(Automata):
> > min_type = "unsigned int"
> >
> > if self.states.__len__() > 1000000:
> > - raise AutomataError("Too many states: %d" %
> > self.states.__len__())
> > + raise AutomataError(f"Too many states: {self.states.__len__()}")
> >
> > return min_type
> >
> > def format_automaton_definition(self) -> list[str]:
> > min_type = self.get_minimun_type()
> > buff = []
> > - buff.append("struct %s {" % self.struct_automaton_def)
> > - buff.append("\tchar *state_names[state_max%s];" % (self.enum_suffix))
> > - buff.append("\tchar *event_names[event_max%s];" % (self.enum_suffix))
> > - buff.append("\t%s function[state_max%s][event_max%s];" % (min_type,
> > self.enum_suffix, self.enum_suffix))
> > - buff.append("\t%s initial_state;" % min_type)
> > - buff.append("\tbool final_states[state_max%s];" % (self.enum_suffix))
> > + buff.append(f"struct {self.struct_automaton_def} {{")
> > + buff.append(f"\tchar *state_names[state_max{self.enum_suffix}];")
> > + buff.append(f"\tchar *event_names[event_max{self.enum_suffix}];")
> > + buff.append(f"\t{min_type}
> > function[state_max{self.enum_suffix}][event_max{self.enum_suffix}];")
> > + buff.append(f"\t{min_type} initial_state;")
> > + buff.append(f"\tbool final_states[state_max{self.enum_suffix}];")
> > buff.append("};\n")
> > return buff
> >
> > def format_aut_init_header(self) -> list[str]:
> > buff = []
> > - buff.append("static const struct %s %s = {" %
> > (self.struct_automaton_def, self.var_automaton_def))
> > + buff.append(f"static const struct {self.struct_automaton_def}
> > {self.var_automaton_def} = {{")
> > return buff
> >
> > def __get_string_vector_per_line_content(self, entries: list[str]) ->
> > str:
> > @@ -134,9 +134,9 @@ class Dot2c(Automata):
> > next_state = self.function[x][y] + self.enum_suffix
> >
> > if linetoolong:
> > - line += "\t\t\t%s" % next_state
> > + line += f"\t\t\t{next_state}"
> > else:
> > - line += "%*s" % (maxlen, next_state)
> > + line += f"{next_state:>{maxlen}}"
> > if y != nr_events-1:
> > line += ",\n" if linetoolong else ", "
> > else:
> > @@ -180,7 +180,7 @@ class Dot2c(Automata):
> >
> > def format_aut_init_final_states(self) -> list[str]:
> > buff = []
> > - buff.append("\t.final_states = { %s }," %
> > self.get_aut_init_final_states())
> > + buff.append(f"\t.final_states = {{ {self.get_aut_init_final_states()}
> > }},")
> >
> > return buff
> >
> > @@ -196,7 +196,7 @@ class Dot2c(Automata):
> >
> > def format_invalid_state(self) -> list[str]:
> > buff = []
> > - buff.append("#define %s state_max%s\n" % (self.invalid_state_str,
> > self.enum_suffix))
> > + buff.append(f"#define {self.invalid_state_str}
> > state_max{self.enum_suffix}\n")
> >
> > return buff
> >
> > diff --git a/tools/verification/rvgen/rvgen/dot2k.py
> > b/tools/verification/rvgen/rvgen/dot2k.py
> > index 6128fe2384308..6d4151acc9fe3 100644
> > --- a/tools/verification/rvgen/rvgen/dot2k.py
> > +++ b/tools/verification/rvgen/rvgen/dot2k.py
> > @@ -19,7 +19,7 @@ class dot2k(Monitor, Dot2c):
> > self.monitor_type = MonitorType
> > Monitor.__init__(self, extra_params)
> > Dot2c.__init__(self, file_path, extra_params.get("model_name"))
> > - self.enum_suffix = "_%s" % self.name
> > + self.enum_suffix = f"_{self.name}"
> >
> > def fill_monitor_type(self) -> str:
> > return self.monitor_type.upper()
> > @@ -27,7 +27,7 @@ class dot2k(Monitor, Dot2c):
> > def fill_tracepoint_handlers_skel(self) -> str:
> > buff = []
> > for event in self.events:
> > - buff.append("static void handle_%s(void *data, /* XXX: fill
> > header */)" % event)
> > + buff.append(f"static void handle_{event}(void *data, /* XXX: fill
> > header */)")
> > buff.append("{")
> > handle = "handle_event"
> > if self.is_start_event(event):
> > @@ -38,9 +38,9 @@ class dot2k(Monitor, Dot2c):
> > handle = "handle_start_run_event"
> > if self.monitor_type == "per_task":
> > buff.append("\tstruct task_struct *p = /* XXX: how do I get
> > p? */;");
> > - buff.append("\tda_%s(p, %s%s);" % (handle, event,
> > self.enum_suffix));
> > + buff.append(f"\tda_{handle}(p, {event}{self.enum_suffix});");
> > else:
> > - buff.append("\tda_%s(%s%s);" % (handle, event,
> > self.enum_suffix));
> > + buff.append(f"\tda_{handle}({event}{self.enum_suffix});");
> > buff.append("}")
> > buff.append("")
> > return '\n'.join(buff)
> > @@ -48,20 +48,20 @@ class dot2k(Monitor, Dot2c):
> > def fill_tracepoint_attach_probe(self) -> str:
> > buff = []
> > for event in self.events:
> > - buff.append("\trv_attach_trace_probe(\"%s\", /* XXX: tracepoint
> > */, handle_%s);" % (self.name, event))
> > + buff.append(f"\trv_attach_trace_probe(\"{self.name}\", /* XXX:
> > tracepoint */, handle_{event});")
> > return '\n'.join(buff)
> >
> > def fill_tracepoint_detach_helper(self) -> str:
> > buff = []
> > for event in self.events:
> > - buff.append("\trv_detach_trace_probe(\"%s\", /* XXX: tracepoint
> > */, handle_%s);" % (self.name, event))
> > + buff.append(f"\trv_detach_trace_probe(\"{self.name}\", /* XXX:
> > tracepoint */, handle_{event});")
> > return '\n'.join(buff)
> >
> > def fill_model_h_header(self) -> list[str]:
> > buff = []
> > buff.append("/* SPDX-License-Identifier: GPL-2.0 */")
> > buff.append("/*")
> > - buff.append(" * Automatically generated C representation of %s
> > automaton" % (self.name))
> > + buff.append(f" * Automatically generated C representation of
> > {self.name} automaton")
> > buff.append(" * For further information about this format, see kernel
> > documentation:")
> > buff.append(" * Documentation/trace/rv/deterministic_automata.rst")
> > buff.append(" */")
> > @@ -75,10 +75,10 @@ class dot2k(Monitor, Dot2c):
> > #
> > # Adjust the definition names
> > #
> > - self.enum_states_def = "states_%s" % self.name
> > - self.enum_events_def = "events_%s" % self.name
> > - self.struct_automaton_def = "automaton_%s" % self.name
> > - self.var_automaton_def = "automaton_%s" % self.name
> > + self.enum_states_def = f"states_{self.name}"
> > + self.enum_events_def = f"events_{self.name}"
> > + self.struct_automaton_def = f"automaton_{self.name}"
> > + self.var_automaton_def = f"automaton_{self.name}"
> >
> > buff = self.fill_model_h_header()
> > buff += self.format_model()
> > @@ -113,8 +113,8 @@ class dot2k(Monitor, Dot2c):
> > tp_args.insert(0, tp_args_id)
> > tp_proto_c = ", ".join([a+b for a,b in tp_args])
> > tp_args_c = ", ".join([b for a,b in tp_args])
> > - buff.append(" TP_PROTO(%s)," % tp_proto_c)
> > - buff.append(" TP_ARGS(%s)" % tp_args_c)
> > + buff.append(f" TP_PROTO({tp_proto_c}),")
> > + buff.append(f" TP_ARGS({tp_args_c})")
> > return '\n'.join(buff)
> >
> > def fill_main_c(self) -> str:
> > diff --git a/tools/verification/rvgen/rvgen/generator.py
> > b/tools/verification/rvgen/rvgen/generator.py
> > index af1662e2c20a7..6d16fb68798a7 100644
> > --- a/tools/verification/rvgen/rvgen/generator.py
> > +++ b/tools/verification/rvgen/rvgen/generator.py
> > @@ -40,7 +40,7 @@ class RVGenerator:
> > if platform.system() != "Linux":
> > raise OSError("I can only run on Linux.")
> >
> > - kernel_path = os.path.join("/lib/modules/%s/build" %
> > platform.release(), self.rv_dir)
> > + kernel_path =
> > os.path.join(f"/lib/modules/{platform.release()}/build", self.rv_dir)
> >
> > # if the current kernel is from a distro this may not be a full
> > kernel tree
> > # verify that one of the files we are going to modify is available
> > @@ -69,11 +69,11 @@ class RVGenerator:
> > return self._read_file(path)
> >
> > def fill_parent(self):
> > - return "&rv_%s" % self.parent if self.parent else "NULL"
> > + return f"&rv_{self.parent}" if self.parent else "NULL"
> >
> > def fill_include_parent(self):
> > if self.parent:
> > - return "#include <monitors/%s/%s.h>\n" % (self.parent,
> > self.parent)
> > + return f"#include <monitors/{self.parent}/{self.parent}.h>\n"
> > return ""
> >
> > def fill_tracepoint_handlers_skel(self):
> > @@ -119,7 +119,7 @@ class RVGenerator:
> > buff = []
> > buff.append(" # XXX: add dependencies if there")
> > if self.parent:
> > - buff.append(" depends on RV_MON_%s" % self.parent.upper())
> > + buff.append(f" depends on RV_MON_{self.parent.upper()}")
> > buff.append(" default y")
> > return '\n'.join(buff)
> >
> > @@ -145,31 +145,30 @@ class RVGenerator:
> > monitor_class_type = self.fill_monitor_class_type()
> > if self.auto_patch:
> > 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
> > + f"// Add new monitors based on
> > CONFIG_{monitor_class_type} here",
> > + f"#include
> > <monitors/{self.name}/{self.name}_trace.h>")
> > + return f" - Patching {self.rv_dir}/rv_trace.h, double check the
> > result"
> >
> > - return """ - Edit %s/rv_trace.h:
> > -Add this line where other tracepoints are included and %s is defined:
> > -#include <monitors/%s/%s_trace.h>
> > -""" % (self.rv_dir, monitor_class_type, self.name, self.name)
> > + return f""" - Edit {self.rv_dir}/rv_trace.h:
> > +Add this line where other tracepoints are included and {monitor_class_type}
> > is defined:
> > +#include <monitors/{self.name}/{self.name}_trace.h>
> > +"""
> >
> > def _kconfig_marker(self, container=None) -> str:
> > - return "# Add new %smonitors here" % (container + " "
> > - if container else "")
> > + return f"# Add new {container + ' ' if container else ''}monitors
> > here"
> >
> > def fill_kconfig_tooltip(self):
> > if self.auto_patch:
> > # monitors with a container should stay together in the Kconfig
> > self._patch_file("Kconfig",
> > self._kconfig_marker(self.parent),
> > - "source \"kernel/trace/rv/monitors/%s/Kconfig\""
> > % (self.name))
> > - return " - Patching %s/Kconfig, double check the result" %
> > self.rv_dir
> > + f"source
> > \"kernel/trace/rv/monitors/{self.name}/Kconfig\"")
> > + return f" - Patching {self.rv_dir}/Kconfig, double check the
> > result"
> >
> > - return """ - Edit %s/Kconfig:
> > + return f""" - Edit {self.rv_dir}/Kconfig:
> > Add this line where other monitors are included:
> > -source \"kernel/trace/rv/monitors/%s/Kconfig\"
> > -""" % (self.rv_dir, self.name)
> > +source \"kernel/trace/rv/monitors/{self.name}/Kconfig\"
> > +"""
> >
> > def fill_makefile_tooltip(self):
> > name = self.name
> > @@ -177,18 +176,18 @@ source \"kernel/trace/rv/monitors/%s/Kconfig\"
> > if self.auto_patch:
> > 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
> > + f"obj-$(CONFIG_RV_MON_{name_up}) +=
> > monitors/{name}/{name}.o")
> > + return f" - Patching {self.rv_dir}/Makefile, double check the
> > result"
> >
> > - return """ - Edit %s/Makefile:
> > + return f""" - Edit {self.rv_dir}/Makefile:
> > Add this line where other monitors are included:
> > -obj-$(CONFIG_RV_MON_%s) += monitors/%s/%s.o
> > -""" % (self.rv_dir, name_up, name, name)
> > +obj-$(CONFIG_RV_MON_{name_up}) += monitors/{name}/{name}.o
> > +"""
> >
> > def fill_monitor_tooltip(self):
> > if self.auto_patch:
> > - return " - Monitor created in %s/monitors/%s" % (self.rv_dir,
> > self. name)
> > - return " - Move %s/ to the kernel's monitor directory (%s/monitors)"
> > % (self.name, self.rv_dir)
> > + return f" - Monitor created in
> > {self.rv_dir}/monitors/{self.name}"
> > + return f" - Move {self.name}/ to the kernel's monitor directory
> > ({self.rv_dir}/monitors)"
> >
> > def __create_directory(self):
> > path = self.name
> > @@ -205,13 +204,13 @@ obj-$(CONFIG_RV_MON_%s) += monitors/%s/%s.o
> > file.close()
> >
> > def _create_file(self, file_name, content):
> > - path = "%s/%s" % (self.name, file_name)
> > + path = f"{self.name}/{file_name}"
> > if self.auto_patch:
> > path = os.path.join(self.rv_dir, "monitors", path)
> > self.__write_file(path, content)
> >
> > def __get_main_name(self):
> > - path = "%s/%s" % (self.name, "main.c")
> > + path = f"{self.name}/main.c"
> > if not os.path.exists(path):
> > return "main.c"
> > return "__main.c"
> > @@ -221,11 +220,11 @@ obj-$(CONFIG_RV_MON_%s) += monitors/%s/%s.o
> >
> > self.__create_directory()
> >
> > - path = "%s.c" % self.name
> > + path = f"{self.name}.c"
> > self._create_file(path, main_c)
> >
> > model_h = self.fill_model_h()
> > - path = "%s.h" % self.name
> > + path = f"{self.name}.h"
> > self._create_file(path, model_h)
> >
> > kconfig = self.fill_kconfig()
> > @@ -256,5 +255,5 @@ class Monitor(RVGenerator):
> > def print_files(self):
> > super().print_files()
> > trace_h = self.fill_trace_h()
> > - path = "%s_trace.h" % self.name
> > + path = f"{self.name}_trace.h"
> > self._create_file(path, trace_h)
> > diff --git a/tools/verification/rvgen/rvgen/ltl2k.py
> > b/tools/verification/rvgen/rvgen/ltl2k.py
> > index b075f98d50c47..fa9ea6d597095 100644
> > --- a/tools/verification/rvgen/rvgen/ltl2k.py
> > +++ b/tools/verification/rvgen/rvgen/ltl2k.py
> > @@ -73,7 +73,7 @@ class ltl2k(generator.Monitor):
> > ]
> >
> > for node in self.ba:
> > - buf.append("\tS%i," % node.id)
> > + buf.append(f"\tS{node.id},")
> > buf.append("\tRV_NUM_BA_STATES")
> > buf.append("};")
> > buf.append("static_assert(RV_NUM_BA_STATES <= RV_MAX_BA_STATES);")
> > @@ -82,7 +82,7 @@ class ltl2k(generator.Monitor):
> > def _fill_atoms(self):
> > buf = ["enum ltl_atom {"]
> > for a in sorted(self.atoms):
> > - buf.append("\tLTL_%s," % a)
> > + buf.append(f"\tLTL_{a},")
> > buf.append("\tLTL_NUM_ATOM")
> > buf.append("};")
> > buf.append("static_assert(LTL_NUM_ATOM <= RV_MAX_LTL_ATOM);")
> > @@ -96,7 +96,7 @@ class ltl2k(generator.Monitor):
> > ]
> >
> > for name in self.atoms_abbr:
> > - buf.append("\t\t\"%s\"," % name)
> > + buf.append(f"\t\t\"{name}\",")
> >
> > buf.extend([
> > "\t};",
> > @@ -113,19 +113,19 @@ class ltl2k(generator.Monitor):
> > continue
> >
> > if isinstance(node.op, ltl2ba.AndOp):
> > - buf.append("\tbool %s = %s && %s;" % (node, node.op.left,
> > node.op.right))
> > + buf.append(f"\tbool {node} = {node.op.left} &&
> > {node.op.right};")
> > required_values |= {str(node.op.left), str(node.op.right)}
> > elif isinstance(node.op, ltl2ba.OrOp):
> > - buf.append("\tbool %s = %s || %s;" % (node, node.op.left,
> > node.op.right))
> > + buf.append(f"\tbool {node} = {node.op.left} ||
> > {node.op.right};")
> > required_values |= {str(node.op.left), str(node.op.right)}
> > elif isinstance(node.op, ltl2ba.NotOp):
> > - buf.append("\tbool %s = !%s;" % (node, node.op.child))
> > + buf.append(f"\tbool {node} = !{node.op.child};")
> > required_values.add(str(node.op.child))
> >
> > for atom in self.atoms:
> > if atom.lower() not in required_values:
> > continue
> > - buf.append("\tbool %s = test_bit(LTL_%s, mon->atoms);" %
> > (atom.lower(), atom))
> > + buf.append(f"\tbool {atom.lower()} = test_bit(LTL_{atom}, mon-
> > >atoms);")
> >
> > buf.reverse()
> >
> > @@ -153,7 +153,7 @@ class ltl2k(generator.Monitor):
> > ])
> >
> > for node in self.ba:
> > - buf.append("\tcase S%i:" % node.id)
> > + buf.append(f"\tcase S{node.id}:")
> >
> > for o in sorted(node.outgoing):
> > line = "\t\tif "
> > @@ -163,7 +163,7 @@ class ltl2k(generator.Monitor):
> > lines = break_long_line(line, indent)
> > buf.extend(lines)
> >
> > - buf.append("\t\t\t__set_bit(S%i, next);" % o.id)
> > + buf.append(f"\t\t\t__set_bit(S{o.id}, next);")
> > buf.append("\t\tbreak;")
> > buf.extend([
> > "\t}",
> > @@ -197,7 +197,7 @@ class ltl2k(generator.Monitor):
> > lines = break_long_line(line, indent)
> > buf.extend(lines)
> >
> > - buf.append("\t\t__set_bit(S%i, mon->states);" % node.id)
> > + buf.append(f"\t\t__set_bit(S{node.id}, mon->states);")
> > buf.append("}")
> > return buf
> >
> > @@ -205,23 +205,21 @@ class ltl2k(generator.Monitor):
> > buff = []
> > buff.append("static void handle_example_event(void *data, /* XXX:
> > fill header */)")
> > buff.append("{")
> > - buff.append("\tltl_atom_update(task, LTL_%s, true/false);" %
> > self.atoms[0])
> > + buff.append(f"\tltl_atom_update(task, LTL_{self.atoms[0]},
> > true/false);")
> > buff.append("}")
> > buff.append("")
> > return '\n'.join(buff)
> >
> > def fill_tracepoint_attach_probe(self):
> > - return "\trv_attach_trace_probe(\"%s\", /* XXX: tracepoint */,
> > handle_example_event);" \
> > - % self.name
> > + return f"\trv_attach_trace_probe(\"{self.name}\", /* XXX: tracepoint
> > */, handle_example_event);"
> >
> > def fill_tracepoint_detach_helper(self):
> > - return "\trv_detach_trace_probe(\"%s\", /* XXX: tracepoint */,
> > handle_sample_event);" \
> > - % self.name
> > + return f"\trv_detach_trace_probe(\"{self.name}\", /* XXX: tracepoint
> > */, handle_sample_event);"
> >
> > def fill_atoms_init(self):
> > buff = []
> > for a in self.atoms:
> > - buff.append("\tltl_atom_set(mon, LTL_%s, true/false);" % a)
> > + buff.append(f"\tltl_atom_set(mon, LTL_{a}, true/false);")
> > return '\n'.join(buff)
> >
> > def fill_model_h(self):
^ permalink raw reply
* Re: [PATCH v2 01/20] rv/rvgen: introduce AutomataError exception class
From: Wander Lairson Costa @ 2026-02-05 19:53 UTC (permalink / raw)
To: Gabriele Monaco
Cc: Nam Cao, Steven Rostedt, open list:RUNTIME VERIFICATION (RV),
open list
In-Reply-To: <3552738a6de5a56e1eb5d985dfc1b414dd1a67d5.camel@redhat.com>
On Thu, Feb 05, 2026 at 07:50:57AM +0100, Gabriele Monaco wrote:
> On Wed, 2026-02-04 at 11:42 -0300, Wander Lairson Costa wrote:
> > Replace the generic except Exception block with a custom AutomataError
> > class that inherits from Exception. This provides more precise exception
> > handling for automata parsing and validation errors while avoiding
> > overly broad exception catches that could mask programming errors like
> > SyntaxError or TypeError.
> >
> > The AutomataError class is raised when DOT file processing fails due to
> > invalid format, I/O errors, or malformed automaton definitions. The
> > main entry point catches this specific exception and provides a
> > user-friendly error message to stderr before exiting.
> >
> > Signed-off-by: Wander Lairson Costa <wander@redhat.com>
>
> Looks very good, thanks.
> Now I wonder though why is LTL not included in the change. ltl2ba.py raises a
> few ValueError that should probably be treated just like these AutomataError.
> You could either create a new exception class or share the same for all model
> errors (DA and LTL).
>
> What do you think?
Good point. I am still learning the code base, I missed it. I will add
AutomataError to the of *ba.py files.
>
> Thanks,
> Gabriele
>
> > ---
> > tools/verification/rvgen/__main__.py | 9 ++++++---
> > tools/verification/rvgen/rvgen/automata.py | 17 ++++++++++++-----
> > tools/verification/rvgen/rvgen/dot2c.py | 4 ++--
> > tools/verification/rvgen/rvgen/generator.py | 7 ++-----
> > 4 files changed, 22 insertions(+), 15 deletions(-)
> >
> > diff --git a/tools/verification/rvgen/__main__.py
> > b/tools/verification/rvgen/__main__.py
> > index fa6fc1f4de2f7..3bd438f8476ed 100644
> > --- a/tools/verification/rvgen/__main__.py
> > +++ b/tools/verification/rvgen/__main__.py
> > @@ -8,11 +8,15 @@
> > # For further information, see:
> > # Documentation/trace/rv/da_monitor_synthesis.rst
> >
> > +from sys import stderr
> > +
> > +
> > if __name__ == '__main__':
> > from rvgen.dot2k import dot2k
> > from rvgen.generator import Monitor
> > from rvgen.container import Container
> > from rvgen.ltl2k import ltl2k
> > + from rvgen.automata import AutomataError
> > import argparse
> > import sys
> >
> > @@ -51,9 +55,8 @@ if __name__ == '__main__':
> > sys.exit(1)
> > else:
> > monitor = Container(vars(params))
> > - except Exception as e:
> > - print('Error: '+ str(e))
> > - print("Sorry : :-(")
> > + except AutomataError as e:
> > + print(f"There was an error processing {params.spec}: {e}",
> > file=sys.stderr)
> > sys.exit(1)
> >
> > print("Writing the monitor into the directory %s" % monitor.name)
> > diff --git a/tools/verification/rvgen/rvgen/automata.py
> > b/tools/verification/rvgen/rvgen/automata.py
> > index 3f06aef8d4fdc..6ecd5ccd8f3d3 100644
> > --- a/tools/verification/rvgen/rvgen/automata.py
> > +++ b/tools/verification/rvgen/rvgen/automata.py
> > @@ -10,6 +10,13 @@
> >
> > import ntpath
> >
> > +class AutomataError(Exception):
> > + """Exception raised for errors in automata parsing and validation.
> > +
> > + Raised when DOT file processing fails due to invalid format, I/O errors,
> > + or malformed automaton definitions.
> > + """
> > +
> > class Automata:
> > """Automata class: Reads a dot file and part it as an automata.
> >
> > @@ -32,11 +39,11 @@ class Automata:
> > basename = ntpath.basename(self.__dot_path)
> > if not basename.endswith(".dot") and not basename.endswith(".gv"):
> > print("not a dot file")
> > - raise Exception("not a dot file: %s" % self.__dot_path)
> > + raise AutomataError("not a dot file: %s" % self.__dot_path)
> >
> > model_name = ntpath.splitext(basename)[0]
> > if model_name.__len__() == 0:
> > - raise Exception("not a dot file: %s" % self.__dot_path)
> > + raise AutomataError("not a dot file: %s" % self.__dot_path)
> >
> > return model_name
> >
> > @@ -45,8 +52,8 @@ class Automata:
> > dot_lines = []
> > try:
> > dot_file = open(self.__dot_path)
> > - except:
> > - raise Exception("Cannot open the file: %s" % self.__dot_path)
> > + except OSError as exc:
> > + raise AutomataError(f"Cannot open the file: {self.__dot_path}")
> > from exc
> >
> > dot_lines = dot_file.read().splitlines()
> > dot_file.close()
> > @@ -55,7 +62,7 @@ class Automata:
> > line = dot_lines[cursor].split()
> >
> > if (line[0] != "digraph") and (line[1] != "state_automaton"):
> > - raise Exception("Not a valid .dot format: %s" % self.__dot_path)
> > + raise AutomataError("Not a valid .dot format: %s" %
> > self.__dot_path)
> > else:
> > cursor += 1
> > return dot_lines
> > diff --git a/tools/verification/rvgen/rvgen/dot2c.py
> > b/tools/verification/rvgen/rvgen/dot2c.py
> > index 06a26bf15a7e9..74147ae2942f9 100644
> > --- a/tools/verification/rvgen/rvgen/dot2c.py
> > +++ b/tools/verification/rvgen/rvgen/dot2c.py
> > @@ -13,7 +13,7 @@
> > # For further information, see:
> > # Documentation/trace/rv/deterministic_automata.rst
> >
> > -from .automata import Automata
> > +from .automata import Automata, AutomataError
> >
> > class Dot2c(Automata):
> > enum_suffix = ""
> > @@ -71,7 +71,7 @@ class Dot2c(Automata):
> > min_type = "unsigned int"
> >
> > if self.states.__len__() > 1000000:
> > - raise Exception("Too many states: %d" % self.states.__len__())
> > + raise AutomataError("Too many states: %d" %
> > self.states.__len__())
> >
> > return min_type
> >
> > diff --git a/tools/verification/rvgen/rvgen/generator.py
> > b/tools/verification/rvgen/rvgen/generator.py
> > index 3441385c11770..a7bee6b1ea70c 100644
> > --- a/tools/verification/rvgen/rvgen/generator.py
> > +++ b/tools/verification/rvgen/rvgen/generator.py
> > @@ -51,10 +51,7 @@ class RVGenerator:
> > raise FileNotFoundError("Could not find the rv directory, do you have
> > the kernel source installed?")
> >
> > def _read_file(self, path):
> > - try:
> > - fd = open(path, 'r')
> > - except OSError:
> > - raise Exception("Cannot open the file: %s" % path)
> > + fd = open(path, 'r')
> >
> > content = fd.read()
> >
> > @@ -65,7 +62,7 @@ class RVGenerator:
> > try:
> > path = os.path.join(self.abs_template_dir, file)
> > return self._read_file(path)
> > - except Exception:
> > + except OSError:
> > # Specific template file not found. Try the generic template file
> > in the template/
> > # directory, which is one level up
> > path = os.path.join(self.abs_template_dir, "..", file)
^ permalink raw reply
* Re: [PATCH v2 01/20] rv/rvgen: introduce AutomataError exception class
From: Wander Lairson Costa @ 2026-02-05 19:51 UTC (permalink / raw)
To: Gabriele Monaco
Cc: Steven Rostedt, Nam Cao, open list:RUNTIME VERIFICATION (RV),
open list
In-Reply-To: <ff532aaa9f97f474c21417401654bba73e30c988.camel@redhat.com>
On Thu, Feb 05, 2026 at 01:08:41PM +0100, Gabriele Monaco wrote:
> On Wed, 2026-02-04 at 11:42 -0300, Wander Lairson Costa wrote:
> > Replace the generic except Exception block with a custom AutomataError
> > class that inherits from Exception. This provides more precise exception
> > handling for automata parsing and validation errors while avoiding
> > overly broad exception catches that could mask programming errors like
> > SyntaxError or TypeError.
> >
> > The AutomataError class is raised when DOT file processing fails due to
> > invalid format, I/O errors, or malformed automaton definitions. The
> > main entry point catches this specific exception and provides a
> > user-friendly error message to stderr before exiting.
> >
> > Signed-off-by: Wander Lairson Costa <wander@redhat.com>
> [...]
> > ---
> > @@ -45,8 +52,8 @@ class Automata:
> > dot_lines = []
> > try:
> > dot_file = open(self.__dot_path)
> > - except:
> > - raise Exception("Cannot open the file: %s" % self.__dot_path)
> > + except OSError as exc:
> > + raise AutomataError(f"Cannot open the file: {self.__dot_path}")
> > from exc
> >
>
> You probably don't want to mask all OSError in main.py and allow only this one.
> I think it's alright to keep a simpler message for wrong model file (also LTL!),
> while throwing a splat in other cases (e.g. missing templates).
>
> But wouldn't it be better to show the error message of the OSError? 99% of the
> time it's going to be ENOENT, but could also be some EPERM or who knows what.
> I'm fine keeping AutomataError here but would at least propagate the error
> message.
>
> Or keep it simple and catch all OSError as well, this one included.
Now that we catch OSError in the main, it makes sense to remove the
try/except clause around open().
>
> Thanks,
> Gabriele
>
> >
> > dot_lines = dot_file.read().splitlines()
> > dot_file.close()
> > @@ -55,7 +62,7 @@ class Automata:
> > line = dot_lines[cursor].split()
> >
> > if (line[0] != "digraph") and (line[1] != "state_automaton"):
> > - raise Exception("Not a valid .dot format: %s" % self.__dot_path)
> > + raise AutomataError("Not a valid .dot format: %s" %
> > self.__dot_path)
> > else:
> > cursor += 1
> > return dot_lines
> > diff --git a/tools/verification/rvgen/rvgen/dot2c.py
> > b/tools/verification/rvgen/rvgen/dot2c.py
> > index 06a26bf15a7e9..74147ae2942f9 100644
> > --- a/tools/verification/rvgen/rvgen/dot2c.py
> > +++ b/tools/verification/rvgen/rvgen/dot2c.py
> > @@ -13,7 +13,7 @@
> > # For further information, see:
> > # Documentation/trace/rv/deterministic_automata.rst
> >
> > -from .automata import Automata
> > +from .automata import Automata, AutomataError
> >
> > class Dot2c(Automata):
> > enum_suffix = ""
> > @@ -71,7 +71,7 @@ class Dot2c(Automata):
> > min_type = "unsigned int"
> >
> > if self.states.__len__() > 1000000:
> > - raise Exception("Too many states: %d" % self.states.__len__())
> > + raise AutomataError("Too many states: %d" %
> > self.states.__len__())
> >
> > return min_type
> >
> > diff --git a/tools/verification/rvgen/rvgen/generator.py
> > b/tools/verification/rvgen/rvgen/generator.py
> > index 3441385c11770..a7bee6b1ea70c 100644
> > --- a/tools/verification/rvgen/rvgen/generator.py
> > +++ b/tools/verification/rvgen/rvgen/generator.py
> > @@ -51,10 +51,7 @@ class RVGenerator:
> > raise FileNotFoundError("Could not find the rv directory, do you have
> > the kernel source installed?")
> >
> > def _read_file(self, path):
> > - try:
> > - fd = open(path, 'r')
> > - except OSError:
> > - raise Exception("Cannot open the file: %s" % path)
> > + fd = open(path, 'r')
> >
> > content = fd.read()
> >
> > @@ -65,7 +62,7 @@ class RVGenerator:
> > try:
> > path = os.path.join(self.abs_template_dir, file)
> > return self._read_file(path)
> > - except Exception:
> > + except OSError:
> > # Specific template file not found. Try the generic template file
> > in the template/
> > # directory, which is one level up
> > path = os.path.join(self.abs_template_dir, "..", file)
>
^ permalink raw reply
* Re: [PATCH v13 00/18] unwind_deferred: Implement sframe handling
From: Steven Rostedt @ 2026-02-05 18:54 UTC (permalink / raw)
To: Namhyung Kim
Cc: Jens Remus, linux-kernel, linux-trace-kernel, bpf, x86, linux-mm,
Josh Poimboeuf, Masami Hiramatsu, Mathieu Desnoyers,
Peter Zijlstra, Ingo Molnar, Jiri Olsa, Arnaldo Carvalho de Melo,
Thomas Gleixner, Andrii Nakryiko, Indu Bhagat, Jose E. Marchesi,
Beau Belgrave, Linus Torvalds, Andrew Morton, Florian Weimer,
Kees Cook, Carlos O'Donell, Sam James, Dylan Hatch,
Borislav Petkov, Dave Hansen, David Hildenbrand, H. Peter Anvin,
Liam R. Howlett, Lorenzo Stoakes, Michal Hocko, Mike Rapoport,
Suren Baghdasaryan, Vlastimil Babka, Heiko Carstens,
Vasily Gorbik
In-Reply-To: <aYTgwoyRl8kxQShT@google.com>
On Thu, 5 Feb 2026 10:26:10 -0800
Namhyung Kim <namhyung@kernel.org> wrote:
> > Namhyung Kim's related perf tools deferred callchain support can be used
> > for testing ("perf record --call-graph fp,defer" and "perf report/script").
>
> Is it possible for users to choose the unwinder - frame pointer or
> SFrame at runtime? I feel like the option should be
> "--call-graph sframe,defer" or just "--call-graph sframe" if it always
> uses deferred unwinding.
Currently no, and I'm not sure we want that do we? The idea is to use the
best option that is available. Why use frame pointers if sframe is
available and it's being called with defer?
If there's no defer, then sframes are not available, so it defaults to the
best option available (which will likely be frame pointers).
-- Steve
^ permalink raw reply
* Re: [PATCH v13 00/18] unwind_deferred: Implement sframe handling
From: Namhyung Kim @ 2026-02-05 18:26 UTC (permalink / raw)
To: Jens Remus
Cc: linux-kernel, linux-trace-kernel, bpf, x86, linux-mm,
Steven Rostedt, Josh Poimboeuf, Masami Hiramatsu,
Mathieu Desnoyers, Peter Zijlstra, Ingo Molnar, Jiri Olsa,
Arnaldo Carvalho de Melo, Thomas Gleixner, Andrii Nakryiko,
Indu Bhagat, Jose E. Marchesi, Beau Belgrave, Linus Torvalds,
Andrew Morton, Florian Weimer, Kees Cook, Carlos O'Donell,
Sam James, Dylan Hatch, Borislav Petkov, Dave Hansen,
David Hildenbrand, H. Peter Anvin, Liam R. Howlett,
Lorenzo Stoakes, Michal Hocko, Mike Rapoport, Suren Baghdasaryan,
Vlastimil Babka, Heiko Carstens, Vasily Gorbik
In-Reply-To: <20260127150554.2760964-1-jremus@linux.ibm.com>
Hello,
On Tue, Jan 27, 2026 at 04:05:35PM +0100, Jens Remus wrote:
> This is the implementation of parsing the SFrame V3 stack trace information
> from an .sframe section in an ELF file. It's a continuation of Josh's and
> Steve's work that can be found here:
>
> https://lore.kernel.org/all/cover.1737511963.git.jpoimboe@kernel.org/
> https://lore.kernel.org/all/20250827201548.448472904@kernel.org/
>
> Currently the only way to get a user space stack trace from a stack
> walk (and not just copying large amount of user stack into the kernel
> ring buffer) is to use frame pointers. This has a few issues. The biggest
> one is that compiling frame pointers into every application and library
> has been shown to cause performance overhead.
>
> Another issue is that the format of the frames may not always be consistent
> between different compilers and some architectures (s390) has no defined
> format to do a reliable stack walk. The only way to perform user space
> profiling on these architectures is to copy the user stack into the kernel
> buffer.
>
> SFrame [1] is now supported in binutils (x86-64, ARM64, and s390). There is
> discussions going on about supporting SFrame in LLVM. SFrame acts more like
> ORC, and lives in the ELF executable file as its own section. Like ORC it
> has two tables where the first table is sorted by instruction pointers (IP)
> and using the current IP and finding it's entry in the first table, it will
> take you to the second table which will tell you where the return address
> of the current function is located and then you can use that address to
> look it up in the first table to find the return address of that function,
> and so on. This performs a user space stack walk.
>
> Now because the .sframe section lives in the ELF file it needs to be faulted
> into memory when it is used. This means that walking the user space stack
> requires being in a faultable context. As profilers like perf request a stack
> trace in interrupt or NMI context, it cannot do the walking when it is
> requested. Instead it must be deferred until it is safe to fault in user
> space. One place this is known to be safe is when the task is about to return
> back to user space.
>
> This series makes the deferred unwind user code implement SFrame format V3
> and enables it on x86-64.
>
> [1]: https://sourceware.org/binutils/wiki/sframe
>
>
> This series applies on top of the tip perf/core branch:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git perf/core
>
> The to be stack-traced user space programs (and libraries) need to be
> built with the recent SFrame stack trace information format V3, as
> generated by the upcoming binutils 2.46 with assembler option --gsframe.
> It can be built from source from the binutils-2_46-branch branch:
>
> git://sourceware.org/git/binutils-gdb.git binutils-2_46-branch
>
> Namhyung Kim's related perf tools deferred callchain support can be used
> for testing ("perf record --call-graph fp,defer" and "perf report/script").
Is it possible for users to choose the unwinder - frame pointer or
SFrame at runtime? I feel like the option should be
"--call-graph sframe,defer" or just "--call-graph sframe" if it always
uses deferred unwinding.
Thanks,
Namhyung
>
>
> Changes since v12 (see patch notes for details):
> - Rebase on tip perf/core branch (d55c571e4333).
> - Add support for SFrame V3, including its new flexible FDEs. SFrame V2
> is not supported.
>
> Changes since v11 (see patch notes for details):
> - Rebase on tip master branch (f8fdee44bf2f) with Namhyung Kim's
> perf/defer-callchain-v4 branch merged on top.
> - Adjust to Peter's latest undwind user enhancements.
> - Simplify logic by using an internal SFrame FDE representation, whose
> FDE function start address field is an address instead of a PC-relative
> offset (from FDE).
> - Rename struct sframe_fre to sframe_fre_internal to align with
> struct sframe_fde_internal.
> - Remove unused pt_regs from unwind_user_next_common() and its
> callers. (Peter)
> - Simplify unwind_user_next_sframe(). (Peter)
> - Fix a few checkpatch errors and warnings.
> - Minor cleanups (e.g. move includes, fix indentation).
>
> Changes since v10:
> - Support for SFrame V2 PC-relative FDE function start address.
> - Support for SFrame V2 representing RA undefined as indication for
> outermost frames.
>
>
> Patches 1, 4, 11, and 17 have been updated to exclusively support the
> latest SFrame V3 stack trace information format, that is generated by
> the upcoming binutils 2.46 release. Old SFrame V2 sections get rejected
> with dynamic debug message "bad/unsupported sframe header".
>
> Patches 7 and 8 add support to unwind user (sframe) for outermost frames.
>
> Patches 12-15 add support to unwind user (sframe) for the new SFrame V3
> flexible FDEs.
>
> Patch 16 improves the performance of searching the SFrame FRE for an IP.
>
> Regards,
> Jens
>
>
> Jens Remus (7):
> unwind_user: Stop when reaching an outermost frame
> unwind_user/sframe: Add support for outermost frame indication
> unwind_user: Enable archs that pass RA in a register
> unwind_user: Flexible FP/RA recovery rules
> unwind_user: Flexible CFA recovery rules
> unwind_user/sframe: Add support for SFrame V3 flexible FDEs
> unwind_user/sframe: Separate reading of FRE from reading of FRE data
> words
>
> Josh Poimboeuf (11):
> unwind_user/sframe: Add support for reading .sframe headers
> unwind_user/sframe: Store .sframe section data in per-mm maple tree
> x86/uaccess: Add unsafe_copy_from_user() implementation
> unwind_user/sframe: Add support for reading .sframe contents
> unwind_user/sframe: Detect .sframe sections in executables
> unwind_user/sframe: Wire up unwind_user to sframe
> unwind_user/sframe: Remove .sframe section on detected corruption
> unwind_user/sframe: Show file name in debug output
> unwind_user/sframe: Add .sframe validation option
> unwind_user/sframe/x86: Enable sframe unwinding on x86
> unwind_user/sframe: Add prctl() interface for registering .sframe
> sections
>
> MAINTAINERS | 1 +
> arch/Kconfig | 23 +
> arch/x86/Kconfig | 1 +
> arch/x86/include/asm/mmu.h | 2 +-
> arch/x86/include/asm/uaccess.h | 39 +-
> arch/x86/include/asm/unwind_user.h | 69 +-
> arch/x86/include/asm/unwind_user_sframe.h | 12 +
> fs/binfmt_elf.c | 48 +-
> include/linux/mm_types.h | 3 +
> include/linux/sframe.h | 60 ++
> include/linux/unwind_user.h | 18 +
> include/linux/unwind_user_types.h | 46 +-
> include/uapi/linux/elf.h | 1 +
> include/uapi/linux/prctl.h | 6 +-
> kernel/fork.c | 10 +
> kernel/sys.c | 9 +
> kernel/unwind/Makefile | 3 +-
> kernel/unwind/sframe.c | 840 ++++++++++++++++++++++
> kernel/unwind/sframe.h | 87 +++
> kernel/unwind/sframe_debug.h | 68 ++
> kernel/unwind/user.c | 105 ++-
> mm/init-mm.c | 2 +
> 22 files changed, 1414 insertions(+), 39 deletions(-)
> create mode 100644 arch/x86/include/asm/unwind_user_sframe.h
> create mode 100644 include/linux/sframe.h
> create mode 100644 kernel/unwind/sframe.c
> create mode 100644 kernel/unwind/sframe.h
> create mode 100644 kernel/unwind/sframe_debug.h
>
> --
> 2.51.0
>
^ permalink raw reply
* Re: [PATCH 2/2] tracing: resolve enum names for function arguments via BTF
From: Steven Rostedt @ 2026-02-05 18:04 UTC (permalink / raw)
To: Donglin Peng
Cc: Alexei Starovoitov, Andrii Nakryiko, Alexei Starovoitov,
Masami Hiramatsu, LKML, Donglin Peng, linux-trace-kernel, bpf,
Eduard Zingerman
In-Reply-To: <CAErzpmu9mNXTOw-WwkSGEdvmJY2VjyUbGWZD=5s6kXNRE_YkJQ@mail.gmail.com>
On Wed, 4 Feb 2026 22:52:11 +0800
Donglin Peng <dolinux.peng@gmail.com> wrote:
> > > I have trace-cmd reading BTF now (just haven't officially released it) and
> > > doing an extract and reading the trace.dat file is much faster than reading
> > > the trace file with arguments. I'll need to implement the enum logic too in
> > > libtraceevent.
> >
> > If you mean to do pretty printing of the trace in user space then +1 from me.
> >
> > I don't like sorting enums either in resolve_btfid, pahole or kernel.
> > Sorted BTF by name was ok, since it doesn't change original semantics.
> > While sorting enums by value gets us to the grey zone where
> > the sequence of enum names in vmlinux.h becomes different than in dwarf.
>
> Thanks, I agreed.
BTW, I just officially released trace-cmd v3.4 (where you can see whats
new in that release here[1]).
The biggest change is that it saves the BTF file in the trace.dat file
and parses it on the report (it requires libtraceevent v1.9):
~# trace-cmd record -p function_graph -O funcgraph-args -g do_sys_openat2
[..]
~# trace-cmd report
trace-cmd-50935 [002] ...1. 3490.518138: funcgraph_entry: | do_sys_openat2(dfd=4294967196, filename=0x557bb9e3ee10, how=0xffff88815220fea8) {
trace-cmd-50935 [002] ...1. 3490.518141: funcgraph_entry: | getname_flags(filename=0x557bb9e3ee10, flags=0) {
trace-cmd-50935 [002] ...1. 3490.518142: funcgraph_entry: | getname_flags.part.0(filename=0x557bb9e3ee10, flags=0) {
trace-cmd-50935 [002] ...1. 3490.518143: funcgraph_entry: | kmem_cache_alloc_noprof(s=0xffff888106c7e000, gfpflags=0xcc0) {
trace-cmd-50935 [002] ...1. 3490.518145: funcgraph_entry: | stack_trace_save(store=0xffff88815220fac8, size=0x40, skipnr=0x0) {
trace-cmd-50935 [002] ...1. 3490.518147: funcgraph_entry: | arch_stack_walk(consume_entry=0xffffffff94d9dfe0, cookie=0xffff88815220fa58, task=0xffff88812d4c3580, regs=0x0) {
trace-cmd-50935 [002] ...1. 3490.518148: funcgraph_entry: | __unwind_start(state=0xffff88815220f988, task=0xffff88812d4c3580, regs=0x0, first_frame=0xffff88815220fa28) {
trace-cmd-50935 [002] ...1. 3490.518149: funcgraph_entry: 1.518 us | get_stack_info(stack=0xffff88815220f938, task=0xffff88812d4c3580, info=0xffff88815220f988, visit_mask=0xffff88815220f9a8); (ret=0x0)
trace-cmd-50935 [002] ...1. 3490.518152: funcgraph_entry: | unwind_next_frame(state=0xffff88815220f988) {
trace-cmd-50935 [002] ...1. 3490.518153: funcgraph_entry: 0.951 us | __rcu_read_lock(); (ret=0xffff88812d4c3580)
-- Steve
[1] https://git.kernel.org/pub/scm/utils/trace-cmd/trace-cmd.git/tag/?h=trace-cmd-v3.4
^ permalink raw reply
* Re: [PATCH v11 00/30] Tracefs support for pKVM
From: Steven Rostedt @ 2026-02-05 17:51 UTC (permalink / raw)
To: Vincent Donnefort
Cc: mhiramat, mathieu.desnoyers, linux-trace-kernel, maz,
oliver.upton, joey.gouly, suzuki.poulose, yuzenghui, kvmarm,
linux-arm-kernel, jstultz, qperret, will, aneesh.kumar,
kernel-team, linux-kernel
In-Reply-To: <20260204174502.06d6e02e@gandalf.local.home>
On Wed, 4 Feb 2026 17:45:02 -0500
Steven Rostedt <rostedt@goodmis.org> wrote:
> > changes since v10
>
> BTW, it's always nice to add a link to the previous version here. That way
> it makes it easy to backtrace from one version to the next (and reviewers
> to see what they said the last time).
>
> Changes since v10: https://lore.kernel.org/all/20260126104419.1649811-1-vdonnefort@google.com/
So I finished my review of v11. Still some with comments, and the ones I
didn't have comments for I added my Reviewed-by tags.
-- Steve
^ permalink raw reply
* Re: [PATCH v11 17/30] tracing: load/unload page callbacks for simple_ring_buffer
From: Steven Rostedt @ 2026-02-05 17:47 UTC (permalink / raw)
To: Vincent Donnefort
Cc: mhiramat, mathieu.desnoyers, linux-trace-kernel, maz,
oliver.upton, joey.gouly, suzuki.poulose, yuzenghui, kvmarm,
linux-arm-kernel, jstultz, qperret, will, aneesh.kumar,
kernel-team, linux-kernel
In-Reply-To: <20260131132848.254084-18-vdonnefort@google.com>
On Sat, 31 Jan 2026 13:28:35 +0000
Vincent Donnefort <vdonnefort@google.com> wrote:
> Add load/unload callback used for each admitted page in the ring-buffer.
> This will be later useful for the pKVM hypervisor which uses a different
> VA space and need to dynamically map/unmap the ring-buffer pages.
>
> Signed-off-by: Vincent Donnefort <vdonnefort@google.com>
Reviewed-by: Steven Rostedt (Google) <rostedt@goodmis.org>
-- Steve
^ permalink raw reply
* Re: [PATCH v11 16/30] Documentation: tracing: Add tracing remotes
From: Steven Rostedt @ 2026-02-05 17:45 UTC (permalink / raw)
To: Vincent Donnefort
Cc: mhiramat, mathieu.desnoyers, linux-trace-kernel, maz,
oliver.upton, joey.gouly, suzuki.poulose, yuzenghui, kvmarm,
linux-arm-kernel, jstultz, qperret, will, aneesh.kumar,
kernel-team, linux-kernel
In-Reply-To: <20260131132848.254084-17-vdonnefort@google.com>
On Sat, 31 Jan 2026 13:28:34 +0000
Vincent Donnefort <vdonnefort@google.com> wrote:
> Add documentation about the newly introduced tracing remotes framework.
>
> Signed-off-by: Vincent Donnefort <vdonnefort@google.com>
>
Reviewed-by: Steven Rostedt (Google) <rostedt@goodmis.org>
But in the future, this document should probably go into more details about
what is expected by each callback.
-- Steve
> diff --git a/Documentation/trace/index.rst b/Documentation/trace/index.rst
> index b4a429dc4f7a..d77ffb7e2d08 100644
> --- a/Documentation/trace/index.rst
> +++ b/Documentation/trace/index.rst
> @@ -90,6 +90,17 @@ interactions.
> user_events
> uprobetracer
>
> +Remote Tracing
> +--------------
> +
> +This section covers the framework to read compatible ring-buffers, written by
> +entities outside of the kernel (most likely firmware or hypervisor)
> +
> +.. toctree::
> + :maxdepth: 1
> +
> + remotes
> +
> Additional Resources
> --------------------
>
> diff --git a/Documentation/trace/remotes.rst b/Documentation/trace/remotes.rst
> new file mode 100644
> index 000000000000..1f9d764f69aa
> --- /dev/null
> +++ b/Documentation/trace/remotes.rst
> @@ -0,0 +1,66 @@
> +.. SPDX-License-Identifier: GPL-2.0
> +
> +===============
> +Tracing Remotes
> +===============
> +
> +:Author: Vincent Donnefort <vdonnefort@google.com>
> +
> +Overview
> +========
> +Firmware and hypervisors are black boxes to the kernel. Having a way to see what
> +they are doing can be useful to debug both. This is where remote tracing buffers
> +come in. A remote tracing buffer is a ring buffer executed by the firmware or
> +hypervisor into memory that is memory mapped to the host kernel. This is similar
> +to how user space memory maps the kernel ring buffer but in this case the kernel
> +is acting like user space and the firmware or hypervisor is the "kernel" side.
> +With a trace remote ring buffer, the firmware and hypervisor can record events
> +for which the host kernel can see and expose to user space.
> +
> +Register a remote
> +=================
> +A remote must provide a set of callbacks `struct trace_remote_callbacks` whom
> +description can be found below. Those callbacks allows Tracefs to enable and
> +disable tracing and events, to load and unload a tracing buffer (a set of
> +ring-buffers) and to swap a reader page with the head page, which enables
> +consuming reading.
> +
> +.. kernel-doc:: include/linux/trace_remote.h
> +
> +Once registered, an instance will appear for this remote in the Tracefs
> +directory **remotes/**. Buffers can then be read using the usual Tracefs files
> +**trace_pipe** and **trace**.
> +
> +Declare a remote event
> +======================
> +Macros are provided to ease the declaration of remote events, in a similar
> +fashion to in-kernel events. A declaration must provide an ID, a description of
> +the event arguments and how to print the event:
> +
> +.. code-block:: c
> +
> + REMOTE_EVENT(foo, EVENT_FOO_ID,
> + RE_STRUCT(
> + re_field(u64, bar)
> + ),
> + RE_PRINTK("bar=%lld", __entry->bar)
> + );
> +
> +Then those events must be declared in a C file with the following:
> +
> +.. code-block:: c
> +
> + #define REMOTE_EVENT_INCLUDE_FILE foo_events.h
> + #include <trace/define_remote_events.h>
> +
> +This will provide a `struct remote_event remote_event_foo` that can be given to
> +`trace_remote_register`.
> +
> +Registered events appear in the remote directory under **events/**.
> +
> +Simple ring-buffer
> +==================
> +A simple implementation for a ring-buffer writer can be found in
> +kernel/trace/simple_ring_buffer.c.
> +
> +.. kernel-doc:: include/linux/simple_ring_buffer.h
^ permalink raw reply
* Re: [PATCH v11 15/30] tracing: selftests: Add trace remote tests
From: Steven Rostedt @ 2026-02-05 17:42 UTC (permalink / raw)
To: Vincent Donnefort
Cc: mhiramat, mathieu.desnoyers, linux-trace-kernel, maz,
oliver.upton, joey.gouly, suzuki.poulose, yuzenghui, kvmarm,
linux-arm-kernel, jstultz, qperret, will, aneesh.kumar,
kernel-team, linux-kernel, Shuah Khan, linux-kselftest
In-Reply-To: <20260131132848.254084-16-vdonnefort@google.com>
On Sat, 31 Jan 2026 13:28:33 +0000
Vincent Donnefort <vdonnefort@google.com> wrote:
> Exercise the tracefs interface for trace remote with a set of tests to
> check:
>
> * loading/unloading (unloading.tc)
> * reset (reset.tc)
> * size changes (buffer_size.tc)
> * consuming read (trace_pipe.tc)
> * non-consuming read (trace.tc)
>
> Cc: Shuah Khan <skhan@linuxfoundation.org>
> Cc: linux-kselftest@vger.kernel.org
> Signed-off-by: Vincent Donnefort <vdonnefort@google.com>
This still fails:
=== Ftrace unit tests ===
[1] Test trace remote buffer size [PASS]
[2] Test hypervisor trace buffer size [UNSUPPORTED]
[3] Test hypervisor trace buffer reset [UNSUPPORTED]
[4] Test hypervisor consuming trace read [UNSUPPORTED]
[5] Test hypervisor non-consuming trace read [UNSUPPORTED]
[6] Test hypervisor trace buffer unloading [UNSUPPORTED]
[7] Test trace remote reset [PASS]
[8] Test trace remote consuming read [FAIL]
[9] Test trace remote non-consuming read [FAIL]
[10] Test trace remote unloading [PASS]
I added this patch and the two failed tests now pass:
diff --git a/tools/testing/selftests/ftrace/test.d/remotes/trace.tc b/tools/testing/selftests/ftrace/test.d/remotes/trace.tc
index 081133ec45ff..dfc954a6a380 100644
--- a/tools/testing/selftests/ftrace/test.d/remotes/trace.tc
+++ b/tools/testing/selftests/ftrace/test.d/remotes/trace.tc
@@ -106,8 +106,10 @@ test_trace()
echo 0 > trace
for cpu in $(get_cpu_ids); do
- echo 0 > /sys/devices/system/cpu/cpu$cpu/online
- break
+ if [ -f /sys/devices/system/cpu/cpu$cpu/online ]; then
+ echo 0 > /sys/devices/system/cpu/cpu$cpu/online
+ break
+ fi
done
for i in $(seq 1 8); do
diff --git a/tools/testing/selftests/ftrace/test.d/remotes/trace_pipe.tc b/tools/testing/selftests/ftrace/test.d/remotes/trace_pipe.tc
index d28eaee10c7c..146f0a9fe311 100644
--- a/tools/testing/selftests/ftrace/test.d/remotes/trace_pipe.tc
+++ b/tools/testing/selftests/ftrace/test.d/remotes/trace_pipe.tc
@@ -102,8 +102,10 @@ test_trace_pipe()
echo 0 > trace
for cpu in $(get_cpu_ids); do
- echo 0 > /sys/devices/system/cpu/cpu$cpu/online
- break
+ if [ -f /sys/devices/system/cpu/cpu$cpu/online ]; then
+ echo 0 > /sys/devices/system/cpu/cpu$cpu/online
+ break
+ fi
done
for i in $(seq 1 8); do
-- Steve
^ permalink raw reply related
* Re: [PATCH 2/2] tracing: resolve enum names for function arguments via BTF
From: Alexei Starovoitov @ 2026-02-05 15:56 UTC (permalink / raw)
To: Donglin Peng
Cc: Steven Rostedt, Andrii Nakryiko, Alexei Starovoitov,
Masami Hiramatsu, LKML, Donglin Peng, linux-trace-kernel, bpf,
Eduard Zingerman
In-Reply-To: <CAErzpmvtJwB7nTD5EFHP9SxPxH-rc3NuJp5xLGSTB7YPbUX8YA@mail.gmail.com>
On Thu, Feb 5, 2026 at 1:21 AM Donglin Peng <dolinux.peng@gmail.com> wrote:
>
> On Wed, Feb 4, 2026 at 10:52 PM Donglin Peng <dolinux.peng@gmail.com> wrote:
> >
> > On Wed, Feb 4, 2026 at 12:00 AM Alexei Starovoitov
> > <alexei.starovoitov@gmail.com> wrote:
> > >
> > > On Tue, Feb 3, 2026 at 7:16 AM Steven Rostedt <rostedt@goodmis.org> wrote:
> > > >
> > > > On Tue, 3 Feb 2026 21:50:47 +0800
> > > > Donglin Peng <dolinux.peng@gmail.com> wrote:
> > > >
> > > > > Testing revealed that sorting within resolve_btfids introduces issues with
> > > > > btf__dedup. Therefore, I plan to move the sorting logic directly into
> > > > > btf__add_enum_value and btf__add_enum64_value in libbpf, which are
> > > > > invoked by pahole. However, it means that we need a newer pahole
> > > > > version.
> > > >
> > > > Sorting isn't a requirement just something I wanted to bring up. If it's
> > > > too complex and doesn't achieve much benefit then let's not do it.
> > > >
> > > > My worry is because "cat trace" takes quite a long time just reading the
> > > > BTF arguments. I'm worried it will just get worse with enums as well.
> > > >
> > > > I have trace-cmd reading BTF now (just haven't officially released it) and
> > > > doing an extract and reading the trace.dat file is much faster than reading
> > > > the trace file with arguments. I'll need to implement the enum logic too in
> > > > libtraceevent.
> > >
> > > If you mean to do pretty printing of the trace in user space then +1 from me.
> > >
> > > I don't like sorting enums either in resolve_btfid, pahole or kernel.
> > > Sorted BTF by name was ok, since it doesn't change original semantics.
> > > While sorting enums by value gets us to the grey zone where
> > > the sequence of enum names in vmlinux.h becomes different than in dwarf.
> >
> > Thanks, I agreed.
> >
> > >
> > > Also id->name mapping in general is not precise.
> > > There is no requirement for enums to be unique.
> > > Just grabbing the first one:
> > > ATA_PIO0 = 1,
> > > ATA_PIO1 = 3,
> > > ATA_PIO2 = 7,
> > > ATA_UDMA0 = 1,
> > > ATA_UDMA1 = 3,
> > > ATA_UDMA2 = 7,
> > > ATA_ID_CYLS = 1,
> > > ATA_ID_HEADS = 3,
> > > SCR_ERROR = 1,
> > > SCR_CONTROL = 2,
> > > SCR_ACTIVE = 3,
> > >
> > > All these names are part of the same enum type.
> > > Which one to print? First one?
>
> Another option is to print all matching entries, incurring increased
> overhead and extended trace log length. However, I prefer printing
> the first matching entry, though it might be inaccurate in rare cases.
I disagree. It's not rare.
I wouldn't print anything. Let user space deal with it.
^ permalink raw reply
* Re: [RFC bpf-next 00/12] bpf: tracing_multi link
From: Alexei Starovoitov @ 2026-02-05 15:55 UTC (permalink / raw)
To: Jiri Olsa
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, bpf,
linux-trace-kernel, Martin KaFai Lau, Eduard Zingerman, Song Liu,
Yonghong Song, Menglong Dong, Steven Rostedt
In-Reply-To: <aYRa_CDL1RqIR_8S@krava>
On Thu, Feb 5, 2026 at 12:55 AM Jiri Olsa <olsajiri@gmail.com> wrote:
>
> On Wed, Feb 04, 2026 at 08:06:50AM -0800, Alexei Starovoitov wrote:
> > On Wed, Feb 4, 2026 at 4:36 AM Jiri Olsa <olsajiri@gmail.com> wrote:
> > >
> > > On Tue, Feb 03, 2026 at 03:17:05PM -0800, Alexei Starovoitov wrote:
> > > > On Tue, Feb 3, 2026 at 1:38 AM Jiri Olsa <jolsa@kernel.org> wrote:
> > > > >
> > > > > hi,
> > > > > as an option to Meglong's change [1] I'm sending proposal for tracing_multi
> > > > > link that does not add static trampoline but attaches program to all needed
> > > > > trampolines.
> > > > >
> > > > > This approach keeps the same performance but has some drawbacks:
> > > > >
> > > > > - when attaching 20k functions we allocate and attach 20k trampolines
> > > > > - during attachment we hold each trampoline mutex, so for above
> > > > > 20k functions we will hold 20k mutexes during the attachment,
> > > > > should be very prone to deadlock, but haven't hit it yet
> > > >
> > > > If you check that it's sorted and always take them in the same order
> > > > then there will be no deadlock.
> > > > Or just grab one global mutex first and then grab trampolines mutexes
> > > > next in any order. The global one will serialize this attach operation.
> > > >
> > > > > It looks the trampoline allocations/generation might not be big a problem
> > > > > and I'll try to find a solution for holding that many mutexes. If there's
> > > > > no better solution I think having one read/write mutex for tracing multi
> > > > > link attach/detach should work.
> > > >
> > > > If you mean to have one global mutex as I proposed above then I don't see
> > > > a downside. It only serializes multiple libbpf calls.
> > >
> > > we also need to serialize it with standard single trampoline attach,
> > > because the direct ftrace update is now done under trampoline->mutex:
> > >
> > > bpf_trampoline_link_prog(tr)
> > > {
> > > mutex_lock(&tr->mutex);
> > > ...
> > > update_ftrace_direct_*
> > > ...
> > > mutex_unlock(&tr->mutex);
> > > }
> > >
> > > for tracing_multi we would link the program first (with tr->mutex)
> > > and do the bulk ftrace update later (without tr->mutex)
> > >
> > > {
> > > for each involved trampoline:
> > > bpf_trampoline_link_prog
> > >
> > > --> and here we could race with some other thread doing single
> > > trampoline attach
> > >
> > > update_ftrace_direct_*
> > > }
> > >
> > > note the current version locks all tr->mutex instances all the way
> > > through the update_ftrace_direct_* update
> > >
> > > I think we could use global rwsem and take read lock on single
> > > trampoline attach path and write lock on tracing_multi attach,
> > >
> > > I thought we could take direct_mutex early, but that would mean
> > > different order with trampoline mutex than we already have in
> > > single attach path
> >
> > I feel we're talking past each other.
> > I meant:
> >
> > For multi:
> > 1. take some global mutex
> > 2. take N tramp mutexes in any order
> >
> > For single:
> > 1. take that 1 specific tramp mutex.
>
> ah ok, I understand, it's to prevent the lockup but keep holding all
> the trampolines locks.. the rwsem I mentioned was for the 'fix', where
> we do not take all the trampolines locks
I don't understand how rwsem would help.
All the operations on trampoline are protected by mutex.
Switching to rw makes sense only if we can designate certain
operations as "read" and others as "write" and number of "reads"
dominate. This won't be the case with multi-fentry.
And we still need to take all of them as "write" to update trampoline.
^ permalink raw reply
* Re: [PATCH v2] trace/hwlat: prevent false sharing in get_sample()
From: Steven Rostedt @ 2026-02-05 14:47 UTC (permalink / raw)
To: Colin Lord
Cc: linux-kernel, linux-trace-kernel, Masami Hiramatsu,
Mathieu Desnoyers
In-Reply-To: <20260202025838.32057-1-clord@mykolab.com>
On Sun, 1 Feb 2026 18:58:38 -0800
Colin Lord <clord@mykolab.com> wrote:
I was about to send this as part of my fixes, but taking a closer look at
it, I have more questions.
> The get_sample() function in the hwlat tracer assumes the caller holds
> hwlat_data.lock, but this is not actually happening. The result is
> unprotected data access to hwlat_data, and in per-cpu mode can result in
> false sharing. The false sharing can cause false positive latency
BTW, what exactly do you mean by "false sharing"?
> events, since the sample_width member is involved and gets read as part
> of the main latency detection loop.
I'm trying to figure out why the change in sample_width would cause any
issue.
>
> Convert hwlat_data.count to atomic64_t so it can be safely accessed
> without locking, and prevent false sharing by pulling sample_width into
> a local variable.
The above still makes sense, but this only effects the seqnum, which may
show either duplicate or skipped numbers. But shouldn't affect any of the
other data.
>
> One system this was tested on was a dual socket server with 32 CPUs on
> each numa node. With settings of 1us threshold, 1000us width, and
> 2000us window, this change reduced the number of latency events from
> 500 per second down to approximately 1 event per minute. Some machines
> tested did not exhibit measurable latency from the false sharing.
Is this because the read of hwlat_data.sample_width is a global variable
and could possibly be causing a cache hit latency that shows up on large
machines?
Is that what you mean by "false sharing"?
Oh, and subjects for the tracing subsystem should start with a capital, and
should be something like:
tracing: Fix get_sample() in hwlat from ...
-- Steve
>
> Signed-off-by: Colin Lord <clord@mykolab.com>
> ---
> Changes in v2:
> - convert hwlat_data.count to atomic64_t
> - leave irqs_disabled block where it originally was, outside of
> get_sample()
>
> Thanks for the v1 review Steve, have updated and retested.
>
> cheers,
> Colin
>
> kernel/trace/trace_hwlat.c | 15 +++++++--------
> 1 file changed, 7 insertions(+), 8 deletions(-)
>
> diff --git a/kernel/trace/trace_hwlat.c b/kernel/trace/trace_hwlat.c
> index 2f7b94e98317..3fe274b84f1c 100644
> --- a/kernel/trace/trace_hwlat.c
> +++ b/kernel/trace/trace_hwlat.c
> @@ -102,9 +102,9 @@ struct hwlat_sample {
> /* keep the global state somewhere. */
> static struct hwlat_data {
>
> - struct mutex lock; /* protect changes */
> + struct mutex lock; /* protect changes */
>
> - u64 count; /* total since reset */
> + atomic64_t count; /* total since reset */
>
> u64 sample_window; /* total sampling window (on+off) */
> u64 sample_width; /* active sampling portion of window */
> @@ -193,8 +193,7 @@ void trace_hwlat_callback(bool enter)
> * get_sample - sample the CPU TSC and look for likely hardware latencies
> *
> * Used to repeatedly capture the CPU TSC (or similar), looking for potential
> - * hardware-induced latency. Called with interrupts disabled and with
> - * hwlat_data.lock held.
> + * hardware-induced latency. Called with interrupts disabled.
> */
> static int get_sample(void)
> {
> @@ -204,6 +203,7 @@ static int get_sample(void)
> time_type start, t1, t2, last_t2;
> s64 diff, outer_diff, total, last_total = 0;
> u64 sample = 0;
> + u64 sample_width = READ_ONCE(hwlat_data.sample_width);
> u64 thresh = tracing_thresh;
> u64 outer_sample = 0;
> int ret = -1;
> @@ -267,7 +267,7 @@ static int get_sample(void)
> if (diff > sample)
> sample = diff; /* only want highest value */
>
> - } while (total <= hwlat_data.sample_width);
> + } while (total <= sample_width);
>
> barrier(); /* finish the above in the view for NMIs */
> trace_hwlat_callback_enabled = false;
> @@ -285,8 +285,7 @@ static int get_sample(void)
> if (kdata->nmi_total_ts)
> do_div(kdata->nmi_total_ts, NSEC_PER_USEC);
>
> - hwlat_data.count++;
> - s.seqnum = hwlat_data.count;
> + s.seqnum = atomic64_inc_return(&hwlat_data.count);
> s.duration = sample;
> s.outer_duration = outer_sample;
> s.nmi_total_ts = kdata->nmi_total_ts;
> @@ -832,7 +831,7 @@ static int hwlat_tracer_init(struct trace_array *tr)
>
> hwlat_trace = tr;
>
> - hwlat_data.count = 0;
> + atomic64_set(&hwlat_data.count, 0);
> tr->max_latency = 0;
> save_tracing_thresh = tracing_thresh;
>
>
> base-commit: 24d479d26b25bce5faea3ddd9fa8f3a6c3129ea7
^ permalink raw reply
* [PATCH v6 4/4] selftests/ftrace: Add accept cases for fprobe list syntax
From: Seokwoo Chung (Ryan) @ 2026-02-05 13:58 UTC (permalink / raw)
To: mhiramat
Cc: rostedt, corbet, shuah, mathieu.desnoyers, linux-kernel,
linux-trace-kernel, linux-doc, linux-kselftest,
Seokwoo Chung (Ryan)
In-Reply-To: <20260205135842.20517-1-seokwoo.chung130@gmail.com>
Add fprobe_list.tc to test the comma-separated symbol list syntax
with :entry/:exit suffixes. Three scenarios are covered:
1. List with default (entry) behavior and ! exclusion
2. List with explicit :entry suffix
3. List with :exit suffix for return probes
Each test verifies that the correct functions appear in
enabled_functions and that excluded (!) symbols are absent.
Note: The existing tests add_remove_fprobe.tc, fprobe_syntax_errors.tc,
and add_remove_fprobe_repeat.tc check their "requires" line against the
tracefs README for the old "%return" syntax pattern. Since the README
now documents ":entry|:exit" instead, these tests report UNSUPPORTED.
Their "requires" lines need updating in a follow-up patch.
Signed-off-by: Seokwoo Chung (Ryan) <seokwoo.chung130@gmail.com>
---
.../ftrace/test.d/dynevent/fprobe_list.tc | 92 +++++++++++++++++++
1 file changed, 92 insertions(+)
create mode 100644 tools/testing/selftests/ftrace/test.d/dynevent/fprobe_list.tc
diff --git a/tools/testing/selftests/ftrace/test.d/dynevent/fprobe_list.tc b/tools/testing/selftests/ftrace/test.d/dynevent/fprobe_list.tc
new file mode 100644
index 000000000000..45e57c6f487d
--- /dev/null
+++ b/tools/testing/selftests/ftrace/test.d/dynevent/fprobe_list.tc
@@ -0,0 +1,92 @@
+#!/bin/sh
+# SPDX-License-Identifier: GPL-2.0
+# description: Fprobe event list syntax and :entry/:exit suffixes
+# requires: dynamic_events "f[:[<group>/][<event>]] <func-name>[:entry|:exit] [<args>]":README
+
+# Setup symbols to test. These are common kernel functions.
+PLACE=vfs_read
+PLACE2=vfs_write
+PLACE3=vfs_open
+
+echo 0 > events/enable
+echo > dynamic_events
+
+# Get baseline count of enabled functions (should be 0 if clean, but be safe)
+if [ -f enabled_functions ]; then
+ ocnt=`cat enabled_functions | wc -l`
+else
+ ocnt=0
+fi
+
+# Test 1: List default (entry) with exclusion
+# Target: Trace vfs_read and vfs_open, but EXCLUDE vfs_write
+echo "f:test/list_entry $PLACE,!$PLACE2,$PLACE3" >> dynamic_events
+grep -q "test/list_entry" dynamic_events
+test -d events/test/list_entry
+
+echo 1 > events/test/list_entry/enable
+
+grep -q "$PLACE" enabled_functions
+grep -q "$PLACE3" enabled_functions
+! grep -q "$PLACE2" enabled_functions
+
+# Check count (Baseline + 2 new functions)
+cnt=`cat enabled_functions | wc -l`
+if [ $cnt -ne $((ocnt + 2)) ]; then
+ exit_fail
+fi
+
+# Cleanup Test 1
+echo 0 > events/test/list_entry/enable
+echo "-:test/list_entry" >> dynamic_events
+! grep -q "test/list_entry" dynamic_events
+
+# Count should return to baseline
+cnt=`cat enabled_functions | wc -l`
+if [ $cnt -ne $ocnt ]; then
+ exit_fail
+fi
+
+# Test 2: List with explicit :entry suffix
+# (Should behave exactly like Test 1)
+echo "f:test/list_entry_exp $PLACE,!$PLACE2,$PLACE3:entry" >> dynamic_events
+grep -q "test/list_entry_exp" dynamic_events
+test -d events/test/list_entry_exp
+
+echo 1 > events/test/list_entry_exp/enable
+
+grep -q "$PLACE" enabled_functions
+grep -q "$PLACE3" enabled_functions
+! grep -q "$PLACE2" enabled_functions
+
+cnt=`cat enabled_functions | wc -l`
+if [ $cnt -ne $((ocnt + 2)) ]; then
+ exit_fail
+fi
+
+# Cleanup Test 2
+echo 0 > events/test/list_entry_exp/enable
+echo "-:test/list_entry_exp" >> dynamic_events
+
+# Test 3: List with :exit suffix
+echo "f:test/list_exit $PLACE,!$PLACE2,$PLACE3:exit" >> dynamic_events
+grep -q "test/list_exit" dynamic_events
+test -d events/test/list_exit
+
+echo 1 > events/test/list_exit/enable
+
+# Even for return probes, enabled_functions lists the attached symbols
+grep -q "$PLACE" enabled_functions
+grep -q "$PLACE3" enabled_functions
+! grep -q "$PLACE2" enabled_functions
+
+cnt=`cat enabled_functions | wc -l`
+if [ $cnt -ne $((ocnt + 2)) ]; then
+ exit_fail
+fi
+
+# Cleanup Test 3
+echo 0 > events/test/list_exit/enable
+echo "-:test/list_exit" >> dynamic_events
+
+clear_trace
--
2.43.0
^ permalink raw reply related
* [PATCH v6 3/4] docs: tracing/fprobe: Document list filters and :entry/:exit
From: Seokwoo Chung (Ryan) @ 2026-02-05 13:58 UTC (permalink / raw)
To: mhiramat
Cc: rostedt, corbet, shuah, mathieu.desnoyers, linux-kernel,
linux-trace-kernel, linux-doc, linux-kselftest,
Seokwoo Chung (Ryan)
In-Reply-To: <20260205135842.20517-1-seokwoo.chung130@gmail.com>
Update fprobe event documentation to describe comma-separated symbol lists,
exclusions, and explicit suffixes.
Signed-off-by: Seokwoo Chung (Ryan) <seokwoo.chung130@gmail.com>
---
Documentation/trace/fprobetrace.rst | 17 ++++++++++++++---
1 file changed, 14 insertions(+), 3 deletions(-)
diff --git a/Documentation/trace/fprobetrace.rst b/Documentation/trace/fprobetrace.rst
index b4c2ca3d02c1..bbcfd57f0005 100644
--- a/Documentation/trace/fprobetrace.rst
+++ b/Documentation/trace/fprobetrace.rst
@@ -25,14 +25,18 @@ Synopsis of fprobe-events
-------------------------
::
- f[:[GRP1/][EVENT1]] SYM [FETCHARGS] : Probe on function entry
- f[MAXACTIVE][:[GRP1/][EVENT1]] SYM%return [FETCHARGS] : Probe on function exit
+ f[:[GRP1/][EVENT1]] SYM[%return] [FETCHARGS] : Single function
+ f[:[GRP1/][EVENT1]] SYM[,[!]SYM[,...]][:entry|:exit] [FETCHARGS] :Multiple
+ function
t[:[GRP2/][EVENT2]] TRACEPOINT [FETCHARGS] : Probe on tracepoint
GRP1 : Group name for fprobe. If omitted, use "fprobes" for it.
GRP2 : Group name for tprobe. If omitted, use "tracepoints" for it.
EVENT1 : Event name for fprobe. If omitted, the event name is
- "SYM__entry" or "SYM__exit".
+ - For a single literal symbol, the event name is
+ "SYM__entry" or "SYM__exit".
+ - For a *list or any wildcard*, an explicit [GRP1/][EVENT1] is
+ required; otherwise the parser rejects it.
EVENT2 : Event name for tprobe. If omitted, the event name is
the same as "TRACEPOINT", but if the "TRACEPOINT" starts
with a digit character, "_TRACEPOINT" is used.
@@ -40,6 +44,13 @@ Synopsis of fprobe-events
can be probed simultaneously, or 0 for the default value
as defined in Documentation/trace/fprobe.rst
+ SYM : Function name or comma-separated list of symbols.
+ - SYM prefixed with "!" are exclusions.
+ - ":entry" suffix means it probes entry of given symbols
+ (default)
+ - ":exit" suffix means it probes exit of given symbols.
+ - "%return" suffix means it probes exit of SYM (single
+ symbol).
FETCHARGS : Arguments. Each probe can have up to 128 args.
ARG : Fetch "ARG" function argument using BTF (only for function
entry or tracepoint.) (\*1)
--
2.43.0
^ permalink raw reply related
* [PATCH v6 2/4] fprobe: Support comma-separated filters in register_fprobe()
From: Seokwoo Chung (Ryan) @ 2026-02-05 13:58 UTC (permalink / raw)
To: mhiramat
Cc: rostedt, corbet, shuah, mathieu.desnoyers, linux-kernel,
linux-trace-kernel, linux-doc, linux-kselftest,
Seokwoo Chung (Ryan)
In-Reply-To: <20260205135842.20517-1-seokwoo.chung130@gmail.com>
register_fprobe() passes its filter and notfilter strings directly to
glob_match(), which only understands shell-style globs (*, ?, [...]).
Comma-separated symbol lists such as "vfs_read,vfs_open" never match
any symbol because no kernel symbol contains a comma.
Add glob_match_comma_list() that splits the filter on commas and
checks each entry individually with glob_match(). The existing
single-pattern fast path is preserved (no commas means the loop
executes exactly once).
This is required by the comma-separated fprobe list syntax introduced
in the preceding patch; without it, enabling a list-mode fprobe event
fails with "Could not enable event".
Signed-off-by: Seokwoo Chung (Ryan) <seokwoo.chung130@gmail.com>
---
kernel/trace/fprobe.c | 30 ++++++++++++++++++++++++++++--
1 file changed, 28 insertions(+), 2 deletions(-)
diff --git a/kernel/trace/fprobe.c b/kernel/trace/fprobe.c
index 1188eefef07c..2acd24b80d04 100644
--- a/kernel/trace/fprobe.c
+++ b/kernel/trace/fprobe.c
@@ -672,12 +672,38 @@ struct filter_match_data {
struct module **mods;
};
+/*
+ * Check if @name matches any comma-separated glob pattern in @list.
+ * If @list contains no commas, this is equivalent to glob_match().
+ */
+static bool glob_match_comma_list(const char *list, const char *name)
+{
+ const char *cur = list;
+
+ while (*cur) {
+ const char *sep = strchr(cur, ',');
+ int len = sep ? sep - cur : strlen(cur);
+ char pat[KSYM_NAME_LEN];
+
+ if (len > 0 && len < KSYM_NAME_LEN) {
+ memcpy(pat, cur, len);
+ pat[len] = '\0';
+ if (glob_match(pat, name))
+ return true;
+ }
+ if (!sep)
+ break;
+ cur = sep + 1;
+ }
+ return false;
+}
+
static int filter_match_callback(void *data, const char *name, unsigned long addr)
{
struct filter_match_data *match = data;
- if (!glob_match(match->filter, name) ||
- (match->notfilter && glob_match(match->notfilter, name)))
+ if (!glob_match_comma_list(match->filter, name) ||
+ (match->notfilter && glob_match_comma_list(match->notfilter, name)))
return 0;
if (!ftrace_location(addr))
--
2.43.0
^ permalink raw reply related
* [PATCH v6 1/4] tracing/fprobe: Support comma-separated symbols and :entry/:exit
From: Seokwoo Chung (Ryan) @ 2026-02-05 13:58 UTC (permalink / raw)
To: mhiramat
Cc: rostedt, corbet, shuah, mathieu.desnoyers, linux-kernel,
linux-trace-kernel, linux-doc, linux-kselftest,
Seokwoo Chung (Ryan)
In-Reply-To: <20260205135842.20517-1-seokwoo.chung130@gmail.com>
Extend the fprobe event interface to support:
- Comma-separated symbol lists: "func1,func2,func3"
- Exclusion prefix: "func1,!func2,func3"
- Explicit :entry and :exit suffixes (replacing %return for lists)
Single-symbol probes retain backward compatibility with %return.
The list parsing is factored into a dedicated parse_fprobe_list()
helper that splits comma-separated input into filter (included) and
nofilter (excluded) strings. Tracepoint validation now reports the
error position via trace_probe_log_err() so users can see what went
wrong in tracefs/error_log.
Changes since v5:
- Fix missing closing brace in the empty-token check that caused a
build error.
- Remove redundant strchr/strstr checks for tracepoint validation
(the character validation loop already rejects ',', ':', and '%').
- Add trace_probe_log_err() to the tracepoint character validation
loop per reviewer feedback.
- Remove unnecessary braces around single-statement if per kernel
coding style.
- Extract list parsing into parse_fprobe_list() per reviewer feedback
to keep parse_fprobe_spec() focused.
Update tracefs/README to reflect the new syntax.
Signed-off-by: Seokwoo Chung (Ryan) <seokwoo.chung130@gmail.com>
---
kernel/trace/trace.c | 3 +-
kernel/trace/trace_fprobe.c | 219 ++++++++++++++++++++++++++++--------
2 files changed, 174 insertions(+), 48 deletions(-)
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 8bd4ec08fb36..649a6e6021b4 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -5578,7 +5578,8 @@ static const char readme_msg[] =
"\t r[maxactive][:[<group>/][<event>]] <place> [<args>]\n"
#endif
#ifdef CONFIG_FPROBE_EVENTS
- "\t f[:[<group>/][<event>]] <func-name>[%return] [<args>]\n"
+ "\t f[:[<group>/][<event>]] <func-name>[:entry|:exit] [<args>]\n"
+ "\t (single symbols still accept %return)\n"
"\t t[:[<group>/][<event>]] <tracepoint> [<args>]\n"
#endif
#ifdef CONFIG_HIST_TRIGGERS
diff --git a/kernel/trace/trace_fprobe.c b/kernel/trace/trace_fprobe.c
index 262c0556e4af..f8846cd1d020 100644
--- a/kernel/trace/trace_fprobe.c
+++ b/kernel/trace/trace_fprobe.c
@@ -187,11 +187,14 @@ DEFINE_FREE(tuser_put, struct tracepoint_user *,
*/
struct trace_fprobe {
struct dyn_event devent;
+ char *filter;
struct fprobe fp;
+ bool list_mode;
+ char *nofilter;
const char *symbol;
+ struct trace_probe tp;
bool tprobe;
struct tracepoint_user *tuser;
- struct trace_probe tp;
};
static bool is_trace_fprobe(struct dyn_event *ev)
@@ -559,6 +562,8 @@ static void free_trace_fprobe(struct trace_fprobe *tf)
trace_probe_cleanup(&tf->tp);
if (tf->tuser)
tracepoint_user_put(tf->tuser);
+ kfree(tf->filter);
+ kfree(tf->nofilter);
kfree(tf->symbol);
kfree(tf);
}
@@ -838,7 +843,12 @@ static int __register_trace_fprobe(struct trace_fprobe *tf)
if (trace_fprobe_is_tracepoint(tf))
return __regsiter_tracepoint_fprobe(tf);
- /* TODO: handle filter, nofilter or symbol list */
+ /* Registration path:
+ * - list_mode: pass filter/nofilter
+ * - single: pass symbol only (legacy)
+ */
+ if (tf->list_mode)
+ return register_fprobe(&tf->fp, tf->filter, tf->nofilter);
return register_fprobe(&tf->fp, tf->symbol, NULL);
}
@@ -1154,60 +1164,131 @@ static struct notifier_block tprobe_event_module_nb = {
};
#endif /* CONFIG_MODULES */
-static int parse_symbol_and_return(int argc, const char *argv[],
- char **symbol, bool *is_return,
- bool is_tracepoint)
+static bool has_wildcard(const char *s)
{
- char *tmp = strchr(argv[1], '%');
- int i;
+ return s && (strchr(s, '*') || strchr(s, '?'));
+}
- if (tmp) {
- int len = tmp - argv[1];
+static int parse_fprobe_list(char *b, char **filter, char **nofilter)
+{
+ char *f __free(kfree) = NULL;
+ char *nf __free(kfree) = NULL;
+ char *tmp = b, *tok;
+ size_t sz;
- if (!is_tracepoint && !strcmp(tmp, "%return")) {
- *is_return = true;
- } else {
- trace_probe_log_err(len, BAD_ADDR_SUFFIX);
- return -EINVAL;
- }
- *symbol = kmemdup_nul(argv[1], len, GFP_KERNEL);
- } else
- *symbol = kstrdup(argv[1], GFP_KERNEL);
- if (!*symbol)
+ sz = strlen(b) + 1;
+
+ f = kzalloc(sz, GFP_KERNEL);
+ nf = kzalloc(sz, GFP_KERNEL);
+ if (!f || !nf)
return -ENOMEM;
- if (*is_return)
- return 0;
+ while ((tok = strsep(&tmp, ",")) != NULL) {
+ char *dst;
+ bool neg = (*tok == '!');
- if (is_tracepoint) {
- tmp = *symbol;
- while (*tmp && (isalnum(*tmp) || *tmp == '_'))
- tmp++;
- if (*tmp) {
- /* find a wrong character. */
- trace_probe_log_err(tmp - *symbol, BAD_TP_NAME);
- kfree(*symbol);
- *symbol = NULL;
+ if (*tok == '\0') {
+ trace_probe_log_err(tmp - b - 1, BAD_TP_NAME);
return -EINVAL;
}
+
+ if (neg)
+ tok++;
+ dst = neg ? nf : f;
+ if (dst[0] != '\0')
+ strcat(dst, ",");
+ strcat(dst, tok);
}
- /* If there is $retval, this should be a return fprobe. */
- for (i = 2; i < argc; i++) {
- tmp = strstr(argv[i], "$retval");
- if (tmp && !isalnum(tmp[7]) && tmp[7] != '_') {
- if (is_tracepoint) {
- trace_probe_log_set_index(i);
- trace_probe_log_err(tmp - argv[i], RETVAL_ON_PROBE);
- kfree(*symbol);
- *symbol = NULL;
+ *filter = no_free_ptr(f);
+ *nofilter = no_free_ptr(nf);
+
+ return 0;
+}
+
+static int parse_fprobe_spec(const char *in, bool is_tracepoint,
+ char **base, bool *is_return, bool *list_mode,
+ char **filter, char **nofilter)
+{
+ char *work __free(kfree) = NULL;
+ char *b __free(kfree) = NULL;
+ char *f __free(kfree) = NULL;
+ char *nf __free(kfree) = NULL;
+ bool legacy_ret = false;
+ bool list = false;
+ const char *p;
+ int ret = 0;
+
+ if (!in || !base || !is_return || !list_mode || !filter || !nofilter)
+ return -EINVAL;
+
+ *base = NULL; *filter = NULL; *nofilter = NULL;
+ *is_return = false; *list_mode = false;
+
+ if (is_tracepoint) {
+ for (p = in; *p; p++)
+ if (!isalnum(*p) && *p != '_') {
+ trace_probe_log_err(p - in, BAD_TP_NAME);
+ return -EINVAL;
+ }
+ b = kstrdup(in, GFP_KERNEL);
+ if (!b)
+ return -ENOMEM;
+ *base = no_free_ptr(b);
+ return 0;
+ }
+
+ work = kstrdup(in, GFP_KERNEL);
+ if (!work)
+ return -ENOMEM;
+
+ p = strstr(work, "%return");
+ if (p && p[7] == '\0') {
+ *is_return = true;
+ legacy_ret = true;
+ *(char *)p = '\0';
+ } else {
+ /*
+ * If "symbol:entry" or "symbol:exit" is given, it is new
+ * style probe.
+ */
+ p = strrchr(work, ':');
+ if (p) {
+ if (!strcmp(p, ":exit")) {
+ *is_return = true;
+ *(char *)p = '\0';
+ } else if (!strcmp(p, ":entry")) {
+ *(char *)p = '\0';
+ } else {
return -EINVAL;
}
- *is_return = true;
- break;
}
}
- return 0;
+
+ list = !!strchr(work, ',');
+
+ if (list && legacy_ret)
+ return -EINVAL;
+
+ if (legacy_ret)
+ *is_return = true;
+
+ b = kstrdup(work, GFP_KERNEL);
+ if (!b)
+ return -ENOMEM;
+
+ if (list) {
+ ret = parse_fprobe_list(b, &f, &nf);
+ if (ret)
+ return ret;
+ *list_mode = true;
+ }
+
+ *base = no_free_ptr(b);
+ *filter = no_free_ptr(f);
+ *nofilter = no_free_ptr(nf);
+
+ return ret;
}
static int trace_fprobe_create_internal(int argc, const char *argv[],
@@ -1241,6 +1322,8 @@ static int trace_fprobe_create_internal(int argc, const char *argv[],
const char *event = NULL, *group = FPROBE_EVENT_SYSTEM;
struct module *mod __free(module_put) = NULL;
const char **new_argv __free(kfree) = NULL;
+ char *parsed_nofilter __free(kfree) = NULL;
+ char *parsed_filter __free(kfree) = NULL;
char *symbol __free(kfree) = NULL;
char *ebuf __free(kfree) = NULL;
char *gbuf __free(kfree) = NULL;
@@ -1249,6 +1332,7 @@ static int trace_fprobe_create_internal(int argc, const char *argv[],
char *dbuf __free(kfree) = NULL;
int i, new_argc = 0, ret = 0;
bool is_tracepoint = false;
+ bool list_mode = false;
bool is_return = false;
if ((argv[0][0] != 'f' && argv[0][0] != 't') || argc < 2)
@@ -1270,11 +1354,26 @@ static int trace_fprobe_create_internal(int argc, const char *argv[],
trace_probe_log_set_index(1);
- /* a symbol(or tracepoint) must be specified */
- ret = parse_symbol_and_return(argc, argv, &symbol, &is_return, is_tracepoint);
+ /* Parse spec early (single vs list, suffix, base symbol) */
+ ret = parse_fprobe_spec(argv[1], is_tracepoint, &symbol, &is_return,
+ &list_mode, &parsed_filter, &parsed_nofilter);
if (ret < 0)
return -EINVAL;
+ for (i = 2; i < argc; i++) {
+ char *tmp = strstr(argv[i], "$retval");
+
+ if (tmp && !isalnum(tmp[7]) && tmp[7] != '_') {
+ if (is_tracepoint) {
+ trace_probe_log_set_index(i);
+ trace_probe_log_err(tmp - argv[i], RETVAL_ON_PROBE);
+ return -EINVAL;
+ }
+ is_return = true;
+ break;
+ }
+ }
+
trace_probe_log_set_index(0);
if (event) {
gbuf = kmalloc(MAX_EVENT_NAME_LEN, GFP_KERNEL);
@@ -1287,6 +1386,15 @@ static int trace_fprobe_create_internal(int argc, const char *argv[],
}
if (!event) {
+ /*
+ * Event name rules:
+ * - For list/wildcard: require explicit [GROUP/]EVENT
+ * - For single literal: autogenerate symbol__entry/symbol__exit
+ */
+ if (list_mode || has_wildcard(symbol)) {
+ trace_probe_log_err(0, NO_GROUP_NAME);
+ return -EINVAL;
+ }
ebuf = kmalloc(MAX_EVENT_NAME_LEN, GFP_KERNEL);
if (!ebuf)
return -ENOMEM;
@@ -1322,7 +1430,8 @@ static int trace_fprobe_create_internal(int argc, const char *argv[],
NULL, NULL, NULL, sbuf);
}
}
- if (!ctx->funcname)
+
+ if (!list_mode && !has_wildcard(symbol) && !is_tracepoint)
ctx->funcname = symbol;
abuf = kmalloc(MAX_BTF_ARGS_LEN, GFP_KERNEL);
@@ -1356,6 +1465,21 @@ static int trace_fprobe_create_internal(int argc, const char *argv[],
return ret;
}
+ /* carry list parsing result into tf */
+ if (!is_tracepoint) {
+ tf->list_mode = list_mode;
+ if (parsed_filter) {
+ tf->filter = kstrdup(parsed_filter, GFP_KERNEL);
+ if (!tf->filter)
+ return -ENOMEM;
+ }
+ if (parsed_nofilter) {
+ tf->nofilter = kstrdup(parsed_nofilter, GFP_KERNEL);
+ if (!tf->nofilter)
+ return -ENOMEM;
+ }
+ }
+
/* parse arguments */
for (i = 0; i < argc; i++) {
trace_probe_log_set_index(i + 2);
@@ -1442,8 +1566,9 @@ static int trace_fprobe_show(struct seq_file *m, struct dyn_event *ev)
seq_printf(m, ":%s/%s", trace_probe_group_name(&tf->tp),
trace_probe_name(&tf->tp));
- seq_printf(m, " %s%s", trace_fprobe_symbol(tf),
- trace_fprobe_is_return(tf) ? "%return" : "");
+ seq_printf(m, " %s", trace_fprobe_symbol(tf));
+ if (!trace_fprobe_is_tracepoint(tf) && trace_fprobe_is_return(tf))
+ seq_puts(m, ":exit");
for (i = 0; i < tf->tp.nr_args; i++)
seq_printf(m, " %s=%s", tf->tp.args[i].name, tf->tp.args[i].comm);
--
2.43.0
^ permalink raw reply related
* [PATCH v6 0/4] tracing/fprobe: Support comma-separated symbol lists and :entry/:exit suffixes
From: Seokwoo Chung (Ryan) @ 2026-02-05 13:58 UTC (permalink / raw)
To: mhiramat
Cc: rostedt, corbet, shuah, mathieu.desnoyers, linux-kernel,
linux-trace-kernel, linux-doc, linux-kselftest,
Seokwoo Chung (Ryan)
Extend the fprobe event interface to accept comma-separated symbol lists
with ! exclusion prefix, and :entry/:exit suffixes as an alternative to
%return. Single-symbol probes retain full backward compatibility with
%return.
Example usage:
f:mygroup/myevent vfs_read,!vfs_write,vfs_open:entry
f:mygroup/myexit vfs_read,vfs_open:exit
Changes since v5:
- Fix missing closing brace in the empty-token check that caused a
build error.
- Remove redundant strchr/strstr checks for tracepoint validation;
the character validation loop already rejects ',', ':', and '%'.
- Add trace_probe_log_err() to the tracepoint character validation
loop so users see what went wrong in tracefs/error_log (reviewer
feedback from Masami Hiramatsu).
- Remove unnecessary braces around single-statement if per kernel
coding style (reviewer feedback).
- Extract list parsing into parse_fprobe_list() to keep
parse_fprobe_spec() focused (reviewer feedback).
- New patch 2/4: add glob_match_comma_list() in kernel/trace/fprobe.c
so register_fprobe() correctly handles comma-separated filter
strings. Without this, enabling a list-mode fprobe event failed
with "Could not enable event" because glob_match() does not
understand commas.
- Reorder: documentation patch now comes after all code changes.
- Updated selftest commit message to note that existing tests
(add_remove_fprobe.tc, fprobe_syntax_errors.tc,
add_remove_fprobe_repeat.tc) report UNSUPPORTED because their
"requires" lines still check for the old %return syntax in README.
Their requires lines need updating in a follow-up patch.
Tested in QEMU/KVM but I am not too confident if I configured correctly and
would like to ask for further testing.
Seokwoo Chung (Ryan) (4):
tracing/fprobe: Support comma-separated symbols and :entry/:exit
fprobe: Support comma-separated filters in register_fprobe()
docs: tracing/fprobe: Document list filters and :entry/:exit
selftests/ftrace: Add accept cases for fprobe list syntax
Documentation/trace/fprobetrace.rst | 17 +-
kernel/trace/fprobe.c | 30 ++-
kernel/trace/trace.c | 3 +-
kernel/trace/trace_fprobe.c | 219 ++++++++++++++----
.../ftrace/test.d/dynevent/fprobe_list.tc | 92 ++++++++
5 files changed, 308 insertions(+), 53 deletions(-)
create mode 100644 tools/testing/selftests/ftrace/test.d/dynevent/fprobe_list.tc
--
2.43.0
^ permalink raw reply
* Re: [RFC bpf-next 06/12] bpf: Add bpf_trampoline_multi_attach/detach functions
From: Jiri Olsa @ 2026-02-05 13:45 UTC (permalink / raw)
To: Menglong Dong
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, bpf,
linux-trace-kernel, Martin KaFai Lau, Eduard Zingerman, Song Liu,
Yonghong Song, Menglong Dong, Steven Rostedt, Mark Rutland
In-Reply-To: <1945516.tdWV9SEqCh@7940hx>
On Thu, Feb 05, 2026 at 05:16:49PM +0800, Menglong Dong wrote:
> On 2026/2/3 17:38 Jiri Olsa <jolsa@kernel.org> write:
> > Adding bpf_trampoline_multi_attach/detach functions that allows
> > to attach/detach multi tracing trampoline.
> >
> > The attachment is defined with bpf_program and array of BTF ids
> > of functions to attach the bpf program to.
> >
> [...]
> > @@ -367,7 +367,11 @@ static struct bpf_trampoline *bpf_trampoline_lookup(u64 key, unsigned long ip)
> > head = &trampoline_ip_table[hash_64(tr->ip, TRAMPOLINE_HASH_BITS)];
> > hlist_add_head(&tr->hlist_ip, head);
> > refcount_set(&tr->refcnt, 1);
> > +#ifdef CONFIG_LOCKDEP
> > + mutex_init_with_key(&tr->mutex, &__lockdep_no_track__);
> > +#else
> > mutex_init(&tr->mutex);
> > +#endif
> > for (i = 0; i < BPF_TRAMP_MAX; i++)
> > INIT_HLIST_HEAD(&tr->progs_hlist[i]);
> > out:
> > @@ -1400,6 +1404,188 @@ int __weak arch_bpf_trampoline_size(const struct btf_func_model *m, u32 flags,
> > return -ENOTSUPP;
> > }
> >
> > +#if defined(CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS) && defined(CONFIG_HAVE_SINGLE_FTRACE_DIRECT_OPS)
>
> Hi, Jiri. It's great to see your tracing_multi link finally. It looks great ;)
heya, thanks ;-)
>
> After analyzing a little deeper on the SINGLE_FTRACE_DIRECT_OPS, I
> understand why it is only supported on x86_64 for now. It seems that
> it's a little hard to implement it in the other arch, as we need to
> restructure the implement of ftrace direct call.
>
> So do we need some more ftrace API here to make the tracing multi-link
> independent from SINGLE_FTRACE_DIRECT_OPS? Otherwise, we can only
> use it on x86_64.
I tried to describe it in commit [2] changelog:
At the moment we can enable this only on x86 arch, because arm relies
on ftrace_ops object representing just single trampoline image (stored
in ftrace_ops::direct_call). Archs that do not support this will continue
to use *_ftrace_direct api.
>
> Have you ever tried to implement the SINGLE_FTRACE_DIRECT_OPS on arm64?
> The direct call on arm64 is so complex, and I didn't work it out :/
yes, it seems to be difficult atm, Mark commented on that in [1],
I don't know arm that good to be of much help in here, cc-ing Mark
jirka
[1] https://lore.kernel.org/bpf/aIyNOd18TRLu8EpY@J2N7QTR9R3/
[2] 424f6a361096 ("bpf,x86: Use single ftrace_ops for direct calls")
>
> Thanks!
> Menglong Dong
>
> > +
> > +struct fentry_multi_data {
> > + struct ftrace_hash *unreg;
> > + struct ftrace_hash *modify;
> > + struct ftrace_hash *reg;
> > +};
> > +
> [...]
> >
> >
> >
>
>
>
>
^ permalink raw reply
* Re: [PATCH] uprobes: replace deprecated kmap_atomic() with kmap_local_page()
From: Oleg Nesterov @ 2026-02-05 12:29 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Carlos López, linux-trace-kernel, linux-perf-users,
Masami Hiramatsu, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
Ian Rogers, Adrian Hunter, James Clark, open list:UPROBES
In-Reply-To: <20260205111208.GK232055@noisy.programming.kicks-ass.net>
On 02/05, Peter Zijlstra wrote:
>
> On Thu, Feb 05, 2026 at 11:57:25AM +0100, Carlos López wrote:
> > kmap_atomic() has been deprecated for a few years, and kmap_local_page()
> > should be used instead, so replace it. Since the uprobes code relied on
> > kmap_atomic() disabling page faults to access user pages, add the
> > required calls to keep the existing behavior.
>
> Are those pagefault_disable()s really needed here, or is this just
> cargo-culting?
No, I think there are not needed.
And the (already in tip) patches from Keke do not add pagefault_disable().
is_trap_at_addr() does pagefault_disable() because it is called with
mm->mmap_lock held.
Oleg.
^ permalink raw reply
* Re: [PATCH v2 01/20] rv/rvgen: introduce AutomataError exception class
From: Gabriele Monaco @ 2026-02-05 12:08 UTC (permalink / raw)
To: Wander Lairson Costa
Cc: Steven Rostedt, Nam Cao, open list:RUNTIME VERIFICATION (RV),
open list
In-Reply-To: <20260204144914.104028-2-wander@redhat.com>
On Wed, 2026-02-04 at 11:42 -0300, Wander Lairson Costa wrote:
> Replace the generic except Exception block with a custom AutomataError
> class that inherits from Exception. This provides more precise exception
> handling for automata parsing and validation errors while avoiding
> overly broad exception catches that could mask programming errors like
> SyntaxError or TypeError.
>
> The AutomataError class is raised when DOT file processing fails due to
> invalid format, I/O errors, or malformed automaton definitions. The
> main entry point catches this specific exception and provides a
> user-friendly error message to stderr before exiting.
>
> Signed-off-by: Wander Lairson Costa <wander@redhat.com>
[...]
> ---
> @@ -45,8 +52,8 @@ class Automata:
> dot_lines = []
> try:
> dot_file = open(self.__dot_path)
> - except:
> - raise Exception("Cannot open the file: %s" % self.__dot_path)
> + except OSError as exc:
> + raise AutomataError(f"Cannot open the file: {self.__dot_path}")
> from exc
>
You probably don't want to mask all OSError in main.py and allow only this one.
I think it's alright to keep a simpler message for wrong model file (also LTL!),
while throwing a splat in other cases (e.g. missing templates).
But wouldn't it be better to show the error message of the OSError? 99% of the
time it's going to be ENOENT, but could also be some EPERM or who knows what.
I'm fine keeping AutomataError here but would at least propagate the error
message.
Or keep it simple and catch all OSError as well, this one included.
Thanks,
Gabriele
>
> dot_lines = dot_file.read().splitlines()
> dot_file.close()
> @@ -55,7 +62,7 @@ class Automata:
> line = dot_lines[cursor].split()
>
> if (line[0] != "digraph") and (line[1] != "state_automaton"):
> - raise Exception("Not a valid .dot format: %s" % self.__dot_path)
> + raise AutomataError("Not a valid .dot format: %s" %
> self.__dot_path)
> else:
> cursor += 1
> return dot_lines
> diff --git a/tools/verification/rvgen/rvgen/dot2c.py
> b/tools/verification/rvgen/rvgen/dot2c.py
> index 06a26bf15a7e9..74147ae2942f9 100644
> --- a/tools/verification/rvgen/rvgen/dot2c.py
> +++ b/tools/verification/rvgen/rvgen/dot2c.py
> @@ -13,7 +13,7 @@
> # For further information, see:
> # Documentation/trace/rv/deterministic_automata.rst
>
> -from .automata import Automata
> +from .automata import Automata, AutomataError
>
> class Dot2c(Automata):
> enum_suffix = ""
> @@ -71,7 +71,7 @@ class Dot2c(Automata):
> min_type = "unsigned int"
>
> if self.states.__len__() > 1000000:
> - raise Exception("Too many states: %d" % self.states.__len__())
> + raise AutomataError("Too many states: %d" %
> self.states.__len__())
>
> return min_type
>
> diff --git a/tools/verification/rvgen/rvgen/generator.py
> b/tools/verification/rvgen/rvgen/generator.py
> index 3441385c11770..a7bee6b1ea70c 100644
> --- a/tools/verification/rvgen/rvgen/generator.py
> +++ b/tools/verification/rvgen/rvgen/generator.py
> @@ -51,10 +51,7 @@ class RVGenerator:
> raise FileNotFoundError("Could not find the rv directory, do you have
> the kernel source installed?")
>
> def _read_file(self, path):
> - try:
> - fd = open(path, 'r')
> - except OSError:
> - raise Exception("Cannot open the file: %s" % path)
> + fd = open(path, 'r')
>
> content = fd.read()
>
> @@ -65,7 +62,7 @@ class RVGenerator:
> try:
> path = os.path.join(self.abs_template_dir, file)
> return self._read_file(path)
> - except Exception:
> + except OSError:
> # Specific template file not found. Try the generic template file
> in the template/
> # directory, which is one level up
> path = os.path.join(self.abs_template_dir, "..", file)
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox