* Re: [PATCH 14/26] rv/rvgen: remove redundant initial_state removal
From: Gabriele Monaco @ 2026-01-20 8:01 UTC (permalink / raw)
To: Wander Lairson Costa
Cc: Steven Rostedt, Nam Cao, open list,
open list:RUNTIME VERIFICATION (RV)
In-Reply-To: <20260119205601.105821-15-wander@redhat.com>
On Mon, 2026-01-19 at 17:45 -0300, Wander Lairson Costa wrote:
> Remove an unnecessary and incorrect list removal operation in the
> automata state variable processing. The code attempted to remove
> initial_state from the states list, but this element was never
> added to the list in the first place. States with the __init_
> prefix are explicitly excluded from the states list during the
> parsing loop, with only the initial_state variable being set
> from them.
The initial state is not the state with __init_, but the state pointed to by
that, the purpose of removing it after sorting and putting it back is for it to
be the first (we may argue there are better ways to do that, but it works).
After this change, the initial state is duplicated..
I think we should just drop this.
Thanks,
Gabriele
>
> Calling remove() on an element that does not exist in a list
> raises a ValueError. This code would have failed during execution
> when processing any DOT file containing an initial state marker.
> The subsequent insert operation at index 0 correctly adds the
> initial_state to the beginning of the states list, making the
> removal operation both incorrect and redundant.
>
> Signed-off-by: Wander Lairson Costa <wander@redhat.com>
> ---
> tools/verification/rvgen/rvgen/automata.py | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/tools/verification/rvgen/rvgen/automata.py
> b/tools/verification/rvgen/rvgen/automata.py
> index 7841a6e70bad2..b302af3e5133e 100644
> --- a/tools/verification/rvgen/rvgen/automata.py
> +++ b/tools/verification/rvgen/rvgen/automata.py
> @@ -111,7 +111,6 @@ class Automata:
> cursor += 1
>
> states = sorted(set(states))
> - states.remove(initial_state)
>
> # Insert the initial state at the beginning of the states
> states.insert(0, initial_state)
^ permalink raw reply
* Re: [PATCH 13/26] rv/rvgen: fix DOT file validation logic error
From: Gabriele Monaco @ 2026-01-20 7:56 UTC (permalink / raw)
To: Wander Lairson Costa
Cc: Steven Rostedt, Nam Cao, open list,
open list:RUNTIME VERIFICATION (RV)
In-Reply-To: <20260119205601.105821-14-wander@redhat.com>
On Mon, 2026-01-19 at 17:45 -0300, Wander Lairson Costa wrote:
> Fix incorrect boolean logic in automata DOT file format validation
> that allowed malformed files to pass undetected. The previous
> implementation used a logical AND operator where OR was required,
> causing the validation to only reject files when both the first
> token was not "digraph" AND the second token was not
> "state_automaton". This meant a file starting with "digraph" but
> having an incorrect second token would incorrectly pass validation.
>
> The corrected logic properly rejects DOT files where either the
> first token is not "digraph" or the second token is not
> "state_automaton", ensuring that only properly formatted automaton
> definition files are accepted for processing. Without this fix,
> invalid DOT files could cause downstream parsing failures or
> generate incorrect C code for runtime verification monitors.
>
> Signed-off-by: Wander Lairson Costa <wander@redhat.com>
Right, that slipped. Thanks!
Reviewed-by: Gabriele Monaco <gmonaco@redhat.com>
> ---
> tools/verification/rvgen/rvgen/automata.py | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/verification/rvgen/rvgen/automata.py
> b/tools/verification/rvgen/rvgen/automata.py
> index 9e1c097ad0e4a..7841a6e70bad2 100644
> --- a/tools/verification/rvgen/rvgen/automata.py
> +++ b/tools/verification/rvgen/rvgen/automata.py
> @@ -59,7 +59,7 @@ class Automata:
> # checking the first line:
> line = dot_lines[cursor].split()
>
> - if (line[0] != "digraph") and (line[1] != "state_automaton"):
> + if (line[0] != "digraph") or (line[1] != "state_automaton"):
> raise AutomataError(f"Not a valid .dot format:
> {self.__dot_path}")
> else:
> cursor += 1
^ permalink raw reply
* Re: [PATCH 12/26] rv/rvgen: fix PEP 8 whitespace violations
From: Gabriele Monaco @ 2026-01-20 7:53 UTC (permalink / raw)
To: Wander Lairson Costa
Cc: Steven Rostedt, Nam Cao, open list,
open list:RUNTIME VERIFICATION (RV)
In-Reply-To: <20260119205601.105821-13-wander@redhat.com>
On Mon, 2026-01-19 at 17:45 -0300, Wander Lairson Costa wrote:
> Fix whitespace violations throughout the rvgen codebase to comply
> with PEP 8 style guidelines. The changes address missing whitespace
> after commas, around operators, and in collection literals that
> were flagged by pycodestyle.
>
> The fixes include adding whitespace after commas in string replace
> chains and function arguments, adding whitespace around arithmetic
> operators, removing extra whitespace in list comprehensions, and
> fixing dictionary literal spacing. These changes improve code
> readability and consistency with Python coding standards.
>
> Signed-off-by: Wander Lairson Costa <wander@redhat.com>
Looks good, thanks
Reviewed-by: Gabriele Monaco <gmonaco@redhat.com>
> ---
> tools/verification/rvgen/rvgen/automata.py | 12 ++++++------
> tools/verification/rvgen/rvgen/dot2c.py | 2 +-
> tools/verification/rvgen/rvgen/dot2k.py | 4 ++--
> tools/verification/rvgen/rvgen/generator.py | 2 +-
> 4 files changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/tools/verification/rvgen/rvgen/automata.py
> b/tools/verification/rvgen/rvgen/automata.py
> index c0c8d13030007..9e1c097ad0e4a 100644
> --- a/tools/verification/rvgen/rvgen/automata.py
> +++ b/tools/verification/rvgen/rvgen/automata.py
> @@ -95,7 +95,7 @@ class Automata:
> raw_state = line[-1]
>
> # "enabled_fired"}; -> enabled_fired
> - state = raw_state.replace('"', '').replace('};',
> '').replace(',','_')
> + state = raw_state.replace('"', '').replace('};', '').replace(',',
> '_')
> if state[0:7] == "__init_":
> initial_state = state[7:]
> else:
> @@ -132,7 +132,7 @@ class Automata:
> # ------------ event is here ------------^^^^^
> if self.__dot_lines[cursor].split()[1] == "->":
> line = self.__dot_lines[cursor].split()
> - event = line[-2].replace('"','')
> + event = line[-2].replace('"', '')
>
> # when a transition has more than one labels, they are like
> this
> # "local_irq_enable\nhw_local_irq_enable_n"
> @@ -162,7 +162,7 @@ class Automata:
> nr_state += 1
>
> # declare the matrix....
> - matrix = [[ self.invalid_state_str for x in range(nr_event)] for y in
> range(nr_state)]
> + matrix = [[self.invalid_state_str for x in range(nr_event)] for y in
> range(nr_state)]
>
> # and we are back! Let's fill the matrix
> cursor = self.__get_cursor_begin_events()
> @@ -170,9 +170,9 @@ class Automata:
> while self.__dot_lines[cursor].lstrip()[0] == '"':
> if self.__dot_lines[cursor].split()[1] == "->":
> line = self.__dot_lines[cursor].split()
> - origin_state = line[0].replace('"','').replace(',','_')
> - dest_state = line[2].replace('"','').replace(',','_')
> - possible_events = line[-2].replace('"','').replace("\\n", "
> ")
> + origin_state = line[0].replace('"', '').replace(',', '_')
> + dest_state = line[2].replace('"', '').replace(',', '_')
> + possible_events = line[-2].replace('"', '').replace("\\n", "
> ")
> for event in possible_events.split():
> matrix[states_dict[origin_state]][events_dict[event]] =
> dest_state
> cursor += 1
> diff --git a/tools/verification/rvgen/rvgen/dot2c.py
> b/tools/verification/rvgen/rvgen/dot2c.py
> index fa9e9ae16640f..b291c29160fc2 100644
> --- a/tools/verification/rvgen/rvgen/dot2c.py
> +++ b/tools/verification/rvgen/rvgen/dot2c.py
> @@ -172,7 +172,7 @@ class Dot2c(Automata):
> line += f"\t\t\t{next_state}"
> else:
> line += f"{next_state:>{maxlen}}"
> - if y != nr_events-1:
> + if y != nr_events - 1:
> line += ",\n" if linetoolong else ", "
> else:
> line += "\n\t\t}," if linetoolong else " },"
> diff --git a/tools/verification/rvgen/rvgen/dot2k.py
> b/tools/verification/rvgen/rvgen/dot2k.py
> index 291385adb2c20..de44840f63eda 100644
> --- a/tools/verification/rvgen/rvgen/dot2k.py
> +++ b/tools/verification/rvgen/rvgen/dot2k.py
> @@ -109,8 +109,8 @@ class dot2k(Monitor, Dot2c):
> tp_args = tp_args_event if tp_type == "event" else tp_args_error
> if self.monitor_type == "per_task":
> 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])
> + 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(f" TP_PROTO({tp_proto_c}),")
> buff.append(f" TP_ARGS({tp_args_c})")
> return '\n'.join(buff)
> diff --git a/tools/verification/rvgen/rvgen/generator.py
> b/tools/verification/rvgen/rvgen/generator.py
> index ea1fa0f5d818d..0491f8c9cb0b9 100644
> --- a/tools/verification/rvgen/rvgen/generator.py
> +++ b/tools/verification/rvgen/rvgen/generator.py
> @@ -229,7 +229,7 @@ obj-$(CONFIG_RV_MON_{name_up}) += monitors/{name}/{name}.o
>
>
> class Monitor(RVGenerator):
> - monitor_types = { "global" : 1, "per_cpu" : 2, "per_task" : 3 }
> + monitor_types = {"global": 1, "per_cpu": 2, "per_task": 3}
>
> def __init__(self, extra_params={}):
> super().__init__(extra_params)
^ permalink raw reply
* Re: [PATCH 11/26] rv/rvgen: fix typo in generator module docstring
From: Gabriele Monaco @ 2026-01-20 7:51 UTC (permalink / raw)
To: Wander Lairson Costa
Cc: Steven Rostedt, Nam Cao, open list,
open list:RUNTIME VERIFICATION (RV)
In-Reply-To: <20260119205601.105821-12-wander@redhat.com>
On Mon, 2026-01-19 at 17:45 -0300, Wander Lairson Costa wrote:
> Fix typo in the module docstring: "Abtract" should be "Abstract".
>
> Signed-off-by: Wander Lairson Costa <wander@redhat.com>
> ---
> tools/verification/rvgen/rvgen/generator.py | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/verification/rvgen/rvgen/generator.py
> b/tools/verification/rvgen/rvgen/generator.py
> index fc9be5f6aaa1f..ea1fa0f5d818d 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
I believe you can fix typos together in the same patch (i.e. merge 10/26 and
11/26).
Reviewed-by: Gabriele Monaco <gmonaco@redhat.com>
to both.
Thanks,
Gabriele
^ permalink raw reply
* Re: [PATCH 08/26] rv/rvgen: simplify boolean comparison
From: Gabriele Monaco @ 2026-01-20 7:48 UTC (permalink / raw)
To: Wander Lairson Costa
Cc: Steven Rostedt, Nam Cao, open list,
open list:RUNTIME VERIFICATION (RV)
In-Reply-To: <20260119205601.105821-9-wander@redhat.com>
On Mon, 2026-01-19 at 17:45 -0300, Wander Lairson Costa wrote:
> Replace explicit boolean comparison with truthiness test in the dot2c
> module. The previous implementation used the redundant pattern of
> comparing a boolean variable directly to False, which is not idiomatic
> Python and adds unnecessary verbosity to the code.
>
> Python's truthiness allows for more concise and readable boolean
> checks. The expression "if not first" is clearer and more Pythonic
> than "if first == False" while maintaining identical semantics. This
> pattern is preferred in PEP 8 and is the standard approach in the
> Python community.
>
> This change continues the ongoing code quality improvements to align
> the codebase with modern Python best practices.
>
> Signed-off-by: Wander Lairson Costa <wander@redhat.com>
I'm starting to wonder if those simple cleanup patches with a tiny change and 3
paragraph of commit message aren't a bit too noisy. We may put at least the
simple ones together.
Other than that:
Reviewed-by: Gabriele Monaco <gmonaco@redhat.com>
Thanks,
Gabriele
> ---
> tools/verification/rvgen/rvgen/dot2c.py | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/verification/rvgen/rvgen/dot2c.py
> b/tools/verification/rvgen/rvgen/dot2c.py
> index c97bb9466af6d..fa9e9ae16640f 100644
> --- a/tools/verification/rvgen/rvgen/dot2c.py
> +++ b/tools/verification/rvgen/rvgen/dot2c.py
> @@ -202,7 +202,7 @@ class Dot2c(Automata):
> line = ""
> first = True
> for state in self.states:
> - if first == False:
> + if not first:
> line = line + ', '
> else:
> first = False
^ permalink raw reply
* Re: [PATCH 07/26] rv/rvgen: replace __contains__() with in operator
From: Gabriele Monaco @ 2026-01-20 7:45 UTC (permalink / raw)
To: Wander Lairson Costa, Steven Rostedt, Nam Cao, open list,
open list:RUNTIME VERIFICATION (RV)
In-Reply-To: <20260119205601.105821-8-wander@redhat.com>
On Mon, 2026-01-19 at 17:45 -0300, Wander Lairson Costa wrote:
> Replace the direct call to the __contains__() dunder method with the
> idiomatic in operator in the dot2c module. The previous implementation
> explicitly called the __contains__() method to check for membership in
> the final_states collection, which is not the recommended Python
> style.
>
> Python provides the in operator as the proper way to test membership,
> which internally calls the __contains__() method. Directly calling
> dunder methods bypasses Python's abstraction layer and reduces code
> readability. Using the in operator makes the code more natural and
> familiar to Python developers while maintaining identical functionality.
>
> Signed-off-by: Wander Lairson Costa <wander@redhat.com>
Neat, thanks!
Reviewed-by: Gabriele Monaco <gmonaco@redhat.com>
> ---
> tools/verification/rvgen/rvgen/dot2c.py | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/verification/rvgen/rvgen/dot2c.py
> b/tools/verification/rvgen/rvgen/dot2c.py
> index b9a2c009a9246..c97bb9466af6d 100644
> --- a/tools/verification/rvgen/rvgen/dot2c.py
> +++ b/tools/verification/rvgen/rvgen/dot2c.py
> @@ -207,7 +207,7 @@ class Dot2c(Automata):
> else:
> first = False
>
> - if self.final_states.__contains__(state):
> + if state in self.final_states:
> line = line + '1'
> else:
> line = line + '0'
^ permalink raw reply
* Re: [PATCH 06/26] rv/rvgen: use context managers for file operations
From: Gabriele Monaco @ 2026-01-20 7:44 UTC (permalink / raw)
To: Wander Lairson Costa, Steven Rostedt, Nam Cao, open list,
open list:RUNTIME VERIFICATION (RV)
In-Reply-To: <20260119205601.105821-7-wander@redhat.com>
On Mon, 2026-01-19 at 17:45 -0300, Wander Lairson Costa wrote:
> Replace manual file open and close operations with context managers
> throughout the rvgen codebase. The previous implementation used
> explicit open() and close() calls, which could lead to resource leaks
> if exceptions occurred between opening and closing the file handles.
>
> This change affects three file operations: reading DOT specification
> files in the automata parser, reading template files in the generator
> base class, and writing generated monitor files. All now use the with
> statement to ensure proper resource cleanup even in error conditions.
>
> Context managers provide automatic cleanup through the with statement,
> which guarantees that file handles are closed when the with block
> exits regardless of whether an exception occurred. This follows PEP
> 343 recommendations and is the standard Python idiom for resource
> management. The change also reduces code verbosity while improving
> safety and maintainability.
>
> Signed-off-by: Wander Lairson Costa <wander@redhat.com>
Looks reasonable, thanks!
Reviewed-by: Gabriele Monaco <gmonaco@redhat.com>
> ---
> tools/verification/rvgen/rvgen/automata.py | 6 ++----
> tools/verification/rvgen/rvgen/generator.py | 12 ++++--------
> 2 files changed, 6 insertions(+), 12 deletions(-)
>
> diff --git a/tools/verification/rvgen/rvgen/automata.py
> b/tools/verification/rvgen/rvgen/automata.py
> index ed02d0c69b410..70ff98abea751 100644
> --- a/tools/verification/rvgen/rvgen/automata.py
> +++ b/tools/verification/rvgen/rvgen/automata.py
> @@ -51,13 +51,11 @@ class Automata:
> cursor = 0
> dot_lines = []
> try:
> - dot_file = open(self.__dot_path)
> + with open(self.__dot_path) as dot_file:
> + dot_lines = dot_file.read().splitlines()
> 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()
> -
> # checking the first line:
> line = dot_lines[cursor].split()
>
> diff --git a/tools/verification/rvgen/rvgen/generator.py
> b/tools/verification/rvgen/rvgen/generator.py
> index 6d16fb68798a7..ee75e111feef1 100644
> --- a/tools/verification/rvgen/rvgen/generator.py
> +++ b/tools/verification/rvgen/rvgen/generator.py
> @@ -51,11 +51,8 @@ class RVGenerator:
> raise FileNotFoundError("Could not find the rv directory, do you have
> the kernel source installed?")
>
> def _read_file(self, path):
> - fd = open(path, 'r')
> -
> - content = fd.read()
> -
> - fd.close()
> + with open(path, 'r') as fd:
> + content = fd.read()
> return content
>
> def _read_template_file(self, file):
> @@ -199,9 +196,8 @@ obj-$(CONFIG_RV_MON_{name_up}) += monitors/{name}/{name}.o
> return
>
> def __write_file(self, file_name, content):
> - file = open(file_name, 'w')
> - file.write(content)
> - file.close()
> + with open(file_name, 'w') as file:
> + file.write(content)
>
> def _create_file(self, file_name, content):
> path = f"{self.name}/{file_name}"
^ permalink raw reply
* Re: [PATCH 05/26] rv/rvgen: remove unnecessary semicolons
From: Gabriele Monaco @ 2026-01-20 7:42 UTC (permalink / raw)
To: Wander Lairson Costa, Steven Rostedt, Nam Cao, open list,
open list:RUNTIME VERIFICATION (RV)
In-Reply-To: <20260119205601.105821-6-wander@redhat.com>
On Mon, 2026-01-19 at 17:45 -0300, Wander Lairson Costa wrote:
> Remove unnecessary semicolons from Python code in the rvgen tool.
> Python does not require semicolons to terminate statements, and
> their presence goes against PEP 8 style guidelines. These semicolons
> were likely added out of habit from C-style languages.
>
> The changes affect four instances across two files. In dot2c.py, one
> semicolon is removed from a boolean assignment. In dot2k.py, three
> semicolons are removed from string append operations that build
> generated C code. Note that the semicolons inside the string literals
> themselves are correctly preserved as they are part of the C code
> being generated, not Python syntax.
>
> This cleanup improves consistency with Python coding standards and
> aligns with the recent improvements to remove other Python
> anti-patterns from the codebase.
>
> Signed-off-by: Wander Lairson Costa <wander@redhat.com>
Muscle memory is hard to control. Thanks!
Reviewed-by: Gabriele Monaco <gmonaco@redhat.com>
> ---
> tools/verification/rvgen/rvgen/dot2c.py | 2 +-
> tools/verification/rvgen/rvgen/dot2k.py | 6 +++---
> 2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/tools/verification/rvgen/rvgen/dot2c.py
> b/tools/verification/rvgen/rvgen/dot2c.py
> index 0fb3617ad8ce9..b9a2c009a9246 100644
> --- a/tools/verification/rvgen/rvgen/dot2c.py
> +++ b/tools/verification/rvgen/rvgen/dot2c.py
> @@ -120,7 +120,7 @@ class Dot2c(Automata):
> for entry in buff:
> if first:
> string = string + "\t\t\"" + entry
> - first = False;
> + first = False
> else:
> string = string + "\",\n\t\t\"" + entry
> string = string + "\""
> diff --git a/tools/verification/rvgen/rvgen/dot2k.py
> b/tools/verification/rvgen/rvgen/dot2k.py
> index 1c0d0235bdf62..291385adb2c20 100644
> --- a/tools/verification/rvgen/rvgen/dot2k.py
> +++ b/tools/verification/rvgen/rvgen/dot2k.py
> @@ -37,10 +37,10 @@ class dot2k(Monitor, Dot2c):
> buff.append("\t/* XXX: validate that this event is only valid
> in the initial state */")
> 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(f"\tda_{handle}_{self.name}(p,
> {event}{self.enum_suffix});");
> + buff.append("\tstruct task_struct *p = /* XXX: how do I get
> p? */;")
> + buff.append(f"\tda_{handle}_{self.name}(p,
> {event}{self.enum_suffix});")
> else:
> -
> buff.append(f"\tda_{handle}_{self.name}({event}{self.enum_suffix});");
> +
> buff.append(f"\tda_{handle}_{self.name}({event}{self.enum_suffix});")
> buff.append("}")
> buff.append("")
> return '\n'.join(buff)
^ permalink raw reply
* Re: [PATCH 04/26] rv/rvgen: replace __len__() calls with len()
From: Gabriele Monaco @ 2026-01-20 7:41 UTC (permalink / raw)
To: Wander Lairson Costa
Cc: Steven Rostedt, Nam Cao, open list,
open list:RUNTIME VERIFICATION (RV)
In-Reply-To: <20260119205601.105821-5-wander@redhat.com>
On Mon, 2026-01-19 at 17:45 -0300, Wander Lairson Costa wrote:
> Replace all direct calls to the __len__() dunder method with the
> idiomatic len() built-in function across the rvgen codebase. This
> change eliminates a Python anti-pattern where dunder methods are
> called directly instead of using their corresponding built-in
> functions.
>
> The changes affect nine instances across two files. In automata.py,
> the empty string check is further improved by using truthiness
> testing instead of explicit length comparison. In dot2c.py, all
> length checks in the get_minimun_type, __get_max_strlen_of_states,
> and get_aut_init_function methods now use the standard len()
> function. Additionally, spacing around keyword arguments has been
> corrected to follow PEP 8 guidelines.
>
> Direct calls to dunder methods like __len__() are discouraged in
> Python because they bypass the language's abstraction layer and
> reduce code readability. Using len() provides the same functionality
> while adhering to Python community standards and making the code more
> familiar to Python developers.
>
> Signed-off-by: Wander Lairson Costa <wander@redhat.com>
> ---
Totally needed.
Reviewed-by: Gabriele Monaco <gmonaco@redhat.com>
Thanks,
Gabriele
> tools/verification/rvgen/rvgen/automata.py | 2 +-
> tools/verification/rvgen/rvgen/dot2c.py | 16 ++++++++--------
> 2 files changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/tools/verification/rvgen/rvgen/automata.py
> b/tools/verification/rvgen/rvgen/automata.py
> index 3e24313a2eaec..ed02d0c69b410 100644
> --- a/tools/verification/rvgen/rvgen/automata.py
> +++ b/tools/verification/rvgen/rvgen/automata.py
> @@ -42,7 +42,7 @@ class Automata:
> raise AutomataError(f"not a dot file: {self.__dot_path}")
>
> model_name = ntpath.splitext(basename)[0]
> - if model_name.__len__() == 0:
> + if not model_name:
> raise AutomataError(f"not a dot file: {self.__dot_path}")
>
> return model_name
> diff --git a/tools/verification/rvgen/rvgen/dot2c.py
> b/tools/verification/rvgen/rvgen/dot2c.py
> index 78959d345c26e..0fb3617ad8ce9 100644
> --- a/tools/verification/rvgen/rvgen/dot2c.py
> +++ b/tools/verification/rvgen/rvgen/dot2c.py
> @@ -86,14 +86,14 @@ class Dot2c(Automata):
> def get_minimun_type(self):
> min_type = "unsigned char"
>
> - if self.states.__len__() > 255:
> + if len(self.states) > 255:
> min_type = "unsigned short"
>
> - if self.states.__len__() > 65535:
> + if len(self.states) > 65535:
> min_type = "unsigned int"
>
> - if self.states.__len__() > 1000000:
> - raise AutomataError(f"Too many states: {self.states.__len__()}")
> + if len(self.states) > 1000000:
> + raise AutomataError(f"Too many states: {len(self.states)}")
>
> return min_type
>
> @@ -149,12 +149,12 @@ class Dot2c(Automata):
> return buff
>
> def __get_max_strlen_of_states(self):
> - max_state_name = max(self.states, key = len).__len__()
> - return max(max_state_name, self.invalid_state_str.__len__())
> + max_state_name = len(max(self.states, key=len))
> + return max(max_state_name, len(self.invalid_state_str))
>
> def get_aut_init_function(self):
> - nr_states = self.states.__len__()
> - nr_events = self.events.__len__()
> + nr_states = len(self.states)
> + nr_events = len(self.events)
> buff = []
>
> maxlen = self.__get_max_strlen_of_states() + len(self.enum_suffix)
^ permalink raw reply
* Re: [PATCH 01/26] rv/rvgen: introduce AutomataError exception class
From: Gabriele Monaco @ 2026-01-20 7:33 UTC (permalink / raw)
To: Wander Lairson Costa, Nam Cao
Cc: Steven Rostedt, open list, open list:RUNTIME VERIFICATION (RV)
In-Reply-To: <20260119205601.105821-2-wander@redhat.com>
On Mon, 2026-01-19 at 17:45 -0300, Wander Lairson Costa wrote:
> Replace generic Exception usage with a custom AutomataError class
> that inherits from OSError throughout the rvgen tool. This change
> 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 inherits from OSError rather than Exception
> because most error conditions involve file I/O operations such as
> reading DOT files or handling file access issues. This semantic
> alignment makes exception handling more specific and appropriate.
> The exception is raised when DOT file processing fails due to invalid
> format, I/O errors, or malformed automaton definitions.
>
> Additionally, remove the broad try-except block from __main__.py that
> was catching all exceptions. This allows Python's default exception
> handling to provide complete stack traces, making debugging
> significantly easier by showing exact error types and locations.
>
> Signed-off-by: Wander Lairson Costa <wander@redhat.com>
Thanks for the extensive series!
See my comments below.
Mind that I likely know python less than you do, so just call me out when I
start babbling.
> ---
> tools/verification/rvgen/__main__.py | 25 +++++++++------------
> tools/verification/rvgen/rvgen/automata.py | 17 +++++++++-----
> tools/verification/rvgen/rvgen/dot2c.py | 4 ++--
> tools/verification/rvgen/rvgen/generator.py | 7 ++----
> 4 files changed, 26 insertions(+), 27 deletions(-)
>
> diff --git a/tools/verification/rvgen/__main__.py
> b/tools/verification/rvgen/__main__.py
> index fa6fc1f4de2f7..768b11a1e978b 100644
> --- a/tools/verification/rvgen/__main__.py
> +++ b/tools/verification/rvgen/__main__.py
> @@ -39,22 +39,17 @@ if __name__ == '__main__':
>
> params = parser.parse_args()
>
> - try:
> - if params.subcmd == "monitor":
> - print("Opening and parsing the specification file %s" %
> params.spec)
> - if params.monitor_class == "da":
> - monitor = dot2k(params.spec, params.monitor_type,
> vars(params))
> - elif params.monitor_class == "ltl":
> - monitor = ltl2k(params.spec, params.monitor_type,
> vars(params))
> - else:
> - print("Unknown monitor class:", params.monitor_class)
> - sys.exit(1)
> + if params.subcmd == "monitor":
> + print("Opening and parsing the specification file %s" % params.spec)
> + if params.monitor_class == "da":
> + monitor = dot2k(params.spec, params.monitor_type, vars(params))
> + elif params.monitor_class == "ltl":
> + monitor = ltl2k(params.spec, params.monitor_type, vars(params))
> else:
> - monitor = Container(vars(params))
> - except Exception as e:
> - print('Error: '+ str(e))
> - print("Sorry : :-(")
> - sys.exit(1)
> + print("Unknown monitor class:", params.monitor_class)
> + sys.exit(1)
> + else:
> + monitor = Container(vars(params))
>
I agree catching all exceptions like this is quite detrimental while debugging,
but I see the original intent.
When you run commands written in python, you normally don't expect them to blurt
a stack trace when doing relatively normal things, like opening a wrong file.
Sure that might be useful when debugging, but for a user-facing tool we want to
write a meaningful error message and gracefully fail.
Other story is when the exception is something unexpected (that's why leaving a
generic Exception here is bad).
> print("Writing the monitor into the directory %s" % monitor.name)
> monitor.print_files()
> diff --git a/tools/verification/rvgen/rvgen/automata.py
> b/tools/verification/rvgen/rvgen/automata.py
> index d9a3fe2b74bf2..8d88c3b65d00d 100644
> --- a/tools/verification/rvgen/rvgen/automata.py
> +++ b/tools/verification/rvgen/rvgen/automata.py
> @@ -10,6 +10,13 @@
>
> import ntpath
>
> +class AutomataError(OSError):
> + """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.
> + """
> +
I'm not quite familiar with modern python best practices (so again, take my
comments with a grain of salt ;) ), but what is the advantage of using this
custom exception instead of using pre-existing specific exception types?
Although the difference is minimal, here you're throwing an OSError for
something that quite isn't (e.g. wrong format for the dot file).
A ValueError feels more appropriate to me in most of the instances here.
All in all, I would do something like:
* throw a ValueError (or a custom one based on that) whenever we expect wrong
data not dependent on OS features
* throw OSError whenever that was the exception, perhaps changing the message to
something more meaningful to us (like you're already doing here)
* intercept only those errors in main.py and print the message without stack
trace (if the message is clear enough we shouldn't need it).
Does it make sense to you?
Thanks,
Gabriele
> 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 b9b6f14cc536a..1a1770e7f20c0 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 = ""
> @@ -93,7 +93,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
* [PATCH bpf-next v4 2/2] selftests/bpf: test bpf_get_func_arg() for tp_btf
From: Menglong Dong @ 2026-01-20 7:30 UTC (permalink / raw)
To: ast, andrii, yonghong.song
Cc: daniel, john.fastabend, martin.lau, eddyz87, song, kpsingh, sdf,
haoluo, jolsa, mattbobrowski, rostedt, mhiramat,
mathieu.desnoyers, bpf, linux-kernel, linux-trace-kernel
In-Reply-To: <20260120073046.324342-1-dongml2@chinatelecom.cn>
Test bpf_get_func_arg() and bpf_get_func_arg_cnt() for tp_btf. The code
is most copied from test1 and test2.
Signed-off-by: Menglong Dong <dongml2@chinatelecom.cn>
---
.../bpf/prog_tests/get_func_args_test.c | 3 ++
.../selftests/bpf/progs/get_func_args_test.c | 44 +++++++++++++++++++
.../bpf/test_kmods/bpf_testmod-events.h | 10 +++++
.../selftests/bpf/test_kmods/bpf_testmod.c | 4 ++
4 files changed, 61 insertions(+)
diff --git a/tools/testing/selftests/bpf/prog_tests/get_func_args_test.c b/tools/testing/selftests/bpf/prog_tests/get_func_args_test.c
index 64a9c95d4acf..fadee95d3ae8 100644
--- a/tools/testing/selftests/bpf/prog_tests/get_func_args_test.c
+++ b/tools/testing/selftests/bpf/prog_tests/get_func_args_test.c
@@ -33,11 +33,14 @@ void test_get_func_args_test(void)
ASSERT_EQ(topts.retval >> 16, 1, "test_run");
ASSERT_EQ(topts.retval & 0xffff, 1234 + 29, "test_run");
+ ASSERT_OK(trigger_module_test_read(1), "trigger_read");
ASSERT_EQ(skel->bss->test1_result, 1, "test1_result");
ASSERT_EQ(skel->bss->test2_result, 1, "test2_result");
ASSERT_EQ(skel->bss->test3_result, 1, "test3_result");
ASSERT_EQ(skel->bss->test4_result, 1, "test4_result");
+ ASSERT_EQ(skel->bss->test5_result, 1, "test5_result");
+ ASSERT_EQ(skel->bss->test6_result, 1, "test6_result");
cleanup:
get_func_args_test__destroy(skel);
diff --git a/tools/testing/selftests/bpf/progs/get_func_args_test.c b/tools/testing/selftests/bpf/progs/get_func_args_test.c
index e0f34a55e697..5b7233afef05 100644
--- a/tools/testing/selftests/bpf/progs/get_func_args_test.c
+++ b/tools/testing/selftests/bpf/progs/get_func_args_test.c
@@ -121,3 +121,47 @@ int BPF_PROG(fexit_test, int _a, int *_b, int _ret)
test4_result &= err == 0 && ret == 1234;
return 0;
}
+
+__u64 test5_result = 0;
+SEC("tp_btf/bpf_testmod_fentry_test1_tp")
+int BPF_PROG(tp_test1)
+{
+ __u64 cnt = bpf_get_func_arg_cnt(ctx);
+ __u64 a = 0, z = 0;
+ __s64 err;
+
+ test5_result = cnt == 1;
+
+ err = bpf_get_func_arg(ctx, 0, &a);
+ test5_result &= err == 0 && ((int) a == 1);
+
+ /* not valid argument */
+ err = bpf_get_func_arg(ctx, 1, &z);
+ test5_result &= err == -EINVAL;
+
+ return 0;
+}
+
+__u64 test6_result = 0;
+SEC("tp_btf/bpf_testmod_fentry_test2_tp")
+int BPF_PROG(tp_test2)
+{
+ __u64 cnt = bpf_get_func_arg_cnt(ctx);
+ __u64 a = 0, b = 0, z = 0;
+ __s64 err;
+
+ test6_result = cnt == 2;
+
+ /* valid arguments */
+ err = bpf_get_func_arg(ctx, 0, &a);
+ test6_result &= err == 0 && (int) a == 2;
+
+ err = bpf_get_func_arg(ctx, 1, &b);
+ test6_result &= err == 0 && b == 3;
+
+ /* not valid argument */
+ err = bpf_get_func_arg(ctx, 2, &z);
+ test6_result &= err == -EINVAL;
+
+ return 0;
+}
diff --git a/tools/testing/selftests/bpf/test_kmods/bpf_testmod-events.h b/tools/testing/selftests/bpf/test_kmods/bpf_testmod-events.h
index aeef86b3da74..45a5e41f3a92 100644
--- a/tools/testing/selftests/bpf/test_kmods/bpf_testmod-events.h
+++ b/tools/testing/selftests/bpf/test_kmods/bpf_testmod-events.h
@@ -63,6 +63,16 @@ BPF_TESTMOD_DECLARE_TRACE(bpf_testmod_test_writable_bare,
sizeof(struct bpf_testmod_test_writable_ctx)
);
+DECLARE_TRACE(bpf_testmod_fentry_test1,
+ TP_PROTO(int a),
+ TP_ARGS(a)
+);
+
+DECLARE_TRACE(bpf_testmod_fentry_test2,
+ TP_PROTO(int a, u64 b),
+ TP_ARGS(a, b)
+);
+
#endif /* _BPF_TESTMOD_EVENTS_H */
#undef TRACE_INCLUDE_PATH
diff --git a/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c b/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c
index bc07ce9d5477..f3698746f033 100644
--- a/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c
+++ b/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c
@@ -396,11 +396,15 @@ __weak noinline struct file *bpf_testmod_return_ptr(int arg)
noinline int bpf_testmod_fentry_test1(int a)
{
+ trace_bpf_testmod_fentry_test1_tp(a);
+
return a + 1;
}
noinline int bpf_testmod_fentry_test2(int a, u64 b)
{
+ trace_bpf_testmod_fentry_test2_tp(a, b);
+
return a + b;
}
--
2.52.0
^ permalink raw reply related
* [PATCH bpf-next v4 1/2] bpf: support bpf_get_func_arg() for BPF_TRACE_RAW_TP
From: Menglong Dong @ 2026-01-20 7:30 UTC (permalink / raw)
To: ast, andrii, yonghong.song
Cc: daniel, john.fastabend, martin.lau, eddyz87, song, kpsingh, sdf,
haoluo, jolsa, mattbobrowski, rostedt, mhiramat,
mathieu.desnoyers, bpf, linux-kernel, linux-trace-kernel
In-Reply-To: <20260120073046.324342-1-dongml2@chinatelecom.cn>
For now, bpf_get_func_arg() and bpf_get_func_arg_cnt() is not supported by
the BPF_TRACE_RAW_TP, which is not convenient to get the argument of the
tracepoint, especially for the case that the position of the arguments in
a tracepoint can change.
The target tracepoint BTF type id is specified during loading time,
therefore we can get the function argument count from the function
prototype instead of the stack.
Signed-off-by: Menglong Dong <dongml2@chinatelecom.cn>
---
v4:
- fix the error of using bpf_get_func_arg() for BPF_TRACE_ITER
v3:
- remove unnecessary NULL checking for prog->aux->attach_func_proto
v2:
- for nr_args, skip first 'void *__data' argument in btf_trace_##name
typedef
---
kernel/bpf/verifier.c | 32 ++++++++++++++++++++++++++++----
kernel/trace/bpf_trace.c | 4 ++++
2 files changed, 32 insertions(+), 4 deletions(-)
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 9de0ec0c3ed9..0b281b7c41eb 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -23323,8 +23323,20 @@ static int do_misc_fixups(struct bpf_verifier_env *env)
/* Implement bpf_get_func_arg inline. */
if (prog_type == BPF_PROG_TYPE_TRACING &&
insn->imm == BPF_FUNC_get_func_arg) {
- /* Load nr_args from ctx - 8 */
- insn_buf[0] = BPF_LDX_MEM(BPF_DW, BPF_REG_0, BPF_REG_1, -8);
+ if (eatype == BPF_TRACE_RAW_TP) {
+ int nr_args = btf_type_vlen(prog->aux->attach_func_proto);
+
+ /*
+ * skip first 'void *__data' argument in btf_trace_##name
+ * typedef
+ */
+ nr_args--;
+ /* Save nr_args to reg0 */
+ insn_buf[0] = BPF_MOV64_IMM(BPF_REG_0, nr_args);
+ } else {
+ /* Load nr_args from ctx - 8 */
+ insn_buf[0] = BPF_LDX_MEM(BPF_DW, BPF_REG_0, BPF_REG_1, -8);
+ }
insn_buf[1] = BPF_JMP32_REG(BPF_JGE, BPF_REG_2, BPF_REG_0, 6);
insn_buf[2] = BPF_ALU64_IMM(BPF_LSH, BPF_REG_2, 3);
insn_buf[3] = BPF_ALU64_REG(BPF_ADD, BPF_REG_2, BPF_REG_1);
@@ -23376,8 +23388,20 @@ static int do_misc_fixups(struct bpf_verifier_env *env)
/* Implement get_func_arg_cnt inline. */
if (prog_type == BPF_PROG_TYPE_TRACING &&
insn->imm == BPF_FUNC_get_func_arg_cnt) {
- /* Load nr_args from ctx - 8 */
- insn_buf[0] = BPF_LDX_MEM(BPF_DW, BPF_REG_0, BPF_REG_1, -8);
+ if (eatype == BPF_TRACE_RAW_TP) {
+ int nr_args = btf_type_vlen(prog->aux->attach_func_proto);
+
+ /*
+ * skip first 'void *__data' argument in btf_trace_##name
+ * typedef
+ */
+ nr_args--;
+ /* Save nr_args to reg0 */
+ insn_buf[0] = BPF_MOV64_IMM(BPF_REG_0, nr_args);
+ } else {
+ /* Load nr_args from ctx - 8 */
+ insn_buf[0] = BPF_LDX_MEM(BPF_DW, BPF_REG_0, BPF_REG_1, -8);
+ }
new_prog = bpf_patch_insn_data(env, i + delta, insn_buf, 1);
if (!new_prog)
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index f73e08c223b5..0efdad3adcce 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -1734,10 +1734,14 @@ tracing_prog_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
case BPF_FUNC_d_path:
return &bpf_d_path_proto;
case BPF_FUNC_get_func_arg:
+ if (prog->expected_attach_type == BPF_TRACE_RAW_TP)
+ return &bpf_get_func_arg_proto;
return bpf_prog_has_trampoline(prog) ? &bpf_get_func_arg_proto : NULL;
case BPF_FUNC_get_func_ret:
return bpf_prog_has_trampoline(prog) ? &bpf_get_func_ret_proto : NULL;
case BPF_FUNC_get_func_arg_cnt:
+ if (prog->expected_attach_type == BPF_TRACE_RAW_TP)
+ return &bpf_get_func_arg_cnt_proto;
return bpf_prog_has_trampoline(prog) ? &bpf_get_func_arg_cnt_proto : NULL;
case BPF_FUNC_get_attach_cookie:
if (prog->type == BPF_PROG_TYPE_TRACING &&
--
2.52.0
^ permalink raw reply related
* [PATCH bpf-next v4 0/2] bpf: support bpf_get_func_arg() for BPF_TRACE_RAW_TP
From: Menglong Dong @ 2026-01-20 7:30 UTC (permalink / raw)
To: ast, andrii, yonghong.song
Cc: daniel, john.fastabend, martin.lau, eddyz87, song, kpsingh, sdf,
haoluo, jolsa, mattbobrowski, rostedt, mhiramat,
mathieu.desnoyers, bpf, linux-kernel, linux-trace-kernel
Support bpf_get_func_arg() for BPF_TRACE_RAW_TP by getting the function
argument count from "prog->aux->attach_func_proto" during verifier inline.
Changes v4 -> v3:
* fix the error of using bpf_get_func_arg() for BPF_TRACE_ITER
* https://lore.kernel.org/bpf/20260119023732.130642-1-dongml2@chinatelecom.cn/
Changes v3 -> v2:
* remove unnecessary NULL checking for prog->aux->attach_func_proto
* v2: https://lore.kernel.org/bpf/20260116071739.121182-1-dongml2@chinatelecom.cn/
Changes v2 -> v1:
* for nr_args, skip first 'void *__data' argument in btf_trace_##name
typedef
* check the result4 and result5 in the selftests
* v1: https://lore.kernel.org/bpf/20260116035024.98214-1-dongml2@chinatelecom.cn/
Menglong Dong (2):
bpf: support bpf_get_func_arg() for BPF_TRACE_RAW_TP
selftests/bpf: test bpf_get_func_arg() for tp_btf
kernel/bpf/verifier.c | 32 ++++++++++++--
kernel/trace/bpf_trace.c | 4 ++
.../bpf/prog_tests/get_func_args_test.c | 3 ++
.../selftests/bpf/progs/get_func_args_test.c | 44 +++++++++++++++++++
.../bpf/test_kmods/bpf_testmod-events.h | 10 +++++
.../selftests/bpf/test_kmods/bpf_testmod.c | 4 ++
6 files changed, 93 insertions(+), 4 deletions(-)
--
2.52.0
^ permalink raw reply
* Re: [PATCH 00/26] rv/rvgen: Robustness, modernization, and fixes
From: Nam Cao @ 2026-01-20 7:20 UTC (permalink / raw)
To: Wander Lairson Costa, Steven Rostedt, Gabriele Monaco,
Wander Lairson Costa, open list,
open list:RUNTIME VERIFICATION (RV)
In-Reply-To: <20260119205601.105821-1-wander@redhat.com>
Wander Lairson Costa <wander@redhat.com> writes:
> This patch series introduces a number of fixes and improvements to the
> RV Generator (rvgen) tool in tools/verification. The primary goal is to
> enhance the tool's robustness, maintainability, and correctness by
> addressing several latent bugs, modernizing the Python codebase, and
> improving its overall structure and error handling.
Great, someone who knows python is fixing our mess.
I had a brief look and they look reasonable. I will do an actual review
when I have time for it. Thanks so much for this work.
Nam
^ permalink raw reply
* Re: [PATCH v3 1/1] kprobes: retry blocked optprobe in do_free_cleaned_kprobes
From: Masami Hiramatsu @ 2026-01-20 5:50 UTC (permalink / raw)
To: hongao
Cc: naveen, anil.s.keshavamurthy, davem, linux-kernel,
linux-trace-kernel
In-Reply-To: <349359900266B25F+20260115023804.3951960-2-hongao@uniontech.com>
On Thu, 15 Jan 2026 10:38:03 +0800
hongao <hongao@uniontech.com> wrote:
> Once the aggrprobe is fully reverted in do_free_cleaned_kprobes(), retry
> optimize_kprobe() on that sibling so it can return to OPTIMIZED.
>
> Also remove the stale comment in __disarm_kprobe().
>
Thanks, let me pick it.
> Signed-off-by: hongao <hongao@uniontech.com>
> ---
> kernel/kprobes.c | 18 ++++++++++++------
> 1 file changed, 12 insertions(+), 6 deletions(-)
>
> diff --git a/kernel/kprobes.c b/kernel/kprobes.c
> index ab8f9fc1f0d1..1bd84d3b4817 100644
> --- a/kernel/kprobes.c
> +++ b/kernel/kprobes.c
> @@ -516,6 +516,7 @@ static LIST_HEAD(freeing_list);
>
> static void kprobe_optimizer(struct work_struct *work);
> static DECLARE_DELAYED_WORK(optimizing_work, kprobe_optimizer);
> +static void optimize_kprobe(struct kprobe *p);
> #define OPTIMIZE_DELAY 5
>
> /*
> @@ -593,6 +594,17 @@ static void do_free_cleaned_kprobes(void)
> */
> continue;
> }
> +
> + /*
> + * The aggregator was holding back another probe while it sat on the
> + * unoptimizing/freeing lists. Now that the aggregator has been fully
> + * reverted we can safely retry the optimization of that sibling.
> + */
> +
> + struct kprobe *_p = get_optimized_kprobe(op->kp.addr);
> + if (unlikely(_p))
> + optimize_kprobe(_p);
> +
> free_aggr_kprobe(&op->kp);
> }
> }
> @@ -1002,12 +1014,6 @@ static void __disarm_kprobe(struct kprobe *p, bool reopt)
> if (unlikely(_p) && reopt)
> optimize_kprobe(_p);
> }
> - /*
> - * TODO: Since unoptimization and real disarming will be done by
> - * the worker thread, we can not check whether another probe are
> - * unoptimized because of this probe here. It should be re-optimized
> - * by the worker thread.
> - */
> }
>
> #else /* !CONFIG_OPTPROBES */
> --
> 2.50.1
>
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
^ permalink raw reply
* Re: [PATCH bpf RESEND v2 1/2] bpf: Fix memory access flags in helper prototypes
From: Zesen Liu @ 2026-01-20 3:39 UTC (permalink / raw)
To: Eduard Zingerman
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa, Matt Bobrowski,
Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Simon Horman, Daniel Xu, bpf, linux-kernel, linux-trace-kernel,
netdev, Shuran Liu, Peili Gao, Haoran Ni
In-Reply-To: <55f01664fc714615206cc8d100cabf4f310f2302.camel@gmail.com>
[...]
> On Jan 20, 2026, at 04:24, Eduard Zingerman <eddyz87@gmail.com> wrote:
>
> Q: why ARG_PTR_TO_UNINIT_MEM here, but not for a previous function and
> not for snprintf variants?
For bpf_get_stack_proto_raw_tp, I chose ARG_PTR_TO_UNINIT_MEM to be consistent with its siblings:
• bpf_get_stack_proto_tp (bpf_trace.c:1425)
• bpf_get_stack_proto (stackmap.c:525)
• bpf_get_stack_sleepable_proto (stackmap.c:541)
All of these are wrappers around the same core function bpf_get_stack() / __bpf_get_stack(), passing the buffer with identical semantics, and all use ARG_PTR_TO_UNINIT_MEM.
^ permalink raw reply
* Re: [PATCH bpf-next v3 1/2] bpf: support bpf_get_func_arg() for BPF_TRACE_RAW_TP
From: Menglong Dong @ 2026-01-20 3:37 UTC (permalink / raw)
To: menglong8.dong, andrii, ast, bot+bpf-ci
Cc: daniel, john.fastabend, martin.lau, eddyz87, song, yonghong.song,
kpsingh, sdf, haoluo, jolsa, mattbobrowski, rostedt, mhiramat,
mathieu.desnoyers, bpf, linux-kernel, linux-trace-kernel, ast,
andrii, daniel, martin.lau, eddyz87, yonghong.song, clm,
ihor.solodrai
In-Reply-To: <c83188087c6d6409524b80b2078285813ccfea8e42ea82e88542e6e8d5e5f73d@mail.kernel.org>
On 2026/1/20 02:44, bot+bpf-ci@kernel.org wrote:
> > diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> > index f73e08c223b5..64976009ba3c 100644
> > --- a/kernel/trace/bpf_trace.c
> > +++ b/kernel/trace/bpf_trace.c
> > @@ -1734,11 +1734,11 @@ tracing_prog_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
> > case BPF_FUNC_d_path:
> > return &bpf_d_path_proto;
> > case BPF_FUNC_get_func_arg:
> > - return bpf_prog_has_trampoline(prog) ? &bpf_get_func_arg_proto : NULL;
> > + return &bpf_get_func_arg_proto;
>
> Removing the bpf_prog_has_trampoline() check here allows all TRACING
> program types to use bpf_get_func_arg(), not just RAW_TP and trampoline-
> based types. In particular, BPF_TRACE_ITER programs can now use this
> helper.
>
> However, looking at do_misc_fixups() in verifier.c, the inline code
> generation only handles BPF_TRACE_RAW_TP specially. For other non-
> trampoline types like BPF_TRACE_ITER, the else branch generates:
>
> BPF_LDX_MEM(BPF_DW, BPF_REG_0, BPF_REG_1, -8)
>
> This loads nr_args from ctx-8, but ITER programs use a different context
> layout (e.g. struct bpf_iter__bpf_map_elem with meta, map, key, value
> pointers). There is no nr_args at ctx-8 for ITER programs.
>
> Would this cause ITER programs using bpf_get_func_arg() to read arbitrary
> memory at ctx-8, potentially leaking kernel data or causing incorrect
> bounds checks for subsequent argument access?
You are right. As Yonghong said, I missed the bpf_iter case, and it should
be skipped.
I'll fix it in the next version.
>
> > case BPF_FUNC_get_func_ret:
> > return bpf_prog_has_trampoline(prog) ? &bpf_get_func_ret_proto : NULL;
> > case BPF_FUNC_get_func_arg_cnt:
> > - return bpf_prog_has_trampoline(prog) ? &bpf_get_func_arg_cnt_proto : NULL;
> > + return &bpf_get_func_arg_cnt_proto;
>
> Same question applies here for bpf_get_func_arg_cnt() with ITER programs.
>
> > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> > index 9de0ec0c3ed9..0b281b7c41eb 100644
> > --- a/kernel/bpf/verifier.c
> > +++ b/kernel/bpf/verifier.c
> > @@ -23323,8 +23323,20 @@ static int do_misc_fixups(struct bpf_verifier_env *env)
> > /* Implement bpf_get_func_arg inline. */
> > if (prog_type == BPF_PROG_TYPE_TRACING &&
> > insn->imm == BPF_FUNC_get_func_arg) {
> > + if (eatype == BPF_TRACE_RAW_TP) {
> > + int nr_args = btf_type_vlen(prog->aux->attach_func_proto);
> > +
> > + /*
> > + * skip first 'void *__data' argument in btf_trace_##name
> > + * typedef
> > + */
> > + nr_args--;
> > + /* Save nr_args to reg0 */
> > + insn_buf[0] = BPF_MOV64_IMM(BPF_REG_0, nr_args);
> > + } else {
> > + /* Load nr_args from ctx - 8 */
> > + insn_buf[0] = BPF_LDX_MEM(BPF_DW, BPF_REG_0, BPF_REG_1, -8);
> > + }
>
> This else branch now applies to BPF_TRACE_ITER as well, since the
> trampoline check was removed in tracing_prog_func_proto(). Should the
> condition be extended to also handle ITER, or should ITER be explicitly
> blocked from using these helpers?
>
> [ ... ]
>
>
> ---
> AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
> See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
>
> CI run summary: https://github.com/kernel-patches/bpf/actions/runs/21147849826
>
^ permalink raw reply
* Re: [PATCH net-next v4] page_pool: Add page_pool_release_stalled tracepoint
From: Leon Hwang @ 2026-01-20 3:16 UTC (permalink / raw)
To: Jakub Kicinski
Cc: netdev, Jesper Dangaard Brouer, Ilias Apalodimas, Steven Rostedt,
Masami Hiramatsu, Mathieu Desnoyers, David S . Miller,
Eric Dumazet, Paolo Abeni, Simon Horman, kerneljasonxing,
lance.yang, jiayuan.chen, linux-kernel, linux-trace-kernel,
Leon Huang Fu
In-Reply-To: <20260119083759.5deeaa3c@kernel.org>
On 20/1/26 00:37, Jakub Kicinski wrote:
> On Mon, 19 Jan 2026 18:21:19 +0800 Leon Hwang wrote:
>> Introduce a new tracepoint to track stalled page pool releases,
>> providing better observability for page pool lifecycle issues.
>
> Sorry, I really want you to answer the questions from the last
> paragraph of:
>
> https://lore.kernel.org/netdev/20260104084347.5de3a537@kernel.org/
Let me share a concrete case where this tracepoint would have helped,
and why netlink notifications were not a good fit.
I encountered the 'pr_warn()' messages during Mellanox NIC flapping on a
system using the 'mlx5_core' driver (kernel 6.6). The root cause turned
out to be an application-level issue: the IBM/sarama “Client SeekBroker
Connection Leak” [1].
In short, some TCP sockets became orphaned while still holding FINACK
skbs in their 'sk_receive_queue'. These skbs were holding inflight pages
from page pools. After NIC flapping, as long as those sockets were not
closed, the inflight pages could not be returned, and the corresponding
page pools could not be released. Once the orphaned sockets were
explicitly closed (as in [2]), the inflight pages were returned and the
page pools were eventually destroyed.
During the investigation, the dmesg output was noisy: there were many
inflight pages across multiple page pools, originating from many
orphaned sockets. This made it difficult to investigate and reason about
the issue using BPF tools.
In this scenario, a netlink notification does not seem like a good fit:
* The situation involved many page pools and many inflight pages.
* Emitting netlink notifications on each retry or stall would likely
generate a large volume of messages.
* What was needed was not a stream of notifications, but the ability to
observe and correlate page pool state over time.
A tracepoint fits this use case better. With a
'page_pool_release_stalled' tracepoint, it becomes straightforward to
use BPF tools to:
* Track which page pools are repeatedly stalled
* Correlate stalls with socket state, RX queues, or driver behavior
* Distinguish expected situations (e.g. orphaned sockets temporarily
holding pages) from genuine kernel or driver issues
From my experience, this tracepoint complements the existing
netlink-based observability rather than duplicating it, while avoiding
the risk of excessive netlink traffic in pathological but realistic
scenarios such as NIC flapping combined with connection leaks.
Thanks,
Leon
[1] https://github.com/IBM/sarama/issues/3143
[2] https://github.com/IBM/sarama/pull/3384
^ permalink raw reply
* [PATCH v4 2/2] mm/vmscan: add tracepoint and reason for kswapd_failures reset
From: Jiayuan Chen @ 2026-01-20 2:43 UTC (permalink / raw)
To: linux-mm
Cc: Jiayuan Chen, Shakeel Butt, Johannes Weiner, Jiayuan Chen,
Andrew Morton, David Hildenbrand, Lorenzo Stoakes,
Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
Suren Baghdasaryan, Michal Hocko, Axel Rasmussen, Yuanchu Xie,
Wei Xu, Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Brendan Jackman, Zi Yan, Qi Zheng, linux-kernel,
linux-trace-kernel
In-Reply-To: <20260120024402.387576-1-jiayuan.chen@linux.dev>
From: Jiayuan Chen <jiayuan.chen@shopee.com>
Currently, kswapd_failures is reset in multiple places (kswapd,
direct reclaim, PCP freeing, memory-tiers), but there's no way to
trace when and why it was reset, making it difficult to debug
memory reclaim issues.
This patch:
1. Introduce kswapd_clear_hopeless() as a wrapper function to
centralize kswapd_failures reset logic.
2. Introduce kswapd_test_hopeless() to encapsulate hopeless node
checks, replacing all open-coded kswapd_failures comparisons.
3. Add kswapd_clear_hopeless_reason enum to distinguish reset sources:
- KSWAPD_CLEAR_HOPELESS_KSWAPD: reset from kswapd context
- KSWAPD_CLEAR_HOPELESS_DIRECT: reset from direct reclaim
- KSWAPD_CLEAR_HOPELESS_PCP: reset from PCP page freeing
- KSWAPD_CLEAR_HOPELESS_OTHER: reset from other paths
4. Add tracepoints for better observability:
- mm_vmscan_kswapd_clear_hopeless: traces each reset with reason
- mm_vmscan_kswapd_reclaim_fail: traces each kswapd reclaim failure
Test results:
$ trace-cmd record -e vmscan:mm_vmscan_kswapd_clear_hopeless -e vmscan:mm_vmscan_kswapd_reclaim_fail
$ # generate memory pressure
$ trace-cmd report
cpus=4
kswapd0-71 [000] 27.216563: mm_vmscan_kswapd_reclaim_fail: nid=0 failures=1
kswapd0-71 [000] 27.217169: mm_vmscan_kswapd_reclaim_fail: nid=0 failures=2
kswapd0-71 [000] 27.217764: mm_vmscan_kswapd_reclaim_fail: nid=0 failures=3
kswapd0-71 [000] 27.218353: mm_vmscan_kswapd_reclaim_fail: nid=0 failures=4
kswapd0-71 [000] 27.218993: mm_vmscan_kswapd_reclaim_fail: nid=0 failures=5
kswapd0-71 [000] 27.219744: mm_vmscan_kswapd_reclaim_fail: nid=0 failures=6
kswapd0-71 [000] 27.220488: mm_vmscan_kswapd_reclaim_fail: nid=0 failures=7
kswapd0-71 [000] 27.221206: mm_vmscan_kswapd_reclaim_fail: nid=0 failures=8
kswapd0-71 [000] 27.221806: mm_vmscan_kswapd_reclaim_fail: nid=0 failures=9
kswapd0-71 [000] 27.222634: mm_vmscan_kswapd_reclaim_fail: nid=0 failures=10
kswapd0-71 [000] 27.223286: mm_vmscan_kswapd_reclaim_fail: nid=0 failures=11
kswapd0-71 [000] 27.223894: mm_vmscan_kswapd_reclaim_fail: nid=0 failures=12
kswapd0-71 [000] 27.224712: mm_vmscan_kswapd_reclaim_fail: nid=0 failures=13
kswapd0-71 [000] 27.225424: mm_vmscan_kswapd_reclaim_fail: nid=0 failures=14
kswapd0-71 [000] 27.226082: mm_vmscan_kswapd_reclaim_fail: nid=0 failures=15
kswapd0-71 [000] 27.226810: mm_vmscan_kswapd_reclaim_fail: nid=0 failures=16
kswapd1-72 [002] 27.386869: mm_vmscan_kswapd_reclaim_fail: nid=1 failures=1
kswapd1-72 [002] 27.387435: mm_vmscan_kswapd_reclaim_fail: nid=1 failures=2
kswapd1-72 [002] 27.388016: mm_vmscan_kswapd_reclaim_fail: nid=1 failures=3
kswapd1-72 [002] 27.388586: mm_vmscan_kswapd_reclaim_fail: nid=1 failures=4
kswapd1-72 [002] 27.389155: mm_vmscan_kswapd_reclaim_fail: nid=1 failures=5
kswapd1-72 [002] 27.389723: mm_vmscan_kswapd_reclaim_fail: nid=1 failures=6
kswapd1-72 [002] 27.390292: mm_vmscan_kswapd_reclaim_fail: nid=1 failures=7
kswapd1-72 [002] 27.392364: mm_vmscan_kswapd_reclaim_fail: nid=1 failures=8
kswapd1-72 [002] 27.392934: mm_vmscan_kswapd_reclaim_fail: nid=1 failures=9
kswapd1-72 [002] 27.393504: mm_vmscan_kswapd_reclaim_fail: nid=1 failures=10
kswapd1-72 [002] 27.394073: mm_vmscan_kswapd_reclaim_fail: nid=1 failures=11
kswapd1-72 [002] 27.394899: mm_vmscan_kswapd_reclaim_fail: nid=1 failures=12
kswapd1-72 [002] 27.395472: mm_vmscan_kswapd_reclaim_fail: nid=1 failures=13
kswapd1-72 [002] 27.396055: mm_vmscan_kswapd_reclaim_fail: nid=1 failures=14
kswapd1-72 [002] 27.396628: mm_vmscan_kswapd_reclaim_fail: nid=1 failures=15
kswapd1-72 [002] 27.397199: mm_vmscan_kswapd_reclaim_fail: nid=1 failures=16
kworker/u18:0-40 [002] 27.410151: mm_vmscan_kswapd_clear_hopeless: nid=0 reason=DIRECT
kswapd0-71 [000] 27.439454: mm_vmscan_kswapd_reclaim_fail: nid=0 failures=1
kswapd0-71 [000] 27.440048: mm_vmscan_kswapd_reclaim_fail: nid=0 failures=2
kswapd0-71 [000] 27.440634: mm_vmscan_kswapd_reclaim_fail: nid=0 failures=3
kswapd0-71 [000] 27.441211: mm_vmscan_kswapd_reclaim_fail: nid=0 failures=4
kswapd0-71 [000] 27.441787: mm_vmscan_kswapd_reclaim_fail: nid=0 failures=5
kswapd0-71 [000] 27.442363: mm_vmscan_kswapd_reclaim_fail: nid=0 failures=6
kswapd0-71 [000] 27.443030: mm_vmscan_kswapd_reclaim_fail: nid=0 failures=7
kswapd0-71 [000] 27.443725: mm_vmscan_kswapd_reclaim_fail: nid=0 failures=8
kswapd0-71 [000] 27.444315: mm_vmscan_kswapd_reclaim_fail: nid=0 failures=9
kswapd0-71 [000] 27.444898: mm_vmscan_kswapd_reclaim_fail: nid=0 failures=10
kswapd0-71 [000] 27.445476: mm_vmscan_kswapd_reclaim_fail: nid=0 failures=11
kswapd0-71 [000] 27.446053: mm_vmscan_kswapd_reclaim_fail: nid=0 failures=12
kswapd0-71 [000] 27.446646: mm_vmscan_kswapd_reclaim_fail: nid=0 failures=13
kswapd0-71 [000] 27.447230: mm_vmscan_kswapd_reclaim_fail: nid=0 failures=14
kswapd0-71 [000] 27.447812: mm_vmscan_kswapd_reclaim_fail: nid=0 failures=15
kswapd0-71 [000] 27.448391: mm_vmscan_kswapd_reclaim_fail: nid=0 failures=16
ann-423 [003] 28.028285: mm_vmscan_kswapd_clear_hopeless: nid=0 reason=PCP
Acked-by: Shakeel Butt <shakeel.butt@linux.dev>
Suggested-by: Johannes Weiner <hannes@cmpxchg.org>
Signed-off-by: Jiayuan Chen <jiayuan.chen@shopee.com>
Signed-off-by: Jiayuan Chen <jiayuan.chen@linux.dev>
---
include/linux/mmzone.h | 19 ++++++++++---
include/trace/events/vmscan.h | 51 +++++++++++++++++++++++++++++++++++
mm/memory-tiers.c | 2 +-
mm/page_alloc.c | 4 +--
mm/show_mem.c | 3 +--
mm/vmscan.c | 29 +++++++++++++-------
mm/vmstat.c | 2 +-
7 files changed, 91 insertions(+), 19 deletions(-)
diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
index 3a0f52188ff6..d26fdd48f106 100644
--- a/include/linux/mmzone.h
+++ b/include/linux/mmzone.h
@@ -1534,16 +1534,27 @@ static inline unsigned long pgdat_end_pfn(pg_data_t *pgdat)
#include <linux/memory_hotplug.h>
void build_all_zonelists(pg_data_t *pgdat);
-void wakeup_kswapd(struct zone *zone, gfp_t gfp_mask, int order,
- enum zone_type highest_zoneidx);
-void kswapd_try_clear_hopeless(struct pglist_data *pgdat,
- unsigned int order, int highest_zoneidx);
bool __zone_watermark_ok(struct zone *z, unsigned int order, unsigned long mark,
int highest_zoneidx, unsigned int alloc_flags,
long free_pages);
bool zone_watermark_ok(struct zone *z, unsigned int order,
unsigned long mark, int highest_zoneidx,
unsigned int alloc_flags);
+
+enum kswapd_clear_hopeless_reason {
+ KSWAPD_CLEAR_HOPELESS_OTHER = 0,
+ KSWAPD_CLEAR_HOPELESS_KSWAPD,
+ KSWAPD_CLEAR_HOPELESS_DIRECT,
+ KSWAPD_CLEAR_HOPELESS_PCP,
+};
+
+void wakeup_kswapd(struct zone *zone, gfp_t gfp_mask, int order,
+ enum zone_type highest_zoneidx);
+void kswapd_try_clear_hopeless(struct pglist_data *pgdat,
+ unsigned int order, int highest_zoneidx);
+void kswapd_clear_hopeless(pg_data_t *pgdat, enum kswapd_clear_hopeless_reason reason);
+bool kswapd_test_hopeless(pg_data_t *pgdat);
+
/*
* Memory initialization context, use to differentiate memory added by
* the platform statically or via memory hotplug interface.
diff --git a/include/trace/events/vmscan.h b/include/trace/events/vmscan.h
index 490958fa10de..ea58e4656abf 100644
--- a/include/trace/events/vmscan.h
+++ b/include/trace/events/vmscan.h
@@ -40,6 +40,16 @@
{_VMSCAN_THROTTLE_CONGESTED, "VMSCAN_THROTTLE_CONGESTED"} \
) : "VMSCAN_THROTTLE_NONE"
+TRACE_DEFINE_ENUM(KSWAPD_CLEAR_HOPELESS_OTHER);
+TRACE_DEFINE_ENUM(KSWAPD_CLEAR_HOPELESS_KSWAPD);
+TRACE_DEFINE_ENUM(KSWAPD_CLEAR_HOPELESS_DIRECT);
+TRACE_DEFINE_ENUM(KSWAPD_CLEAR_HOPELESS_PCP);
+
+#define kswapd_clear_hopeless_reason_ops \
+ {KSWAPD_CLEAR_HOPELESS_KSWAPD, "KSWAPD"}, \
+ {KSWAPD_CLEAR_HOPELESS_DIRECT, "DIRECT"}, \
+ {KSWAPD_CLEAR_HOPELESS_PCP, "PCP"}, \
+ {KSWAPD_CLEAR_HOPELESS_OTHER, "OTHER"}
#define trace_reclaim_flags(file) ( \
(file ? RECLAIM_WB_FILE : RECLAIM_WB_ANON) | \
@@ -535,6 +545,47 @@ TRACE_EVENT(mm_vmscan_throttled,
__entry->usec_delayed,
show_throttle_flags(__entry->reason))
);
+
+TRACE_EVENT(mm_vmscan_kswapd_reclaim_fail,
+
+ TP_PROTO(int nid, int failures),
+
+ TP_ARGS(nid, failures),
+
+ TP_STRUCT__entry(
+ __field(int, nid)
+ __field(int, failures)
+ ),
+
+ TP_fast_assign(
+ __entry->nid = nid;
+ __entry->failures = failures;
+ ),
+
+ TP_printk("nid=%d failures=%d",
+ __entry->nid, __entry->failures)
+);
+
+TRACE_EVENT(mm_vmscan_kswapd_clear_hopeless,
+
+ TP_PROTO(int nid, int reason),
+
+ TP_ARGS(nid, reason),
+
+ TP_STRUCT__entry(
+ __field(int, nid)
+ __field(int, reason)
+ ),
+
+ TP_fast_assign(
+ __entry->nid = nid;
+ __entry->reason = reason;
+ ),
+
+ TP_printk("nid=%d reason=%s",
+ __entry->nid,
+ __print_symbolic(__entry->reason, kswapd_clear_hopeless_reason_ops))
+);
#endif /* _TRACE_VMSCAN_H */
/* This part must be outside protection */
diff --git a/mm/memory-tiers.c b/mm/memory-tiers.c
index 864811fff409..d6ef5ba8e70a 100644
--- a/mm/memory-tiers.c
+++ b/mm/memory-tiers.c
@@ -956,7 +956,7 @@ static ssize_t demotion_enabled_store(struct kobject *kobj,
struct pglist_data *pgdat;
for_each_online_pgdat(pgdat)
- atomic_set(&pgdat->kswapd_failures, 0);
+ kswapd_clear_hopeless(pgdat, KSWAPD_CLEAR_HOPELESS_OTHER);
}
return count;
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index c380f063e8b7..1b1dedc7ede1 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -2916,9 +2916,9 @@ static bool free_frozen_page_commit(struct zone *zone,
* 'hopeless node' to stay in that state for a while. Let
* kswapd work again by resetting kswapd_failures.
*/
- if (atomic_read(&pgdat->kswapd_failures) >= MAX_RECLAIM_RETRIES &&
+ if (kswapd_test_hopeless(pgdat) &&
next_memory_node(pgdat->node_id) < MAX_NUMNODES)
- atomic_set(&pgdat->kswapd_failures, 0);
+ kswapd_clear_hopeless(pgdat, KSWAPD_CLEAR_HOPELESS_PCP);
}
return ret;
}
diff --git a/mm/show_mem.c b/mm/show_mem.c
index 3a4b5207635d..24078ac3e6bc 100644
--- a/mm/show_mem.c
+++ b/mm/show_mem.c
@@ -278,8 +278,7 @@ static void show_free_areas(unsigned int filter, nodemask_t *nodemask, int max_z
#endif
K(node_page_state(pgdat, NR_PAGETABLE)),
K(node_page_state(pgdat, NR_SECONDARY_PAGETABLE)),
- str_yes_no(atomic_read(&pgdat->kswapd_failures) >=
- MAX_RECLAIM_RETRIES),
+ str_yes_no(kswapd_test_hopeless(pgdat)),
K(node_page_state(pgdat, NR_BALLOON_PAGES)));
}
diff --git a/mm/vmscan.c b/mm/vmscan.c
index ecd019b8b452..0ec2baa4ed4e 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -507,7 +507,7 @@ static bool skip_throttle_noprogress(pg_data_t *pgdat)
* If kswapd is disabled, reschedule if necessary but do not
* throttle as the system is likely near OOM.
*/
- if (atomic_read(&pgdat->kswapd_failures) >= MAX_RECLAIM_RETRIES)
+ if (kswapd_test_hopeless(pgdat))
return true;
/*
@@ -6453,7 +6453,7 @@ static bool allow_direct_reclaim(pg_data_t *pgdat)
int i;
bool wmark_ok;
- if (atomic_read(&pgdat->kswapd_failures) >= MAX_RECLAIM_RETRIES)
+ if (kswapd_test_hopeless(pgdat))
return true;
for_each_managed_zone_pgdat(zone, pgdat, i, ZONE_NORMAL) {
@@ -6862,7 +6862,7 @@ static bool prepare_kswapd_sleep(pg_data_t *pgdat, int order,
wake_up_all(&pgdat->pfmemalloc_wait);
/* Hopeless node, leave it to direct reclaim */
- if (atomic_read(&pgdat->kswapd_failures) >= MAX_RECLAIM_RETRIES)
+ if (kswapd_test_hopeless(pgdat))
return true;
if (pgdat_balanced(pgdat, order, highest_zoneidx)) {
@@ -7134,8 +7134,11 @@ static int balance_pgdat(pg_data_t *pgdat, int order, int highest_zoneidx)
* watermark_high at this point. We need to avoid increasing the
* failure count to prevent the kswapd thread from stopping.
*/
- if (!sc.nr_reclaimed && !boosted)
- atomic_inc(&pgdat->kswapd_failures);
+ if (!sc.nr_reclaimed && !boosted) {
+ int fail_cnt = atomic_inc_return(&pgdat->kswapd_failures);
+ /* kswapd context, low overhead to trace every failure */
+ trace_mm_vmscan_kswapd_reclaim_fail(pgdat->node_id, fail_cnt);
+ }
out:
clear_reclaim_active(pgdat, highest_zoneidx);
@@ -7394,7 +7397,7 @@ void wakeup_kswapd(struct zone *zone, gfp_t gfp_flags, int order,
return;
/* Hopeless node, leave it to direct reclaim if possible */
- if (atomic_read(&pgdat->kswapd_failures) >= MAX_RECLAIM_RETRIES ||
+ if (kswapd_test_hopeless(pgdat) ||
(pgdat_balanced(pgdat, order, highest_zoneidx) &&
!pgdat_watermark_boosted(pgdat, highest_zoneidx))) {
/*
@@ -7414,9 +7417,11 @@ void wakeup_kswapd(struct zone *zone, gfp_t gfp_flags, int order,
wake_up_interruptible(&pgdat->kswapd_wait);
}
-static void kswapd_clear_hopeless(pg_data_t *pgdat)
+void kswapd_clear_hopeless(pg_data_t *pgdat, enum kswapd_clear_hopeless_reason reason)
{
- atomic_set(&pgdat->kswapd_failures, 0);
+ /* Only trace actual resets, not redundant zero-to-zero */
+ if (atomic_xchg(&pgdat->kswapd_failures, 0))
+ trace_mm_vmscan_kswapd_clear_hopeless(pgdat->node_id, reason);
}
/*
@@ -7429,7 +7434,13 @@ void kswapd_try_clear_hopeless(struct pglist_data *pgdat,
unsigned int order, int highest_zoneidx)
{
if (pgdat_balanced(pgdat, order, highest_zoneidx))
- kswapd_clear_hopeless(pgdat);
+ kswapd_clear_hopeless(pgdat, current_is_kswapd() ?
+ KSWAPD_CLEAR_HOPELESS_KSWAPD : KSWAPD_CLEAR_HOPELESS_DIRECT);
+}
+
+bool kswapd_test_hopeless(pg_data_t *pgdat)
+{
+ return atomic_read(&pgdat->kswapd_failures) >= MAX_RECLAIM_RETRIES;
}
#ifdef CONFIG_HIBERNATION
diff --git a/mm/vmstat.c b/mm/vmstat.c
index 65de88cdf40e..3d65f5c9c224 100644
--- a/mm/vmstat.c
+++ b/mm/vmstat.c
@@ -1855,7 +1855,7 @@ static void zoneinfo_show_print(struct seq_file *m, pg_data_t *pgdat,
"\n start_pfn: %lu"
"\n reserved_highatomic: %lu"
"\n free_highatomic: %lu",
- atomic_read(&pgdat->kswapd_failures) >= MAX_RECLAIM_RETRIES,
+ kswapd_test_hopeless(pgdat),
zone->zone_start_pfn,
zone->nr_reserved_highatomic,
zone->nr_free_highatomic);
--
2.43.0
^ permalink raw reply related
* [PATCH v4 1/2] mm/vmscan: mitigate spurious kswapd_failures reset from direct reclaim
From: Jiayuan Chen @ 2026-01-20 2:43 UTC (permalink / raw)
To: linux-mm
Cc: Jiayuan Chen, Shakeel Butt, Jiayuan Chen, Andrew Morton,
David Hildenbrand, Lorenzo Stoakes, Liam R. Howlett,
Vlastimil Babka, Mike Rapoport, Suren Baghdasaryan, Michal Hocko,
Axel Rasmussen, Yuanchu Xie, Wei Xu, Steven Rostedt,
Masami Hiramatsu, Mathieu Desnoyers, Brendan Jackman,
Johannes Weiner, Zi Yan, Qi Zheng, linux-kernel,
linux-trace-kernel
In-Reply-To: <20260120024402.387576-1-jiayuan.chen@linux.dev>
From: Jiayuan Chen <jiayuan.chen@shopee.com>
When kswapd fails to reclaim memory, kswapd_failures is incremented.
Once it reaches MAX_RECLAIM_RETRIES, kswapd stops running to avoid
futile reclaim attempts. However, any successful direct reclaim
unconditionally resets kswapd_failures to 0, which can cause problems.
We observed an issue in production on a multi-NUMA system where a
process allocated large amounts of anonymous pages on a single NUMA
node, causing its watermark to drop below high and evicting most file
pages:
$ numastat -m
Per-node system memory usage (in MBs):
Node 0 Node 1 Total
--------------- --------------- ---------------
MemTotal 128222.19 127983.91 256206.11
MemFree 1414.48 1432.80 2847.29
MemUsed 126807.71 126551.11 252358.82
SwapCached 0.00 0.00 0.00
Active 29017.91 25554.57 54572.48
Inactive 92749.06 95377.00 188126.06
Active(anon) 28998.96 23356.47 52355.43
Inactive(anon) 92685.27 87466.11 180151.39
Active(file) 18.95 2198.10 2217.05
Inactive(file) 63.79 7910.89 7974.68
With swap disabled, only file pages can be reclaimed. When kswapd is
woken (e.g., via wake_all_kswapds()), it runs continuously but cannot
raise free memory above the high watermark since reclaimable file pages
are insufficient. Normally, kswapd would eventually stop after
kswapd_failures reaches MAX_RECLAIM_RETRIES.
However, containers on this machine have memory.high set in their
cgroup. Business processes continuously trigger the high limit, causing
frequent direct reclaim that keeps resetting kswapd_failures to 0. This
prevents kswapd from ever stopping.
The key insight is that direct reclaim triggered by cgroup memory.high
performs aggressive scanning to throttle the allocating process. With
sufficiently aggressive scanning, even hot pages will eventually be
reclaimed, making direct reclaim "successful" at freeing some memory.
However, this success does not mean the node has reached a balanced
state - the freed memory may still be insufficient to bring free pages
above the high watermark. Unconditionally resetting kswapd_failures in
this case keeps kswapd alive indefinitely.
The result is that kswapd runs endlessly. Unlike direct reclaim which
only reclaims from the allocating cgroup, kswapd scans the entire node's
memory. This causes hot file pages from all workloads on the node to be
evicted, not just those from the cgroup triggering memory.high. These
pages constantly refault, generating sustained heavy IO READ pressure
across the entire system.
Fix this by only resetting kswapd_failures when the node is actually
balanced. This allows both kswapd and direct reclaim to clear
kswapd_failures upon successful reclaim, but only when the reclaim
actually resolves the memory pressure (i.e., the node becomes balanced).
Acked-by: Shakeel Butt <shakeel.butt@linux.dev>
Signed-off-by: Jiayuan Chen <jiayuan.chen@shopee.com>
Signed-off-by: Jiayuan Chen <jiayuan.chen@linux.dev>
---
include/linux/mmzone.h | 2 ++
mm/vmscan.c | 22 ++++++++++++++++++++--
2 files changed, 22 insertions(+), 2 deletions(-)
diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
index 75ef7c9f9307..3a0f52188ff6 100644
--- a/include/linux/mmzone.h
+++ b/include/linux/mmzone.h
@@ -1536,6 +1536,8 @@ static inline unsigned long pgdat_end_pfn(pg_data_t *pgdat)
void build_all_zonelists(pg_data_t *pgdat);
void wakeup_kswapd(struct zone *zone, gfp_t gfp_mask, int order,
enum zone_type highest_zoneidx);
+void kswapd_try_clear_hopeless(struct pglist_data *pgdat,
+ unsigned int order, int highest_zoneidx);
bool __zone_watermark_ok(struct zone *z, unsigned int order, unsigned long mark,
int highest_zoneidx, unsigned int alloc_flags,
long free_pages);
diff --git a/mm/vmscan.c b/mm/vmscan.c
index 670fe9fae5ba..ecd019b8b452 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -5067,7 +5067,7 @@ static void lru_gen_shrink_node(struct pglist_data *pgdat, struct scan_control *
blk_finish_plug(&plug);
done:
if (sc->nr_reclaimed > reclaimed)
- atomic_set(&pgdat->kswapd_failures, 0);
+ kswapd_try_clear_hopeless(pgdat, sc->order, sc->reclaim_idx);
}
/******************************************************************************
@@ -6141,7 +6141,7 @@ static void shrink_node(pg_data_t *pgdat, struct scan_control *sc)
* successful direct reclaim run will revive a dormant kswapd.
*/
if (reclaimable)
- atomic_set(&pgdat->kswapd_failures, 0);
+ kswapd_try_clear_hopeless(pgdat, sc->order, sc->reclaim_idx);
else if (sc->cache_trim_mode)
sc->cache_trim_mode_failed = 1;
}
@@ -7414,6 +7414,24 @@ void wakeup_kswapd(struct zone *zone, gfp_t gfp_flags, int order,
wake_up_interruptible(&pgdat->kswapd_wait);
}
+static void kswapd_clear_hopeless(pg_data_t *pgdat)
+{
+ atomic_set(&pgdat->kswapd_failures, 0);
+}
+
+/*
+ * Reset kswapd_failures only when the node is balanced. Without this
+ * check, successful direct reclaim (e.g., from cgroup memory.high
+ * throttling) can keep resetting kswapd_failures even when the node
+ * cannot be balanced, causing kswapd to run endlessly.
+ */
+void kswapd_try_clear_hopeless(struct pglist_data *pgdat,
+ unsigned int order, int highest_zoneidx)
+{
+ if (pgdat_balanced(pgdat, order, highest_zoneidx))
+ kswapd_clear_hopeless(pgdat);
+}
+
#ifdef CONFIG_HIBERNATION
/*
* Try to free `nr_to_reclaim' of memory, system-wide, and return the number of
--
2.43.0
^ permalink raw reply related
* [PATCH v4 0/2] mm/vmscan: mitigate spurious kswapd_failures reset and add tracepoints
From: Jiayuan Chen @ 2026-01-20 2:43 UTC (permalink / raw)
To: linux-mm
Cc: Jiayuan Chen, Andrew Morton, David Hildenbrand, Lorenzo Stoakes,
Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
Suren Baghdasaryan, Michal Hocko, Axel Rasmussen, Yuanchu Xie,
Wei Xu, Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Brendan Jackman, Johannes Weiner, Zi Yan, Qi Zheng, Shakeel Butt,
Jiayuan Chen, linux-kernel, linux-trace-kernel
== Problem ==
We observed an issue in production on a multi-NUMA system where kswapd
runs endlessly, causing sustained heavy IO READ pressure across the
entire system.
The root cause is that direct reclaim triggered by cgroup memory.high
keeps resetting kswapd_failures to 0, even when the node cannot be
balanced. This prevents kswapd from ever stopping after reaching
MAX_RECLAIM_RETRIES.
```bash
bpftrace -e '
#include <linux/mmzone.h>
#include <linux/shrinker.h>
kprobe:balance_pgdat {
$pgdat = (struct pglist_data *)arg0;
if ($pgdat->kswapd_failures > 0) {
printf("[node %d] [%lu] kswapd end, kswapd_failures %d\n",
$pgdat->node_id, jiffies, $pgdat->kswapd_failures);
}
}
tracepoint:vmscan:mm_vmscan_direct_reclaim_end {
printf("[cpu %d] [%ul] reset kswapd_failures %d \n", cpu, jiffies,
args.nr_reclaimed)
}
'
```
The trace results showed that when kswapd_failures reaches 15, continuous
direct reclaim keeps resetting it to 0. This was accompanied by a flood of
kswapd_failures log entries, and shortly after, we observed massive
refaults occurring.
== Solution ==
Patch 1 fixes the issue by only resetting kswapd_failures when the node
is actually balanced. This introduces pgdat_try_reset_kswapd_failures()
as a wrapper that checks pgdat_balanced() before resetting.
Patch 2 extends the wrapper to track why kswapd_failures was reset,
adding tracepoints for better observability:
- mm_vmscan_reset_kswapd_failures: traces each reset with reason
- mm_vmscan_kswapd_reclaim_fail: traces each kswapd reclaim failure
---
v3 -> v4: https://lore.kernel.org/linux-mm/20260114074049.229935-1-jiayuan.chen@linux.dev/
- Add Acked-by tags
- Some modifications suggested by Johannes Weiner
v2 -> v3: https://lore.kernel.org/all/20251226080042.291657-1-jiayuan.chen@linux.dev/
- Add tracepoints for kswapd_failures reset and reclaim failure
- Expand commit message with test results
v1 -> v2: https://lore.kernel.org/all/20251222122022.254268-1-jiayuan.chen@linux.dev/
Jiayuan Chen (2):
mm/vmscan: mitigate spurious kswapd_failures reset from direct reclaim
mm/vmscan: add tracepoint and reason for kswapd_failures reset
include/linux/mmzone.h | 17 ++++++++++--
include/trace/events/vmscan.h | 51 +++++++++++++++++++++++++++++++++++
mm/memory-tiers.c | 2 +-
mm/page_alloc.c | 4 +--
mm/show_mem.c | 3 +--
mm/vmscan.c | 45 +++++++++++++++++++++++++------
mm/vmstat.c | 2 +-
7 files changed, 108 insertions(+), 16 deletions(-)
--
2.43.0
^ permalink raw reply
* Re: [RFC PATCH v1 09/37] KVM: guest_memfd: Skip LRU for guest_memfd folios
From: Yan Zhao @ 2026-01-20 2:15 UTC (permalink / raw)
To: Ackerley Tng
Cc: cgroups, kvm, linux-doc, linux-fsdevel, linux-kernel,
linux-kselftest, linux-mm, linux-trace-kernel, x86, akpm,
binbin.wu, bp, brauner, chao.p.peng, chenhuacai, corbet,
dave.hansen, dave.hansen, david, dmatlack, erdemaktas, fan.du,
fvdl, haibo1.xu, hannes, hch, hpa, hughd, ira.weiny,
isaku.yamahata, jack, james.morse, jarkko, jgg, jgowans, jhubbard,
jroedel, jthoughton, jun.miao, kai.huang, keirf, kent.overstreet,
liam.merwick, maciej.wieczor-retman, mail, maobibo,
mathieu.desnoyers, maz, mhiramat, mhocko, mic, michael.roth,
mingo, mlevitsk, mpe, muchun.song, nikunj, nsaenz, oliver.upton,
palmer, pankaj.gupta, paul.walmsley, pbonzini, peterx, pgonda,
prsampat, pvorel, qperret, richard.weiyang, rick.p.edgecombe,
rientjes, rostedt, roypat, rppt, seanjc, shakeel.butt, shuah,
steven.price, steven.sistare, suzuki.poulose, tabba, tglx,
thomas.lendacky, vannapurve, vbabka, viro, vkuznets, wei.w.wang,
will, willy, wyihan, xiaoyao.li, yilun.xu, yuzenghui, zhiquan1.li
In-Reply-To: <02aad35b728f4918e62dc6eb1d1d5546487b099e.1760731772.git.ackerleytng@google.com>
> +static struct folio *__kvm_gmem_get_folio(struct address_space *mapping,
> + pgoff_t index,
> + struct mempolicy *policy)
> +{
> + const gfp_t gfp = mapping_gfp_mask(mapping);
> + struct folio *folio;
> + int err;
> +
> + folio = filemap_lock_folio(mapping, index);
> + if (!IS_ERR(folio))
> + return folio;
> +
> + folio = filemap_alloc_folio(gfp, 0, policy);
> + if (!folio)
> + return ERR_PTR(-ENOMEM);
> +
> + err = mem_cgroup_charge(folio, NULL, gfp);
> + if (err)
> + goto err_put;
> +
> + __folio_set_locked(folio);
> +
> + err = __filemap_add_folio(mapping, folio, index, gfp, NULL);
> + if (err) {
mem_cgroup_uncharge(folio);
> + __folio_clear_locked(folio);
> + goto err_put;
> + }
> +
> + return folio;
> +
> +err_put:
> + folio_put(folio);
> + return ERR_PTR(err);
> +}
> +
> /*
> * Returns a locked folio on success. The caller is responsible for
> * setting the up-to-date flag before the memory is mapped into the guest.
> @@ -160,6 +195,7 @@ static struct mempolicy *kvm_gmem_get_folio_policy(struct gmem_inode *gi,
> static struct folio *kvm_gmem_get_folio(struct inode *inode, pgoff_t index)
> {
> /* TODO: Support huge pages. */
> + struct address_space *mapping = inode->i_mapping;
> struct mempolicy *policy;
> struct folio *folio;
>
> @@ -167,16 +203,17 @@ static struct folio *kvm_gmem_get_folio(struct inode *inode, pgoff_t index)
> * Fast-path: See if folio is already present in mapping to avoid
> * policy_lookup.
> */
> - folio = filemap_lock_folio(inode->i_mapping, index);
> + folio = filemap_lock_folio(mapping, index);
> if (!IS_ERR(folio))
> return folio;
>
> policy = kvm_gmem_get_folio_policy(GMEM_I(inode), index);
> - folio = __filemap_get_folio_mpol(inode->i_mapping, index,
> - FGP_LOCK | FGP_CREAT,
> - mapping_gfp_mask(inode->i_mapping), policy);
> - mpol_cond_put(policy);
>
> + do {
> + folio = __kvm_gmem_get_folio(mapping, index, policy);
> + } while (IS_ERR(folio) && PTR_ERR(folio) == -EEXIST);
Why not just return ERR_PTR(-EEXIST) up to kvm_gmem_get_pfn() and have a higher
level retry?
> + mpol_cond_put(policy);
> return folio;
> }
^ permalink raw reply
* Re: [PATCH bpf-next v3 1/2] bpf: support bpf_get_func_arg() for BPF_TRACE_RAW_TP
From: Menglong Dong @ 2026-01-20 1:24 UTC (permalink / raw)
To: Menglong Dong, Jiri Olsa
Cc: andrii, ast, daniel, john.fastabend, martin.lau, eddyz87, song,
yonghong.song, kpsingh, sdf, haoluo, mattbobrowski, rostedt,
mhiramat, mathieu.desnoyers, bpf, linux-kernel,
linux-trace-kernel
In-Reply-To: <aW7APKlKCgg2_YvW@krava>
On 2026/1/20 07:37, Jiri Olsa wrote:
> On Mon, Jan 19, 2026 at 10:37:31AM +0800, Menglong Dong wrote:
> > For now, bpf_get_func_arg() and bpf_get_func_arg_cnt() is not supported by
> > the BPF_TRACE_RAW_TP, which is not convenient to get the argument of the
> > tracepoint, especially for the case that the position of the arguments in
> > a tracepoint can change.
> >
> > The target tracepoint BTF type id is specified during loading time,
> > therefore we can get the function argument count from the function
> > prototype instead of the stack.
> >
> > Signed-off-by: Menglong Dong <dongml2@chinatelecom.cn>
> > ---
> > v3:
> > - remove unnecessary NULL checking for prog->aux->attach_func_proto
> >
> > v2:
> > - for nr_args, skip first 'void *__data' argument in btf_trace_##name
> > typedef
> > ---
> > kernel/bpf/verifier.c | 32 ++++++++++++++++++++++++++++----
> > kernel/trace/bpf_trace.c | 4 ++--
> > 2 files changed, 30 insertions(+), 6 deletions(-)
> >
> > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> > index faa1ecc1fe9d..4f52342573f0 100644
> > --- a/kernel/bpf/verifier.c
> > +++ b/kernel/bpf/verifier.c
> > @@ -23316,8 +23316,20 @@ static int do_misc_fixups(struct bpf_verifier_env *env)
> > /* Implement bpf_get_func_arg inline. */
> > if (prog_type == BPF_PROG_TYPE_TRACING &&
> > insn->imm == BPF_FUNC_get_func_arg) {
> > - /* Load nr_args from ctx - 8 */
> > - insn_buf[0] = BPF_LDX_MEM(BPF_DW, BPF_REG_0, BPF_REG_1, -8);
> > + if (eatype == BPF_TRACE_RAW_TP) {
> > + int nr_args = btf_type_vlen(prog->aux->attach_func_proto);
> > +
> > + /*
> > + * skip first 'void *__data' argument in btf_trace_##name
> > + * typedef
> > + */
> > + nr_args--;
> > + /* Save nr_args to reg0 */
> > + insn_buf[0] = BPF_MOV64_IMM(BPF_REG_0, nr_args);
> > + } else {
> > + /* Load nr_args from ctx - 8 */
> > + insn_buf[0] = BPF_LDX_MEM(BPF_DW, BPF_REG_0, BPF_REG_1, -8);
> > + }
> > insn_buf[1] = BPF_JMP32_REG(BPF_JGE, BPF_REG_2, BPF_REG_0, 6);
> > insn_buf[2] = BPF_ALU64_IMM(BPF_LSH, BPF_REG_2, 3);
> > insn_buf[3] = BPF_ALU64_REG(BPF_ADD, BPF_REG_2, BPF_REG_1);
> > @@ -23369,8 +23381,20 @@ static int do_misc_fixups(struct bpf_verifier_env *env)
> > /* Implement get_func_arg_cnt inline. */
> > if (prog_type == BPF_PROG_TYPE_TRACING &&
> > insn->imm == BPF_FUNC_get_func_arg_cnt) {
> > - /* Load nr_args from ctx - 8 */
> > - insn_buf[0] = BPF_LDX_MEM(BPF_DW, BPF_REG_0, BPF_REG_1, -8);
> > + if (eatype == BPF_TRACE_RAW_TP) {
> > + int nr_args = btf_type_vlen(prog->aux->attach_func_proto);
> > +
> > + /*
> > + * skip first 'void *__data' argument in btf_trace_##name
> > + * typedef
> > + */
> > + nr_args--;
> > + /* Save nr_args to reg0 */
>
> I think we can attach single bpf program to multiple rawtp tracepoints,
> in which case this would not work properly for such program links on
> tracepoints with different nr_args, right?
Hi, Jiri. As for now, I think we can't do that when I look into
bpf_raw_tp_link_attach(). For the BPF_TRACE_RAW_TP, we specify
a target btf type id when loading the bpf prog. And during
attaching, it seems that we can only attach to that target, which
means that we can't attach to multiple rawtp tracepoint. And
we can't change the target btf id when reattach, too. Right?
Part of the implement of bpf_raw_tp_link_attach():
static int bpf_raw_tp_link_attach(struct bpf_prog *prog,
const char __user *user_tp_name, u64 cookie,
enum bpf_attach_type attach_type)
{
struct bpf_link_primer link_primer;
struct bpf_raw_tp_link *link;
struct bpf_raw_event_map *btp;
const char *tp_name;
char buf[128];
int err;
switch (prog->type) {
case BPF_PROG_TYPE_TRACING:
case BPF_PROG_TYPE_EXT:
case BPF_PROG_TYPE_LSM:
if (user_tp_name)
/* The attach point for this category of programs
* should be specified via btf_id during program load.
*/
return -EINVAL;
if (prog->type == BPF_PROG_TYPE_TRACING &&
prog->expected_attach_type == BPF_TRACE_RAW_TP) {
tp_name = prog->aux->attach_func_name;
break;
}
[......]
}
[......]
}
Thanks!
Menglong Dong
>
> jirka
>
>
> > + insn_buf[0] = BPF_MOV64_IMM(BPF_REG_0, nr_args);
> > + } else {
> > + /* Load nr_args from ctx - 8 */
> > + insn_buf[0] = BPF_LDX_MEM(BPF_DW, BPF_REG_0, BPF_REG_1, -8);
> > + }
> >
> > new_prog = bpf_patch_insn_data(env, i + delta, insn_buf, 1);
> > if (!new_prog)
> > diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> > index 6e076485bf70..9b1b56851d26 100644
> > --- a/kernel/trace/bpf_trace.c
> > +++ b/kernel/trace/bpf_trace.c
> > @@ -1734,11 +1734,11 @@ tracing_prog_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
> > case BPF_FUNC_d_path:
> > return &bpf_d_path_proto;
> > case BPF_FUNC_get_func_arg:
> > - return bpf_prog_has_trampoline(prog) ? &bpf_get_func_arg_proto : NULL;
> > + return &bpf_get_func_arg_proto;
> > case BPF_FUNC_get_func_ret:
> > return bpf_prog_has_trampoline(prog) ? &bpf_get_func_ret_proto : NULL;
> > case BPF_FUNC_get_func_arg_cnt:
> > - return bpf_prog_has_trampoline(prog) ? &bpf_get_func_arg_cnt_proto : NULL;
> > + return &bpf_get_func_arg_cnt_proto;
> > case BPF_FUNC_get_attach_cookie:
> > if (prog->type == BPF_PROG_TYPE_TRACING &&
> > prog->expected_attach_type == BPF_TRACE_RAW_TP)
>
>
^ permalink raw reply
* [PATCH v4 4/4] tracing/Documentation: Add a section about backup instance
From: Masami Hiramatsu (Google) @ 2026-01-20 1:09 UTC (permalink / raw)
To: Steven Rostedt
Cc: Masami Hiramatsu, Mathieu Desnoyers, linux-kernel,
linux-trace-kernel
In-Reply-To: <176887135615.578403.6988045330349053692.stgit@mhiramat.tok.corp.google.com>
From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Add a section about backup instance to the debugging.rst.
Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
Documentation/trace/debugging.rst | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/Documentation/trace/debugging.rst b/Documentation/trace/debugging.rst
index 4d88c346fc38..73e2154bcf98 100644
--- a/Documentation/trace/debugging.rst
+++ b/Documentation/trace/debugging.rst
@@ -159,3 +159,22 @@ If setting it from the kernel command line, it is recommended to also
disable tracing with the "traceoff" flag, and enable tracing after boot up.
Otherwise the trace from the most recent boot will be mixed with the trace
from the previous boot, and may make it confusing to read.
+
+Using a backup instance for keeping previous boot data
+------------------------------------------------------
+
+It is also possible to record trace data at system boot time by specifying
+events with the persistent ring buffer, but in this case the data before the
+reboot will be lost before it can be read. This problem can be solved by a
+backup instance. From the kernel command line::
+
+ reserve_mem=12M:4096:trace trace_instance=boot_map@trace,sched,irq trace_instance=backup=boot_map
+
+When the boot up, the previous data in the`boot_map` is copied to "backup"
+instance, and the "sched:*" and "irq:*" events for current boot are traced on
+"boot_map". Thus user can read the previous boot data from the "backup" instance
+without stopping trace.
+
+Note that this "backup" instance is readonly, and will be removed automatically
+if you clear the trace data or read out all trace data from "trace_pipe" or
+"trace_pipe_raw" files.
\ No newline at end of file
^ permalink raw reply related
* [PATCH v4 3/4] tracing: Remove the backup instance automatically after read
From: Masami Hiramatsu (Google) @ 2026-01-20 1:09 UTC (permalink / raw)
To: Steven Rostedt
Cc: Masami Hiramatsu, Mathieu Desnoyers, linux-kernel,
linux-trace-kernel
In-Reply-To: <176887135615.578403.6988045330349053692.stgit@mhiramat.tok.corp.google.com>
From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Since the backup instance is readonly, after reading all data
via pipe, no data is left on the instance. Thus it can be
removed safely after closing all files.
This also removes it if user resets the ring buffer manually
via 'trace' file.
Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
Changes in v4:
- Update description.
---
kernel/trace/trace.c | 64 +++++++++++++++++++++++++++++++++++++++++++++++++-
kernel/trace/trace.h | 6 +++++
2 files changed, 69 insertions(+), 1 deletion(-)
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index b27e1cdeffb0..7fa0809cd71b 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -590,6 +590,55 @@ void trace_set_ring_buffer_expanded(struct trace_array *tr)
tr->ring_buffer_expanded = true;
}
+static int __remove_instance(struct trace_array *tr);
+
+static void trace_array_autoremove(struct work_struct *work)
+{
+ struct trace_array *tr = container_of(work, struct trace_array, autoremove_work);
+
+ guard(mutex)(&event_mutex);
+ guard(mutex)(&trace_types_lock);
+
+ /*
+ * This can be fail if someone gets @tr before starting this
+ * function, but in that case, this will be kicked again when
+ * putting it. So we don't care the result.
+ */
+ __remove_instance(tr);
+}
+
+static struct workqueue_struct *autoremove_wq;
+
+static void trace_array_init_autoremove(struct trace_array *tr)
+{
+ INIT_WORK(&tr->autoremove_work, trace_array_autoremove);
+}
+
+static void trace_array_kick_autoremove(struct trace_array *tr)
+{
+ if (!work_pending(&tr->autoremove_work) && autoremove_wq)
+ queue_work(autoremove_wq, &tr->autoremove_work);
+}
+
+static void trace_array_cancel_autoremove(struct trace_array *tr)
+{
+ if (work_pending(&tr->autoremove_work))
+ cancel_work(&tr->autoremove_work);
+}
+
+__init static int trace_array_init_autoremove_wq(void)
+{
+ autoremove_wq = alloc_workqueue("tr_autoremove_wq",
+ WQ_UNBOUND | WQ_HIGHPRI, 0);
+ if (!autoremove_wq) {
+ pr_err("Unable to allocate tr_autoremove_wq\n");
+ return -ENOMEM;
+ }
+ return 0;
+}
+
+late_initcall_sync(trace_array_init_autoremove_wq);
+
LIST_HEAD(ftrace_trace_arrays);
int trace_array_get(struct trace_array *this_tr)
@@ -598,7 +647,7 @@ int trace_array_get(struct trace_array *this_tr)
guard(mutex)(&trace_types_lock);
list_for_each_entry(tr, &ftrace_trace_arrays, list) {
- if (tr == this_tr) {
+ if (tr == this_tr && !tr->free_on_close) {
tr->ref++;
return 0;
}
@@ -611,6 +660,12 @@ static void __trace_array_put(struct trace_array *this_tr)
{
WARN_ON(!this_tr->ref);
this_tr->ref--;
+ /*
+ * When free_on_close is set, prepare removing the array
+ * when the last reference is released.
+ */
+ if (this_tr->ref == 1 && this_tr->free_on_close)
+ trace_array_kick_autoremove(this_tr);
}
/**
@@ -6219,6 +6274,10 @@ static void update_last_data(struct trace_array *tr)
/* Only if the buffer has previous boot data clear and update it. */
tr->flags &= ~TRACE_ARRAY_FL_LAST_BOOT;
+ /* If this is a backup instance, mark it for autoremove. */
+ if (tr->flags & TRACE_ARRAY_FL_VMALLOC)
+ tr->free_on_close = true;
+
/* Reset the module list and reload them */
if (tr->scratch) {
struct trace_scratch *tscratch = tr->scratch;
@@ -10390,6 +10449,8 @@ trace_array_create_systems(const char *name, const char *systems,
if (ftrace_allocate_ftrace_ops(tr) < 0)
goto out_free_tr;
+ trace_array_init_autoremove(tr);
+
ftrace_init_trace_array(tr);
init_trace_flags_index(tr);
@@ -10538,6 +10599,7 @@ static int __remove_instance(struct trace_array *tr)
if (update_marker_trace(tr, 0))
synchronize_rcu();
+ trace_array_cancel_autoremove(tr);
tracing_set_nop(tr);
clear_ftrace_function_probes(tr);
event_trace_del_tracer(tr);
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index a098011951cc..947f641a9cf0 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -447,6 +447,12 @@ struct trace_array {
* we do not waste memory on systems that are not using tracing.
*/
bool ring_buffer_expanded;
+ /*
+ * If the ring buffer is a read only backup instance, it will be
+ * removed after dumping all data via pipe, because no readable data.
+ */
+ bool free_on_close;
+ struct work_struct autoremove_work;
};
enum {
^ permalink raw reply related
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