* Re: [PATCH v2] tracing/probe: reject empty immediate strings
From: Masami Hiramatsu @ 2026-04-04 0:33 UTC (permalink / raw)
To: Pengpeng Hou
Cc: rostedt, mhiramat, mathieu.desnoyers, linux-trace-kernel,
linux-kernel
In-Reply-To: <20260401160315.88518-1-pengpeng@iscas.ac.cn>
On Thu, 2 Apr 2026 00:03:15 +0800
Pengpeng Hou <pengpeng@iscas.ac.cn> wrote:
> parse_probe_arg() accepts quoted immediate strings and passes the body
> after the opening quote to __parse_imm_string(). That helper currently
> computes strlen(str) and immediately dereferences str[len - 1], which
> underflows when the body is empty.
>
> Reject empty immediate strings before checking for the closing quote.
>
Oops, good catch!
Let me pick it.
Thank you!
> Fixes: a42e3c4de964 ("tracing/probe: Add immediate string parameter support")
> Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
> ---
> Changes since v1:
> - resend as a standalone patch instead of part of an accidental
> cross-subsystem 1/4 series
>
> kernel/trace/trace_probe.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
> index e0a5dc86c07e..e1c73065dae5 100644
> --- a/kernel/trace/trace_probe.c
> +++ b/kernel/trace/trace_probe.c
> @@ -1068,7 +1068,7 @@ static int __parse_imm_string(char *str, char **pbuf, int offs)
> {
> size_t len = strlen(str);
>
> - if (str[len - 1] != '"') {
> + if (!len || str[len - 1] != '"') {
> trace_probe_log_err(offs + len, IMMSTR_NO_CLOSE);
> return -EINVAL;
> }
> --
> 2.50.1 (Apple Git-155)
>
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
^ permalink raw reply
* Re: [PATCH] kernel/trace: fixed static warnings
From: Abhijith Sriram @ 2026-04-04 6:03 UTC (permalink / raw)
To: Masami Hiramatsu
Cc: Steven Rostedt, Mathieu Desnoyers, open list:TRACING,
open list:TRACING
In-Reply-To: <20260404091806.4e1cf73d2775d128f1dc5277@kernel.org>
On Sat, Apr 4, 2026 at 2:18 AM Masami Hiramatsu <mhiramat@kernel.org> wrote:
>
> On Thu, 2 Apr 2026 21:54:04 +0200
> abhijithsriram95@gmail.com wrote:
>
> > From: Abhijith Sriram <abhijithsriram95@gmail.com>
> >
> > The change in the function argument description
> > was due to the static code checker script reading
> > the word filter back to back
> >
> > Signed-off-by: Abhijith Sriram <abhijithsriram95@gmail.com>
> > ---
> > kernel/trace/trace_events_trigger.c | 10 ++++++----
> > 1 file changed, 6 insertions(+), 4 deletions(-)
> >
> > diff --git a/kernel/trace/trace_events_trigger.c b/kernel/trace/trace_events_trigger.c
> > index 655db2e82513..477d8dee3362 100644
> > --- a/kernel/trace/trace_events_trigger.c
> > +++ b/kernel/trace/trace_events_trigger.c
> > @@ -246,7 +246,7 @@ event_triggers_post_call(struct trace_event_file *file,
> > }
> > EXPORT_SYMBOL_GPL(event_triggers_post_call);
> >
> > -#define SHOW_AVAILABLE_TRIGGERS (void *)(1UL)
> > +#define SHOW_AVAILABLE_TRIGGERS ((void *)(1UL))
>
> This is OK.
>
> >
> > static void *trigger_next(struct seq_file *m, void *t, loff_t *pos)
> > {
> > @@ -325,6 +325,7 @@ static const struct seq_operations event_triggers_seq_ops = {
> > static int event_trigger_regex_open(struct inode *inode, struct file *file)
> > {
> > int ret;
> > + struct seq_file *m = NULL;
> >
> > ret = security_locked_down(LOCKDOWN_TRACEFS);
> > if (ret)
> > @@ -351,7 +352,7 @@ static int event_trigger_regex_open(struct inode *inode, struct file *file)
> > if (file->f_mode & FMODE_READ) {
> > ret = seq_open(file, &event_triggers_seq_ops);
> > if (!ret) {
> > - struct seq_file *m = file->private_data;
> > + *m = file->private_data;
>
> Why is this change required?
The original warning says "missing blank line after declaration". I
thought it was cleaner to have the
declaration in the beginning of the function. I made a mistake here
which I fixed in the version 2 of
the patch, please have a look here:
https://lore.kernel.org/linux-trace-kernel/20260403071108.23422-2-abhijithsriram95@gmail.com/T/#u
>
> > m->private = file;
> > }
> > }
> > @@ -388,9 +389,9 @@ static ssize_t event_trigger_regex_write(struct file *file,
> > const char __user *ubuf,
> > size_t cnt, loff_t *ppos)
> > {
> > + char *buf __free(kfree) = NULL;
> > struct trace_event_file *event_file;
> > ssize_t ret;
> > - char *buf __free(kfree) = NULL;
>
> What is this change?
The same missing blank lines after declaration was triggered here,
even though there is a blank line after the char *buf.
If I do give an empty line then there is another error "Trailing white
space". So I reordered it and the warning disappeared.
This change I am not super sure since it is usually recommended that
variables of larger size are declared first
for padding purposes. What do you think?
>
> Thanks,
>
> >
> > if (!cnt)
> > return 0;
> > @@ -633,6 +634,7 @@ clear_event_triggers(struct trace_array *tr)
> >
> > list_for_each_entry(file, &tr->events, list) {
> > struct event_trigger_data *data, *n;
> > +
> > list_for_each_entry_safe(data, n, &file->triggers, list) {
> > trace_event_trigger_enable_disable(file, 0);
> > list_del_rcu(&data->list);
> > @@ -785,7 +787,7 @@ static void unregister_trigger(char *glob,
> > * cmd - the trigger command name
> > * glob - the trigger command name optionally prefaced with '!'
> > * param_and_filter - text following cmd and ':'
> > - * param - text following cmd and ':' and stripped of filter
> > + * param - text following cmd and ':' and filter removed
> > * filter - the optional filter text following (and including) 'if'
> > *
> > * To illustrate the use of these components, here are some concrete
> > --
> > 2.43.0
> >
>
>
> --
> Masami Hiramatsu (Google) <mhiramat@kernel.org>
--
Regards
Abhijith Sriram
^ permalink raw reply
* [PATCH] tracefs: fix default permissions not being applied on initial mount
From: David Carlier @ 2026-04-04 13:47 UTC (permalink / raw)
To: rostedt, mhiramat
Cc: mathieu.desnoyers, kaleshsingh, linux-trace-kernel, linux-kernel,
stable, David Carlier
Commit e4d32142d1de ("tracing: Fix tracefs mount options") moved the
option application from tracefs_fill_super() to tracefs_reconfigure()
called from tracefs_get_tree(). This fixed mount options being ignored
on user-space mounts when the superblock already exists, but introduced
a regression for the initial kernel-internal mount.
On the first mount (via simple_pin_fs during init), sget_fc() transfers
fc->s_fs_info to sb->s_fs_info and sets fc->s_fs_info to NULL. When
tracefs_get_tree() then calls tracefs_reconfigure(), it sees a NULL
fc->s_fs_info and returns early without applying any options. The root
inode keeps mode 0755 from simple_fill_super() instead of the intended
TRACEFS_DEFAULT_MODE (0700).
Furthermore, even on subsequent user-space mounts without an explicit
mode= option, tracefs_apply_options(sb, true) gates the mode behind
fsi->opts & BIT(Opt_mode), which is unset for the defaults. So the
mode is never corrected unless the user explicitly passes mode=0700.
Restore the tracefs_apply_options(sb, false) call in tracefs_fill_super()
to apply default permissions on initial superblock creation, matching
what debugfs does in debugfs_fill_super().
Fixes: e4d32142d1de ("tracing: Fix tracefs mount options")
Cc: stable@vger.kernel.org
Signed-off-by: David Carlier <devnexen@gmail.com>
---
fs/tracefs/inode.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/fs/tracefs/inode.c b/fs/tracefs/inode.c
index edfdf139cb02..36058ba5ee48 100644
--- a/fs/tracefs/inode.c
+++ b/fs/tracefs/inode.c
@@ -491,6 +491,7 @@ static int tracefs_fill_super(struct super_block *sb, struct fs_context *fc)
return err;
sb->s_op = &tracefs_super_operations;
+ tracefs_apply_options(sb, false);
set_default_d_op(sb, &tracefs_dentry_operations);
return 0;
--
2.53.0
^ permalink raw reply related
* Re: [PATCH v5 0/3] PCI Controller event and LTSSM tracepoint support
From: Manivannan Sadhasivam @ 2026-04-04 16:53 UTC (permalink / raw)
To: Steven Rostedt, Shawn Lin
Cc: Bjorn Helgaas, linux-rockchip, linux-pci, linux-trace-kernel,
linux-doc
In-Reply-To: <1774403912-210670-1-git-send-email-shawn.lin@rock-chips.com>
On Wed, Mar 25, 2026 at 09:58:29AM +0800, Shawn Lin wrote:
>
> This patch-set adds new pci controller event and LTSSM tracepoint used by host drivers
> which provide LTSSM trace functionality. The first user is pcie-dw-rockchip with a 256
> Bytes FIFO for recording LTSSM transition.
>
Steve, could you please take a look at the tracing part?
- Mani
> Testing
> =========
>
> This series was tested on RK3588/RK3588s EVB1 with NVMe SSD connected to PCIe3 and PCIe2
> root ports.
>
> echo 1 > /sys/kernel/debug/tracing/events/pci_controller/pcie_ltssm_state_transition/enable
> cat /sys/kernel/debug/tracing/trace_pipe
>
> # tracer: nop
> #
> # entries-in-buffer/entries-written: 64/64 #P:8
> #
> # _-----=> irqs-off/BH-disabled
> # / _----=> need-resched
> # | / _---=> hardirq/softirq
> # || / _--=> preempt-depth
> # ||| / _-=> migrate-disable
> # |||| / delay
> # TASK-PID CPU# ||||| TIMESTAMP FUNCTION
> # | | | ||||| | |
> kworker/0:0-9 [000] ..... 5.600194: pcie_ltssm_state_transition: dev: a40000000.pcie state: DETECT_ACT rate: Unknown
> kworker/0:0-9 [000] ..... 5.600198: pcie_ltssm_state_transition: dev: a40000000.pcie state: DETECT_WAIT rate: Unknown
> kworker/0:0-9 [000] ..... 5.600199: pcie_ltssm_state_transition: dev: a40000000.pcie state: DETECT_ACT rate: Unknown
> kworker/0:0-9 [000] ..... 5.600201: pcie_ltssm_state_transition: dev: a40000000.pcie state: POLL_ACTIVE rate: Unknown
> kworker/0:0-9 [000] ..... 5.600202: pcie_ltssm_state_transition: dev: a40000000.pcie state: POLL_CONFIG rate: Unknown
> kworker/0:0-9 [000] ..... 5.600204: pcie_ltssm_state_transition: dev: a40000000.pcie state: CFG_LINKWD_START rate: Unknown
> kworker/0:0-9 [000] ..... 5.600206: pcie_ltssm_state_transition: dev: a40000000.pcie state: CFG_LINKWD_ACEPT rate: Unknown
> kworker/0:0-9 [000] ..... 5.600207: pcie_ltssm_state_transition: dev: a40000000.pcie state: CFG_LANENUM_WAI rate: Unknown
> kworker/0:0-9 [000] ..... 5.600208: pcie_ltssm_state_transition: dev: a40000000.pcie state: CFG_LANENUM_ACEPT rate: Unknown
> kworker/0:0-9 [000] ..... 5.600210: pcie_ltssm_state_transition: dev: a40000000.pcie state: CFG_COMPLETE rate: Unknown
> kworker/0:0-9 [000] ..... 5.600212: pcie_ltssm_state_transition: dev: a40000000.pcie state: CFG_IDLE rate: Unknown
> kworker/0:0-9 [000] ..... 5.600213: pcie_ltssm_state_transition: dev: a40000000.pcie state: L0 rate: 2.5 GT/s
> kworker/0:0-9 [000] ..... 5.600214: pcie_ltssm_state_transition: dev: a40000000.pcie state: RCVRY_LOCK rate: Unknown
> kworker/0:0-9 [000] ..... 5.600216: pcie_ltssm_state_transition: dev: a40000000.pcie state: RCVRY_RCVRCFG rate: Unknown
> kworker/0:0-9 [000] ..... 5.600217: pcie_ltssm_state_transition: dev: a40000000.pcie state: RCVRY_SPEED rate: Unknown
> kworker/0:0-9 [000] ..... 5.600218: pcie_ltssm_state_transition: dev: a40000000.pcie state: RCVRY_LOCK rate: Unknown
> kworker/0:0-9 [000] ..... 5.600220: pcie_ltssm_state_transition: dev: a40000000.pcie state: RCVRY_EQ1 rate: Unknown
> kworker/0:0-9 [000] ..... 5.600221: pcie_ltssm_state_transition: dev: a40000000.pcie state: RCVRY_EQ2 rate: 8.0 GT/s
> kworker/0:0-9 [000] ..... 5.600222: pcie_ltssm_state_transition: dev: a40000000.pcie state: RCVRY_EQ3 rate: 8.0 GT/s
> kworker/0:0-9 [000] ..... 5.600224: pcie_ltssm_state_transition: dev: a40000000.pcie state: RCVRY_LOCK rate: 8.0 GT/s
> kworker/0:0-9 [000] ..... 5.600225: pcie_ltssm_state_transition: dev: a40000000.pcie state: RCVRY_RCVRCFG rate: 8.0 GT/s
> kworker/0:0-9 [000] ..... 5.600226: pcie_ltssm_state_transition: dev: a40000000.pcie state: RCVRY_IDLE rate: 8.0 GT/s
> kworker/0:0-9 [000] ..... 5.600227: pcie_ltssm_state_transition: dev: a40000000.pcie state: L0 rate: 8.0 GT/s
> kworker/0:0-9 [000] ..... 5.600228: pcie_ltssm_state_transition: dev: a40000000.pcie state: RCVRY_LOCK rate: 8.0 GT/s
> kworker/0:0-9 [000] ..... 5.600229: pcie_ltssm_state_transition: dev: a40000000.pcie state: RCVRY_RCVRCFG rate: 8.0 GT/s
> kworker/0:0-9 [000] ..... 5.600231: pcie_ltssm_state_transition: dev: a40000000.pcie state: RCVRY_IDLE rate: 8.0 GT/s
> kworker/0:0-9 [000] ..... 5.600232: pcie_ltssm_state_transition: dev: a40000000.pcie state: L0 rate: 8.0 GT/s
> kworker/0:0-9 [000] ..... 5.600233: pcie_ltssm_state_transition: dev: a40000000.pcie state: L123_SEND_EIDLE rate: 8.0 GT/s
> kworker/0:0-9 [000] ..... 5.600234: pcie_ltssm_state_transition: dev: a40000000.pcie state: L1_IDLE rate: 8.0 GT/s
> kworker/0:0-9 [000] ..... 5.600236: pcie_ltssm_state_transition: dev: a40000000.pcie state: RCVRY_LOCK rate: 8.0 GT/s
> kworker/0:0-9 [000] ..... 5.600237: pcie_ltssm_state_transition: dev: a40000000.pcie state: RCVRY_RCVRCFG rate: 8.0 GT/s
> kworker/0:0-9 [000] ..... 5.600238: pcie_ltssm_state_transition: dev: a40000000.pcie state: RCVRY_IDLE rate: 8.0 GT/s
> kworker/0:0-9 [000] ..... 5.600239: pcie_ltssm_state_transition: dev: a40000000.pcie state: L0 rate: 8.0 GT/s
>
>
> Changes in v5:
> - rebase
> - use EM/EMe instead
> - remove reg/unreg function and back to use TRACE_EVENT
> - use trace_pcie_ltssm_state_transition_enabled()
>
> Changes in v4:
> - use TRACE_EVENT_FN to notify when to start and stop the tracepoint,
> and export pci_ltssm_tp_enabled() for host drivers to use
> - skip trace if pci_ltssm_tp_enabled() is false.(Steven)
> - wrap into 80 columns(Bjorn)
>
> Changes in v3:
> - add TRACE_DEFINE_ENUM for all enums(Steven Rostedt)
> - Add toctree entry in Documentation/trace/index.rst(Bagas Sanjaya)
> - fix mismatch section underline length(Bagas Sanjaya)
> - Make example snippets in code block(Bagas Sanjaya)
> - warp context into 80 columns and fix the file name(Bjorn)
> - reorder variables(Mani)
> - rename loop to i; rename en to enable(Mani)
> - use FIELD_GET(Mani)
> - add comment about how the FIFO works(Mani)
>
> Changes in v2:
> - use tracepoint
>
> Shawn Lin (3):
> PCI: trace: Add PCI controller LTSSM transition tracepoint
> Documentation: tracing: Add PCI controller event documentation
> PCI: dw-rockchip: Add pcie_ltssm_state_transition trace support
>
> Documentation/trace/events-pci-controller.rst | 42 ++++++++++
> Documentation/trace/index.rst | 1 +
> drivers/pci/controller/dwc/pcie-dw-rockchip.c | 111 ++++++++++++++++++++++++++
> drivers/pci/trace.c | 1 +
> include/trace/events/pci_controller.h | 58 ++++++++++++++
> 5 files changed, 213 insertions(+)
> create mode 100644 Documentation/trace/events-pci-controller.rst
> create mode 100644 include/trace/events/pci_controller.h
>
> --
> 2.7.4
>
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply
* [PATCH v1] Documentation/rtla: Convert links to RST format
From: Costa Shulyupin @ 2026-04-05 16:38 UTC (permalink / raw)
To: Steven Rostedt, Tomas Glozar, Jonathan Corbet, Shuah Khan,
linux-trace-kernel, linux-kernel, linux-doc
Cc: Costa Shulyupin
Web links in the documentation are not properly displayed.
In the man pages web links look like:
Osnoise tracer documentation: < <https://www.kernel.org/doc/html/lat‐
est/trace/osnoise-tracer.html> >
On web pages the URL caption is the URL itself.
Convert tracer documentation links to RST anonymous hyperlink format
for better rendering. Use newer docs.kernel.org instead of
www.kernel.org/doc/html/latest for brevity.
After the change, the links in the man pages look like:
Osnoise tracer <https://docs.kernel.org/trace/osnoise-tracer.html>
On web pages the captions are the titles of the links.
Signed-off-by: Costa Shulyupin <costa.shul@redhat.com>
---
Documentation/tools/rtla/rtla-hwnoise.rst | 2 +-
Documentation/tools/rtla/rtla-osnoise-hist.rst | 2 +-
Documentation/tools/rtla/rtla-osnoise-top.rst | 2 +-
Documentation/tools/rtla/rtla-osnoise.rst | 2 +-
Documentation/tools/rtla/rtla-timerlat-hist.rst | 2 +-
Documentation/tools/rtla/rtla-timerlat-top.rst | 2 +-
Documentation/tools/rtla/rtla-timerlat.rst | 2 +-
7 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/Documentation/tools/rtla/rtla-hwnoise.rst b/Documentation/tools/rtla/rtla-hwnoise.rst
index 26512b15fe7b..5930bbca4522 100644
--- a/Documentation/tools/rtla/rtla-hwnoise.rst
+++ b/Documentation/tools/rtla/rtla-hwnoise.rst
@@ -100,7 +100,7 @@ SEE ALSO
**rtla-osnoise**\(1)
-Osnoise tracer documentation: <https://www.kernel.org/doc/html/latest/trace/osnoise-tracer.html>
+`Osnoise tracer <https://docs.kernel.org/trace/osnoise-tracer.html>`__
AUTHOR
======
diff --git a/Documentation/tools/rtla/rtla-osnoise-hist.rst b/Documentation/tools/rtla/rtla-osnoise-hist.rst
index 007521c865d9..6ddea2c6d490 100644
--- a/Documentation/tools/rtla/rtla-osnoise-hist.rst
+++ b/Documentation/tools/rtla/rtla-osnoise-hist.rst
@@ -59,7 +59,7 @@ SEE ALSO
========
**rtla-osnoise**\(1), **rtla-osnoise-top**\(1)
-*osnoise* tracer documentation: <https://www.kernel.org/doc/html/latest/trace/osnoise-tracer.html>
+`Osnoise tracer <https://docs.kernel.org/trace/osnoise-tracer.html>`__
AUTHOR
======
diff --git a/Documentation/tools/rtla/rtla-osnoise-top.rst b/Documentation/tools/rtla/rtla-osnoise-top.rst
index 6ccadae38945..b91c02ac2bbe 100644
--- a/Documentation/tools/rtla/rtla-osnoise-top.rst
+++ b/Documentation/tools/rtla/rtla-osnoise-top.rst
@@ -54,7 +54,7 @@ SEE ALSO
**rtla-osnoise**\(1), **rtla-osnoise-hist**\(1)
-Osnoise tracer documentation: <https://www.kernel.org/doc/html/latest/trace/osnoise-tracer.html>
+`Osnoise tracer <https://docs.kernel.org/trace/osnoise-tracer.html>`__
AUTHOR
======
diff --git a/Documentation/tools/rtla/rtla-osnoise.rst b/Documentation/tools/rtla/rtla-osnoise.rst
index 540d2bf6c152..decd9e11fcf2 100644
--- a/Documentation/tools/rtla/rtla-osnoise.rst
+++ b/Documentation/tools/rtla/rtla-osnoise.rst
@@ -50,7 +50,7 @@ SEE ALSO
========
**rtla-osnoise-top**\(1), **rtla-osnoise-hist**\(1)
-Osnoise tracer documentation: <https://www.kernel.org/doc/html/latest/trace/osnoise-tracer.html>
+`Osnoise tracer <https://docs.kernel.org/trace/osnoise-tracer.html>`__
AUTHOR
======
diff --git a/Documentation/tools/rtla/rtla-timerlat-hist.rst b/Documentation/tools/rtla/rtla-timerlat-hist.rst
index f56fe546411b..dab75677b06e 100644
--- a/Documentation/tools/rtla/rtla-timerlat-hist.rst
+++ b/Documentation/tools/rtla/rtla-timerlat-hist.rst
@@ -104,7 +104,7 @@ SEE ALSO
========
**rtla-timerlat**\(1), **rtla-timerlat-top**\(1)
-*timerlat* tracer documentation: <https://www.kernel.org/doc/html/latest/trace/timerlat-tracer.html>
+`Timerlat tracer <https://docs.kernel.org/trace/timerlat-tracer.html>`__
AUTHOR
======
diff --git a/Documentation/tools/rtla/rtla-timerlat-top.rst b/Documentation/tools/rtla/rtla-timerlat-top.rst
index 72d85e36c193..05959f1a4661 100644
--- a/Documentation/tools/rtla/rtla-timerlat-top.rst
+++ b/Documentation/tools/rtla/rtla-timerlat-top.rst
@@ -127,7 +127,7 @@ SEE ALSO
--------
**rtla-timerlat**\(1), **rtla-timerlat-hist**\(1)
-*timerlat* tracer documentation: <https://www.kernel.org/doc/html/latest/trace/timerlat-tracer.html>
+`Timerlat tracer <https://docs.kernel.org/trace/timerlat-tracer.html>`__
AUTHOR
------
diff --git a/Documentation/tools/rtla/rtla-timerlat.rst b/Documentation/tools/rtla/rtla-timerlat.rst
index ce9f57e038c3..63718c52aa3f 100644
--- a/Documentation/tools/rtla/rtla-timerlat.rst
+++ b/Documentation/tools/rtla/rtla-timerlat.rst
@@ -45,7 +45,7 @@ SEE ALSO
========
**rtla-timerlat-top**\(1), **rtla-timerlat-hist**\(1)
-*timerlat* tracer documentation: <https://www.kernel.org/doc/html/latest/trace/timerlat-tracer.html>
+`Timerlat tracer <https://docs.kernel.org/trace/timerlat-tracer.html>`__
AUTHOR
======
--
2.53.0
^ permalink raw reply related
* Re: [PATCH v2] tracing/probe: reject empty immediate strings
From: Masami Hiramatsu @ 2026-04-06 1:11 UTC (permalink / raw)
To: Masami Hiramatsu
Cc: Pengpeng Hou, rostedt, mathieu.desnoyers, linux-trace-kernel,
linux-kernel
In-Reply-To: <20260404093359.e3bc3f7ededeba6a73858e5d@kernel.org>
Hi,
On Sat, 4 Apr 2026 09:33:59 +0900
Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote:
> > diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
> > index e0a5dc86c07e..e1c73065dae5 100644
> > --- a/kernel/trace/trace_probe.c
> > +++ b/kernel/trace/trace_probe.c
> > @@ -1068,7 +1068,7 @@ static int __parse_imm_string(char *str, char **pbuf, int offs)
> > {
> > size_t len = strlen(str);
> >
> > - if (str[len - 1] != '"') {
It seems that this is not correct fix, because __parse_imm_string()
is only called from below code:
case '\\': /* Immediate value */
if (arg[1] == '"') { /* Immediate string */
ret = __parse_imm_string(arg + 2, &tmp, ctx->offset + 2);
if (ret)
Thus the call-site already checked the double-quotation.
This means this if block itself is meaningless.
Thanks,
> > + if (!len || str[len - 1] != '"') {
> > trace_probe_log_err(offs + len, IMMSTR_NO_CLOSE);
> > return -EINVAL;
> > }
> > --
> > 2.50.1 (Apple Git-155)
> >
>
>
> --
> Masami Hiramatsu (Google) <mhiramat@kernel.org>
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
^ permalink raw reply
* Re: [PATCH v2] tracing/probe: reject empty immediate strings
From: Masami Hiramatsu @ 2026-04-06 1:20 UTC (permalink / raw)
To: Masami Hiramatsu
Cc: Pengpeng Hou, rostedt, mathieu.desnoyers, linux-trace-kernel,
linux-kernel
In-Reply-To: <20260406101124.e4e5af12634fb1136cd7d132@kernel.org>
On Mon, 6 Apr 2026 10:11:24 +0900
Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote:
> Hi,
>
> On Sat, 4 Apr 2026 09:33:59 +0900
> Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote:
>
> > > diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
> > > index e0a5dc86c07e..e1c73065dae5 100644
> > > --- a/kernel/trace/trace_probe.c
> > > +++ b/kernel/trace/trace_probe.c
> > > @@ -1068,7 +1068,7 @@ static int __parse_imm_string(char *str, char **pbuf, int offs)
> > > {
> > > size_t len = strlen(str);
> > >
> > > - if (str[len - 1] != '"') {
>
> It seems that this is not correct fix, because __parse_imm_string()
> is only called from below code:
>
> case '\\': /* Immediate value */
> if (arg[1] == '"') { /* Immediate string */
> ret = __parse_imm_string(arg + 2, &tmp, ctx->offset + 2);
> if (ret)
>
> Thus the call-site already checked the double-quotation.
> This means this if block itself is meaningless.
Nevermind, this fix is correct. But the title is not correct because
we still can specify an empty string (\""), but this rejects non-closed
empty immediate string (\").
Thank you,
>
> Thanks,
>
> > > + if (!len || str[len - 1] != '"') {
> > > trace_probe_log_err(offs + len, IMMSTR_NO_CLOSE);
> > > return -EINVAL;
> > > }
> > > --
> > > 2.50.1 (Apple Git-155)
> > >
> >
> >
> > --
> > Masami Hiramatsu (Google) <mhiramat@kernel.org>
>
>
> --
> Masami Hiramatsu (Google) <mhiramat@kernel.org>
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
^ permalink raw reply
* Re: [PATCH v2] tracing/probe: reject empty immediate strings
From: Masami Hiramatsu @ 2026-04-06 1:24 UTC (permalink / raw)
To: Pengpeng Hou
Cc: rostedt, mhiramat, mathieu.desnoyers, linux-trace-kernel,
linux-kernel
In-Reply-To: <20260401160315.88518-1-pengpeng@iscas.ac.cn>
I would like to suggest the subject as
"tracing/probe: Reject non-closed empty immediate strings"
On Thu, 2 Apr 2026 00:03:15 +0800
Pengpeng Hou <pengpeng@iscas.ac.cn> wrote:
> parse_probe_arg() accepts quoted immediate strings and passes the body
> after the opening quote to __parse_imm_string(). That helper currently
> computes strlen(str) and immediately dereferences str[len - 1], which
> underflows when the body is empty.
^ and not closed with double-quotation.
>
> Reject empty immediate strings before checking for the closing quote.
^ non-closed.
Thank you,
>
> Fixes: a42e3c4de964 ("tracing/probe: Add immediate string parameter support")
> Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
> ---
> Changes since v1:
> - resend as a standalone patch instead of part of an accidental
> cross-subsystem 1/4 series
>
> kernel/trace/trace_probe.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
> index e0a5dc86c07e..e1c73065dae5 100644
> --- a/kernel/trace/trace_probe.c
> +++ b/kernel/trace/trace_probe.c
> @@ -1068,7 +1068,7 @@ static int __parse_imm_string(char *str, char **pbuf, int offs)
> {
> size_t len = strlen(str);
>
> - if (str[len - 1] != '"') {
> + if (!len || str[len - 1] != '"') {
> trace_probe_log_err(offs + len, IMMSTR_NO_CLOSE);
> return -EINVAL;
> }
> --
> 2.50.1 (Apple Git-155)
>
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
^ permalink raw reply
* Re: [PATCH] kernel/trace: fixed static warnings
From: Masami Hiramatsu @ 2026-04-06 2:00 UTC (permalink / raw)
To: Abhijith Sriram
Cc: Steven Rostedt, Mathieu Desnoyers, open list:TRACING,
open list:TRACING
In-Reply-To: <CAMTgpFfAmzXxjtZXbtLyZL62dAWOmyhNkwJLkgQy7Lcv+VrjLA@mail.gmail.com>
On Sat, 4 Apr 2026 08:03:07 +0200
Abhijith Sriram <abhijithsriram95@gmail.com> wrote:
> On Sat, Apr 4, 2026 at 2:18 AM Masami Hiramatsu <mhiramat@kernel.org> wrote:
> >
> > On Thu, 2 Apr 2026 21:54:04 +0200
> > abhijithsriram95@gmail.com wrote:
> >
> > > From: Abhijith Sriram <abhijithsriram95@gmail.com>
> > >
> > > The change in the function argument description
> > > was due to the static code checker script reading
> > > the word filter back to back
> > >
> > > Signed-off-by: Abhijith Sriram <abhijithsriram95@gmail.com>
> > > ---
> > > kernel/trace/trace_events_trigger.c | 10 ++++++----
> > > 1 file changed, 6 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/kernel/trace/trace_events_trigger.c b/kernel/trace/trace_events_trigger.c
> > > index 655db2e82513..477d8dee3362 100644
> > > --- a/kernel/trace/trace_events_trigger.c
> > > +++ b/kernel/trace/trace_events_trigger.c
> > > @@ -246,7 +246,7 @@ event_triggers_post_call(struct trace_event_file *file,
> > > }
> > > EXPORT_SYMBOL_GPL(event_triggers_post_call);
> > >
> > > -#define SHOW_AVAILABLE_TRIGGERS (void *)(1UL)
> > > +#define SHOW_AVAILABLE_TRIGGERS ((void *)(1UL))
> >
> > This is OK.
> >
> > >
> > > static void *trigger_next(struct seq_file *m, void *t, loff_t *pos)
> > > {
> > > @@ -325,6 +325,7 @@ static const struct seq_operations event_triggers_seq_ops = {
> > > static int event_trigger_regex_open(struct inode *inode, struct file *file)
> > > {
> > > int ret;
> > > + struct seq_file *m = NULL;
> > >
> > > ret = security_locked_down(LOCKDOWN_TRACEFS);
> > > if (ret)
> > > @@ -351,7 +352,7 @@ static int event_trigger_regex_open(struct inode *inode, struct file *file)
> > > if (file->f_mode & FMODE_READ) {
> > > ret = seq_open(file, &event_triggers_seq_ops);
> > > if (!ret) {
> > > - struct seq_file *m = file->private_data;
> > > + *m = file->private_data;
> >
> > Why is this change required?
> The original warning says "missing blank line after declaration". I
> thought it was cleaner to have the
> declaration in the beginning of the function. I made a mistake here
> which I fixed in the version 2 of
> the patch, please have a look here:
> https://lore.kernel.org/linux-trace-kernel/20260403071108.23422-2-abhijithsriram95@gmail.com/T/#u
In that case, you just need to add an empty line, no need to move the
definition becuase it changes the scope of `m` variable.
> >
> > > m->private = file;
> > > }
> > > }
> > > @@ -388,9 +389,9 @@ static ssize_t event_trigger_regex_write(struct file *file,
> > > const char __user *ubuf,
> > > size_t cnt, loff_t *ppos)
> > > {
> > > + char *buf __free(kfree) = NULL;
> > > struct trace_event_file *event_file;
> > > ssize_t ret;
> > > - char *buf __free(kfree) = NULL;
> >
> > What is this change?
> The same missing blank lines after declaration was triggered here,
> even though there is a blank line after the char *buf.
> If I do give an empty line then there is another error "Trailing white
> space". So I reordered it and the warning disappeared.
> This change I am not super sure since it is usually recommended that
> variables of larger size are declared first
> for padding purposes. What do you think?
Ah, that is a known checkpatch's bug. It does not understand __free()
macro. So please ignore that error (or/and fix checkpatch.pl).
Thanks,
> >
> > Thanks,
> >
> > >
> > > if (!cnt)
> > > return 0;
> > > @@ -633,6 +634,7 @@ clear_event_triggers(struct trace_array *tr)
> > >
> > > list_for_each_entry(file, &tr->events, list) {
> > > struct event_trigger_data *data, *n;
> > > +
> > > list_for_each_entry_safe(data, n, &file->triggers, list) {
> > > trace_event_trigger_enable_disable(file, 0);
> > > list_del_rcu(&data->list);
> > > @@ -785,7 +787,7 @@ static void unregister_trigger(char *glob,
> > > * cmd - the trigger command name
> > > * glob - the trigger command name optionally prefaced with '!'
> > > * param_and_filter - text following cmd and ':'
> > > - * param - text following cmd and ':' and stripped of filter
> > > + * param - text following cmd and ':' and filter removed
> > > * filter - the optional filter text following (and including) 'if'
> > > *
> > > * To illustrate the use of these components, here are some concrete
> > > --
> > > 2.43.0
> > >
> >
> >
> > --
> > Masami Hiramatsu (Google) <mhiramat@kernel.org>
> --
> Regards
> Abhijith Sriram
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
^ permalink raw reply
* Re: [PATCH v2] tracing/probe: reject empty immediate strings
From: Pengpeng Hou @ 2026-04-06 8:20 UTC (permalink / raw)
To: Steven Rostedt
Cc: Masami Hiramatsu, Mathieu Desnoyers, linux-trace-kernel,
linux-kernel, pengpeng
In-Reply-To: <20260401160315.88518-1-pengpeng@iscas.ac.cn>
Hi Masami,
Thanks, you're right.
The fix is for a non-closed empty immediate string, not for an empty
immediate string in general. Your suggested subject and wording are more
accurate.
Please feel free to adjust the subject/changelog that way when applying.
If you would prefer a respin from my side, I can resend it.
Thanks,
Pengpeng
^ permalink raw reply
* [PATCH] seq_buf: add KUnit tests for seq_buf_putmem_hex()
From: Shuvam Pandey @ 2026-04-06 3:37 UTC (permalink / raw)
To: Andrew Morton, Steven Rostedt
Cc: Masami Hiramatsu, Mathieu Desnoyers, linux-kernel,
linux-trace-kernel, Shuvam Pandey
The seq_buf KUnit suite does not exercise seq_buf_putmem_hex().
Add one test for the len > 8 chunking path and one overflow test
where a later chunk no longer fits in the buffer.
Signed-off-by: Shuvam Pandey <shuvampandey1@gmail.com>
---
lib/tests/seq_buf_kunit.c | 34 ++++++++++++++++++++++++++++++++++
1 file changed, 34 insertions(+)
diff --git a/lib/tests/seq_buf_kunit.c b/lib/tests/seq_buf_kunit.c
index 8a01579a9..eb466386b 100644
--- a/lib/tests/seq_buf_kunit.c
+++ b/lib/tests/seq_buf_kunit.c
@@ -184,6 +184,38 @@ static void seq_buf_get_buf_commit_test(struct kunit *test)
KUNIT_EXPECT_TRUE(test, seq_buf_has_overflowed(&s));
}
+static void seq_buf_putmem_hex_test(struct kunit *test)
+{
+ DECLARE_SEQ_BUF(s, 24);
+ const u8 data[] = { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9 };
+#ifdef __BIG_ENDIAN
+ const char *expected = "0001020304050607 0809 ";
+#else
+ const char *expected = "0706050403020100 0908 ";
+#endif
+
+ KUNIT_EXPECT_EQ(test, seq_buf_putmem_hex(&s, data, sizeof(data)), 0);
+ KUNIT_EXPECT_FALSE(test, seq_buf_has_overflowed(&s));
+ KUNIT_EXPECT_EQ(test, seq_buf_used(&s), strlen(expected));
+ KUNIT_EXPECT_STREQ(test, seq_buf_str(&s), expected);
+}
+
+static void seq_buf_putmem_hex_overflow_test(struct kunit *test)
+{
+ DECLARE_SEQ_BUF(s, 20);
+ const u8 data[] = { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9 };
+#ifdef __BIG_ENDIAN
+ const char *expected = "0001020304050607 ";
+#else
+ const char *expected = "0706050403020100 ";
+#endif
+
+ KUNIT_EXPECT_EQ(test, seq_buf_putmem_hex(&s, data, sizeof(data)), -1);
+ KUNIT_EXPECT_TRUE(test, seq_buf_has_overflowed(&s));
+ KUNIT_EXPECT_EQ(test, seq_buf_used(&s), 20);
+ KUNIT_EXPECT_STREQ(test, seq_buf_str(&s), expected);
+}
+
static struct kunit_case seq_buf_test_cases[] = {
KUNIT_CASE(seq_buf_init_test),
KUNIT_CASE(seq_buf_declare_test),
@@ -194,6 +226,8 @@ static struct kunit_case seq_buf_test_cases[] = {
KUNIT_CASE(seq_buf_printf_test),
KUNIT_CASE(seq_buf_printf_overflow_test),
KUNIT_CASE(seq_buf_get_buf_commit_test),
+ KUNIT_CASE(seq_buf_putmem_hex_test),
+ KUNIT_CASE(seq_buf_putmem_hex_overflow_test),
{}
};
--
2.50.0
^ permalink raw reply related
* [PATCH] seq_buf: add KUnit tests for seq_buf_putmem_hex()
From: Shuvam Pandey @ 2026-04-06 3:37 UTC (permalink / raw)
To: Andrew Morton, Steven Rostedt
Cc: Masami Hiramatsu, Mathieu Desnoyers, linux-kernel,
linux-trace-kernel, Shuvam Pandey
The seq_buf KUnit suite does not exercise seq_buf_putmem_hex().
Add one test for the len > 8 chunking path and one overflow test
where a later chunk no longer fits in the buffer.
Signed-off-by: Shuvam Pandey <shuvampandey1@gmail.com>
---
lib/tests/seq_buf_kunit.c | 34 ++++++++++++++++++++++++++++++++++
1 file changed, 34 insertions(+)
diff --git a/lib/tests/seq_buf_kunit.c b/lib/tests/seq_buf_kunit.c
index 8a01579a9..eb466386b 100644
--- a/lib/tests/seq_buf_kunit.c
+++ b/lib/tests/seq_buf_kunit.c
@@ -184,6 +184,38 @@ static void seq_buf_get_buf_commit_test(struct kunit *test)
KUNIT_EXPECT_TRUE(test, seq_buf_has_overflowed(&s));
}
+static void seq_buf_putmem_hex_test(struct kunit *test)
+{
+ DECLARE_SEQ_BUF(s, 24);
+ const u8 data[] = { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9 };
+#ifdef __BIG_ENDIAN
+ const char *expected = "0001020304050607 0809 ";
+#else
+ const char *expected = "0706050403020100 0908 ";
+#endif
+
+ KUNIT_EXPECT_EQ(test, seq_buf_putmem_hex(&s, data, sizeof(data)), 0);
+ KUNIT_EXPECT_FALSE(test, seq_buf_has_overflowed(&s));
+ KUNIT_EXPECT_EQ(test, seq_buf_used(&s), strlen(expected));
+ KUNIT_EXPECT_STREQ(test, seq_buf_str(&s), expected);
+}
+
+static void seq_buf_putmem_hex_overflow_test(struct kunit *test)
+{
+ DECLARE_SEQ_BUF(s, 20);
+ const u8 data[] = { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9 };
+#ifdef __BIG_ENDIAN
+ const char *expected = "0001020304050607 ";
+#else
+ const char *expected = "0706050403020100 ";
+#endif
+
+ KUNIT_EXPECT_EQ(test, seq_buf_putmem_hex(&s, data, sizeof(data)), -1);
+ KUNIT_EXPECT_TRUE(test, seq_buf_has_overflowed(&s));
+ KUNIT_EXPECT_EQ(test, seq_buf_used(&s), 20);
+ KUNIT_EXPECT_STREQ(test, seq_buf_str(&s), expected);
+}
+
static struct kunit_case seq_buf_test_cases[] = {
KUNIT_CASE(seq_buf_init_test),
KUNIT_CASE(seq_buf_declare_test),
@@ -194,6 +226,8 @@ static struct kunit_case seq_buf_test_cases[] = {
KUNIT_CASE(seq_buf_printf_test),
KUNIT_CASE(seq_buf_printf_overflow_test),
KUNIT_CASE(seq_buf_get_buf_commit_test),
+ KUNIT_CASE(seq_buf_putmem_hex_test),
+ KUNIT_CASE(seq_buf_putmem_hex_overflow_test),
{}
};
--
2.50.0
^ permalink raw reply related
* Re: [RFC PATCH 0/4] trace, livepatch: Allow kprobe return overriding for livepatched functions
From: Christoph Hellwig @ 2026-04-06 5:36 UTC (permalink / raw)
To: Yafang Shao
Cc: jpoimboe, jikos, mbenes, pmladek, joe.lawrence, rostedt, mhiramat,
mathieu.desnoyers, kpsingh, mattbobrowski, song, jolsa, ast,
daniel, andrii, martin.lau, eddyz87, memxor, yonghong.song,
live-patching, linux-kernel, linux-trace-kernel, bpf
In-Reply-To: <20260402092607.96430-1-laoar.shao@gmail.com>
On Thu, Apr 02, 2026 at 05:26:03PM +0800, Yafang Shao wrote:
> Livepatching allows for rapid experimentation with new kernel features
> without interrupting production workloads.
Myabe it allows, or based on the rest of the mail not quite. But that
is certainly not the intent at all, the intent is to fix critical
bugs without downtime.
> However, static livepatches lack
> the flexibility required to tune features based on task-specific attributes,
> such as cgroup membership, which is critical in multi-tenant k8s
> environments. Furthermore, hardcoding logic into a livepatch prevents
> dynamic adjustments based on the runtime environment.
>
> To address this, we propose a hybrid approach using BPF. Our production use
> case involves:
>
> 1. Deploying a Livepatch function to serve as a stable BPF hook.
>
> 2. Utilizing bpf_override_return() to dynamically modify the return value
> of that hook based on the current task's context.
Whol f**. now. Is this a delayed April 1st post?
^ permalink raw reply
* [PATCH v3] kernel/trace: fixed static warnings
From: abhijithsriram95 @ 2026-04-06 6:00 UTC (permalink / raw)
To: Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Abhijith Sriram, open list, open list:TRACING
From: Abhijith Sriram <abhijithsriram95@gmail.com>
The change in the function argument description
was due to the static code checker script reading
the word filter back to back
Changes in v2:
- corrected *m = file->private_data to m = file->private_data
Changes in v3:
- reverted the changes for struct seq_file *m and
added a new empty line instead
Signed-off-by: Abhijith Sriram <abhijithsriram95@gmail.com>
---
kernel/trace/trace_events_trigger.c | 8 +++++---
new-changes | 6 ++++++
2 files changed, 11 insertions(+), 3 deletions(-)
create mode 100644 new-changes
diff --git a/kernel/trace/trace_events_trigger.c b/kernel/trace/trace_events_trigger.c
index 655db2e82513..08adb593fcd9 100644
--- a/kernel/trace/trace_events_trigger.c
+++ b/kernel/trace/trace_events_trigger.c
@@ -246,7 +246,7 @@ event_triggers_post_call(struct trace_event_file *file,
}
EXPORT_SYMBOL_GPL(event_triggers_post_call);
-#define SHOW_AVAILABLE_TRIGGERS (void *)(1UL)
+#define SHOW_AVAILABLE_TRIGGERS ((void *)(1UL))
static void *trigger_next(struct seq_file *m, void *t, loff_t *pos)
{
@@ -352,6 +352,7 @@ static int event_trigger_regex_open(struct inode *inode, struct file *file)
ret = seq_open(file, &event_triggers_seq_ops);
if (!ret) {
struct seq_file *m = file->private_data;
+
m->private = file;
}
}
@@ -388,9 +389,9 @@ static ssize_t event_trigger_regex_write(struct file *file,
const char __user *ubuf,
size_t cnt, loff_t *ppos)
{
+ char *buf __free(kfree) = NULL;
struct trace_event_file *event_file;
ssize_t ret;
- char *buf __free(kfree) = NULL;
if (!cnt)
return 0;
@@ -633,6 +634,7 @@ clear_event_triggers(struct trace_array *tr)
list_for_each_entry(file, &tr->events, list) {
struct event_trigger_data *data, *n;
+
list_for_each_entry_safe(data, n, &file->triggers, list) {
trace_event_trigger_enable_disable(file, 0);
list_del_rcu(&data->list);
@@ -785,7 +787,7 @@ static void unregister_trigger(char *glob,
* cmd - the trigger command name
* glob - the trigger command name optionally prefaced with '!'
* param_and_filter - text following cmd and ':'
- * param - text following cmd and ':' and stripped of filter
+ * param - text following cmd and ':' and filter removed
* filter - the optional filter text following (and including) 'if'
*
* To illustrate the use of these components, here are some concrete
diff --git a/new-changes b/new-changes
new file mode 100644
index 000000000000..9e3a24de3033
--- /dev/null
+++ b/new-changes
@@ -0,0 +1,6 @@
+Line 25 -> adding const to the pointer address as well.
+
+linw 1193 -> removing else because there is a return statement in the if condition
+line 1727 -> adding new line after statement
+line 1800 -> reordering to solve missing a blank line warning
+line 12364 -> changed the function to kstrtoul
\ No newline at end of file
--
2.43.0
^ permalink raw reply related
* Re: [PATCH] kernel/trace: fixed static warnings
From: Abhijith Sriram @ 2026-04-06 6:03 UTC (permalink / raw)
To: Masami Hiramatsu
Cc: Steven Rostedt, Mathieu Desnoyers, open list:TRACING,
open list:TRACING
In-Reply-To: <20260406110053.b0582d349e42eefb1c4aeda6@kernel.org>
On Mon, Apr 6, 2026 at 4:00 AM Masami Hiramatsu <mhiramat@kernel.org> wrote:
>
> On Sat, 4 Apr 2026 08:03:07 +0200
> Abhijith Sriram <abhijithsriram95@gmail.com> wrote:
>
> > On Sat, Apr 4, 2026 at 2:18 AM Masami Hiramatsu <mhiramat@kernel.org> wrote:
> > >
> > > On Thu, 2 Apr 2026 21:54:04 +0200
> > > abhijithsriram95@gmail.com wrote:
> > >
> > > > From: Abhijith Sriram <abhijithsriram95@gmail.com>
> > > >
> > > > The change in the function argument description
> > > > was due to the static code checker script reading
> > > > the word filter back to back
> > > >
> > > > Signed-off-by: Abhijith Sriram <abhijithsriram95@gmail.com>
> > > > ---
> > > > kernel/trace/trace_events_trigger.c | 10 ++++++----
> > > > 1 file changed, 6 insertions(+), 4 deletions(-)
> > > >
> > > > diff --git a/kernel/trace/trace_events_trigger.c b/kernel/trace/trace_events_trigger.c
> > > > index 655db2e82513..477d8dee3362 100644
> > > > --- a/kernel/trace/trace_events_trigger.c
> > > > +++ b/kernel/trace/trace_events_trigger.c
> > > > @@ -246,7 +246,7 @@ event_triggers_post_call(struct trace_event_file *file,
> > > > }
> > > > EXPORT_SYMBOL_GPL(event_triggers_post_call);
> > > >
> > > > -#define SHOW_AVAILABLE_TRIGGERS (void *)(1UL)
> > > > +#define SHOW_AVAILABLE_TRIGGERS ((void *)(1UL))
> > >
> > > This is OK.
> > >
> > > >
> > > > static void *trigger_next(struct seq_file *m, void *t, loff_t *pos)
> > > > {
> > > > @@ -325,6 +325,7 @@ static const struct seq_operations event_triggers_seq_ops = {
> > > > static int event_trigger_regex_open(struct inode *inode, struct file *file)
> > > > {
> > > > int ret;
> > > > + struct seq_file *m = NULL;
> > > >
> > > > ret = security_locked_down(LOCKDOWN_TRACEFS);
> > > > if (ret)
> > > > @@ -351,7 +352,7 @@ static int event_trigger_regex_open(struct inode *inode, struct file *file)
> > > > if (file->f_mode & FMODE_READ) {
> > > > ret = seq_open(file, &event_triggers_seq_ops);
> > > > if (!ret) {
> > > > - struct seq_file *m = file->private_data;
> > > > + *m = file->private_data;
> > >
> > > Why is this change required?
> > The original warning says "missing blank line after declaration". I
> > thought it was cleaner to have the
> > declaration in the beginning of the function. I made a mistake here
> > which I fixed in the version 2 of
> > the patch, please have a look here:
> > https://lore.kernel.org/linux-trace-kernel/20260403071108.23422-2-abhijithsriram95@gmail.com/T/#u
>
> In that case, you just need to add an empty line, no need to move the
> definition becuase it changes the scope of `m` variable.
I reverted the changes in version 3 of the patch.
Please find it here:
https://lore.kernel.org/linux-trace-kernel/20260406060046.223496-2-abhijithsriram95@gmail.com/T/#u
>
> > >
> > > > m->private = file;
> > > > }
> > > > }
> > > > @@ -388,9 +389,9 @@ static ssize_t event_trigger_regex_write(struct file *file,
> > > > const char __user *ubuf,
> > > > size_t cnt, loff_t *ppos)
> > > > {
> > > > + char *buf __free(kfree) = NULL;
> > > > struct trace_event_file *event_file;
> > > > ssize_t ret;
> > > > - char *buf __free(kfree) = NULL;
> > >
> > > What is this change?
> > The same missing blank lines after declaration was triggered here,
> > even though there is a blank line after the char *buf.
> > If I do give an empty line then there is another error "Trailing white
> > space". So I reordered it and the warning disappeared.
> > This change I am not super sure since it is usually recommended that
> > variables of larger size are declared first
> > for padding purposes. What do you think?
>
> Ah, that is a known checkpatch's bug. It does not understand __free()
> macro. So please ignore that error (or/and fix checkpatch.pl).
>
> Thanks,
>
> > >
> > > Thanks,
> > >
> > > >
> > > > if (!cnt)
> > > > return 0;
> > > > @@ -633,6 +634,7 @@ clear_event_triggers(struct trace_array *tr)
> > > >
> > > > list_for_each_entry(file, &tr->events, list) {
> > > > struct event_trigger_data *data, *n;
> > > > +
> > > > list_for_each_entry_safe(data, n, &file->triggers, list) {
> > > > trace_event_trigger_enable_disable(file, 0);
> > > > list_del_rcu(&data->list);
> > > > @@ -785,7 +787,7 @@ static void unregister_trigger(char *glob,
> > > > * cmd - the trigger command name
> > > > * glob - the trigger command name optionally prefaced with '!'
> > > > * param_and_filter - text following cmd and ':'
> > > > - * param - text following cmd and ':' and stripped of filter
> > > > + * param - text following cmd and ':' and filter removed
> > > > * filter - the optional filter text following (and including) 'if'
> > > > *
> > > > * To illustrate the use of these components, here are some concrete
> > > > --
> > > > 2.43.0
> > > >
> > >
> > >
> > > --
> > > Masami Hiramatsu (Google) <mhiramat@kernel.org>
> > --
> > Regards
> > Abhijith Sriram
>
>
> --
> Masami Hiramatsu (Google) <mhiramat@kernel.org>
Regards
Abhijith Sriram
^ permalink raw reply
* Re: [PATCH v3] kernel/trace: fixed static warnings
From: Masami Hiramatsu @ 2026-04-06 7:18 UTC (permalink / raw)
To: abhijithsriram95
Cc: Steven Rostedt, Mathieu Desnoyers, open list, open list:TRACING
In-Reply-To: <20260406060046.223496-2-abhijithsriram95@gmail.com>
On Mon, 6 Apr 2026 08:00:36 +0200
abhijithsriram95@gmail.com wrote:
> From: Abhijith Sriram <abhijithsriram95@gmail.com>
>
> The change in the function argument description
> was due to the static code checker script reading
> the word filter back to back
>
> Changes in v2:
> - corrected *m = file->private_data to m = file->private_data
>
> Changes in v3:
> - reverted the changes for struct seq_file *m and
> added a new empty line instead
>
> Signed-off-by: Abhijith Sriram <abhijithsriram95@gmail.com>
> ---
> kernel/trace/trace_events_trigger.c | 8 +++++---
> new-changes | 6 ++++++
> 2 files changed, 11 insertions(+), 3 deletions(-)
> create mode 100644 new-changes
>
> diff --git a/kernel/trace/trace_events_trigger.c b/kernel/trace/trace_events_trigger.c
> index 655db2e82513..08adb593fcd9 100644
> --- a/kernel/trace/trace_events_trigger.c
> +++ b/kernel/trace/trace_events_trigger.c
> @@ -246,7 +246,7 @@ event_triggers_post_call(struct trace_event_file *file,
> }
> EXPORT_SYMBOL_GPL(event_triggers_post_call);
>
> -#define SHOW_AVAILABLE_TRIGGERS (void *)(1UL)
> +#define SHOW_AVAILABLE_TRIGGERS ((void *)(1UL))
>
> static void *trigger_next(struct seq_file *m, void *t, loff_t *pos)
> {
> @@ -352,6 +352,7 @@ static int event_trigger_regex_open(struct inode *inode, struct file *file)
> ret = seq_open(file, &event_triggers_seq_ops);
> if (!ret) {
> struct seq_file *m = file->private_data;
> +
> m->private = file;
> }
> }
> @@ -388,9 +389,9 @@ static ssize_t event_trigger_regex_write(struct file *file,
> const char __user *ubuf,
> size_t cnt, loff_t *ppos)
> {
> + char *buf __free(kfree) = NULL;
> struct trace_event_file *event_file;
> ssize_t ret;
> - char *buf __free(kfree) = NULL;
Again, this is not OK. Even if checkpatch.pl complained against this,
there should be no problem. Only if you think this is not sorted by
length, you can do:
struct trace_event_file *event_file;
+ char *buf __free(kfree) = NULL;
ssize_t ret;
- char *buf __free(kfree) = NULL;
This change is acceptable as a cosmetic change.
>
> if (!cnt)
> return 0;
> @@ -633,6 +634,7 @@ clear_event_triggers(struct trace_array *tr)
>
> list_for_each_entry(file, &tr->events, list) {
> struct event_trigger_data *data, *n;
> +
> list_for_each_entry_safe(data, n, &file->triggers, list) {
> trace_event_trigger_enable_disable(file, 0);
> list_del_rcu(&data->list);
> @@ -785,7 +787,7 @@ static void unregister_trigger(char *glob,
> * cmd - the trigger command name
> * glob - the trigger command name optionally prefaced with '!'
> * param_and_filter - text following cmd and ':'
> - * param - text following cmd and ':' and stripped of filter
> + * param - text following cmd and ':' and filter removed
> * filter - the optional filter text following (and including) 'if'
> *
> * To illustrate the use of these components, here are some concrete
> diff --git a/new-changes b/new-changes
> new file mode 100644
> index 000000000000..9e3a24de3033
> --- /dev/null
> +++ b/new-changes
> @@ -0,0 +1,6 @@
> +Line 25 -> adding const to the pointer address as well.
> +
> +linw 1193 -> removing else because there is a return statement in the if condition
> +line 1727 -> adding new line after statement
> +line 1800 -> reordering to solve missing a blank line warning
> +line 12364 -> changed the function to kstrtoul
> \ No newline at end of file
Is this your working note?
Please remove it.
Thank you,
> --
> 2.43.0
>
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
^ permalink raw reply
* [PATCH v4] kernel/trace: fixed static warnings
From: abhijithsriram95 @ 2026-04-06 7:28 UTC (permalink / raw)
To: Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
open list:TRACING, open list:TRACING
Cc: Abhijith Sriram
From: Abhijith Sriram <abhijithsriram95@gmail.com>
The change in the function argument description
was due to the static code checker script reading
the word filter back to back
Changes in v2:
- corrected *m = file->private_data to m = file->private_data
Changes in v3:
- reverted the changes for struct seq_file *m and
added a new empty line instead
Changes in v4:
- added a new empty line before char *buf ...
previously this line was relocated to avoid the
static check warning.
Signed-off-by: Abhijith Sriram <abhijithsriram95@gmail.com>
---
kernel/trace/trace_events_trigger.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/kernel/trace/trace_events_trigger.c b/kernel/trace/trace_events_trigger.c
index 655db2e82513..664283bcd9ea 100644
--- a/kernel/trace/trace_events_trigger.c
+++ b/kernel/trace/trace_events_trigger.c
@@ -246,7 +246,7 @@ event_triggers_post_call(struct trace_event_file *file,
}
EXPORT_SYMBOL_GPL(event_triggers_post_call);
-#define SHOW_AVAILABLE_TRIGGERS (void *)(1UL)
+#define SHOW_AVAILABLE_TRIGGERS ((void *)(1UL))
static void *trigger_next(struct seq_file *m, void *t, loff_t *pos)
{
@@ -352,6 +352,7 @@ static int event_trigger_regex_open(struct inode *inode, struct file *file)
ret = seq_open(file, &event_triggers_seq_ops);
if (!ret) {
struct seq_file *m = file->private_data;
+
m->private = file;
}
}
@@ -390,6 +391,7 @@ static ssize_t event_trigger_regex_write(struct file *file,
{
struct trace_event_file *event_file;
ssize_t ret;
+
char *buf __free(kfree) = NULL;
if (!cnt)
@@ -633,6 +635,7 @@ clear_event_triggers(struct trace_array *tr)
list_for_each_entry(file, &tr->events, list) {
struct event_trigger_data *data, *n;
+
list_for_each_entry_safe(data, n, &file->triggers, list) {
trace_event_trigger_enable_disable(file, 0);
list_del_rcu(&data->list);
@@ -785,7 +788,7 @@ static void unregister_trigger(char *glob,
* cmd - the trigger command name
* glob - the trigger command name optionally prefaced with '!'
* param_and_filter - text following cmd and ':'
- * param - text following cmd and ':' and stripped of filter
+ * param - text following cmd and ':' and filter removed
* filter - the optional filter text following (and including) 'if'
*
* To illustrate the use of these components, here are some concrete
--
2.43.0
^ permalink raw reply related
* Re: [PATCH v3] kernel/trace: fixed static warnings
From: Abhijith Sriram @ 2026-04-06 7:30 UTC (permalink / raw)
To: Masami Hiramatsu
Cc: Steven Rostedt, Mathieu Desnoyers, open list, open list:TRACING
In-Reply-To: <20260406161801.d507eeb247eb8b3b17f490a8@kernel.org>
On Mon, Apr 6, 2026 at 9:18 AM Masami Hiramatsu <mhiramat@kernel.org> wrote:
>
> On Mon, 6 Apr 2026 08:00:36 +0200
> abhijithsriram95@gmail.com wrote:
>
> > From: Abhijith Sriram <abhijithsriram95@gmail.com>
> >
> > The change in the function argument description
> > was due to the static code checker script reading
> > the word filter back to back
> >
> > Changes in v2:
> > - corrected *m = file->private_data to m = file->private_data
> >
> > Changes in v3:
> > - reverted the changes for struct seq_file *m and
> > added a new empty line instead
> >
> > Signed-off-by: Abhijith Sriram <abhijithsriram95@gmail.com>
> > ---
> > kernel/trace/trace_events_trigger.c | 8 +++++---
> > new-changes | 6 ++++++
> > 2 files changed, 11 insertions(+), 3 deletions(-)
> > create mode 100644 new-changes
> >
> > diff --git a/kernel/trace/trace_events_trigger.c b/kernel/trace/trace_events_trigger.c
> > index 655db2e82513..08adb593fcd9 100644
> > --- a/kernel/trace/trace_events_trigger.c
> > +++ b/kernel/trace/trace_events_trigger.c
> > @@ -246,7 +246,7 @@ event_triggers_post_call(struct trace_event_file *file,
> > }
> > EXPORT_SYMBOL_GPL(event_triggers_post_call);
> >
> > -#define SHOW_AVAILABLE_TRIGGERS (void *)(1UL)
> > +#define SHOW_AVAILABLE_TRIGGERS ((void *)(1UL))
> >
> > static void *trigger_next(struct seq_file *m, void *t, loff_t *pos)
> > {
> > @@ -352,6 +352,7 @@ static int event_trigger_regex_open(struct inode *inode, struct file *file)
> > ret = seq_open(file, &event_triggers_seq_ops);
> > if (!ret) {
> > struct seq_file *m = file->private_data;
> > +
> > m->private = file;
> > }
> > }
> > @@ -388,9 +389,9 @@ static ssize_t event_trigger_regex_write(struct file *file,
> > const char __user *ubuf,
> > size_t cnt, loff_t *ppos)
> > {
> > + char *buf __free(kfree) = NULL;
> > struct trace_event_file *event_file;
> > ssize_t ret;
> > - char *buf __free(kfree) = NULL;
>
> Again, this is not OK. Even if checkpatch.pl complained against this,
> there should be no problem. Only if you think this is not sorted by
> length, you can do:
Removed it in the new patch
>
> struct trace_event_file *event_file;
> + char *buf __free(kfree) = NULL;
> ssize_t ret;
> - char *buf __free(kfree) = NULL;
>
> This change is acceptable as a cosmetic change.
>
> >
> > if (!cnt)
> > return 0;
> > @@ -633,6 +634,7 @@ clear_event_triggers(struct trace_array *tr)
> >
> > list_for_each_entry(file, &tr->events, list) {
> > struct event_trigger_data *data, *n;
> > +
> > list_for_each_entry_safe(data, n, &file->triggers, list) {
> > trace_event_trigger_enable_disable(file, 0);
> > list_del_rcu(&data->list);
> > @@ -785,7 +787,7 @@ static void unregister_trigger(char *glob,
> > * cmd - the trigger command name
> > * glob - the trigger command name optionally prefaced with '!'
> > * param_and_filter - text following cmd and ':'
> > - * param - text following cmd and ':' and stripped of filter
> > + * param - text following cmd and ':' and filter removed
> > * filter - the optional filter text following (and including) 'if'
> > *
> > * To illustrate the use of these components, here are some concrete
> > diff --git a/new-changes b/new-changes
> > new file mode 100644
> > index 000000000000..9e3a24de3033
> > --- /dev/null
> > +++ b/new-changes
> > @@ -0,0 +1,6 @@
> > +Line 25 -> adding const to the pointer address as well.
> > +
> > +linw 1193 -> removing else because there is a return statement in the if condition
> > +line 1727 -> adding new line after statement
> > +line 1800 -> reordering to solve missing a blank line warning
> > +line 12364 -> changed the function to kstrtoul
> > \ No newline at end of file
>
> Is this your working note?
> Please remove it.
Removed it in the new patch
>
> Thank you,
>
> > --
> > 2.43.0
> >
>
>
> --
> Masami Hiramatsu (Google) <mhiramat@kernel.org>
Thank you for your patience. I am still learning and trying to find my hold.
Here is the new patch version:
https://lore.kernel.org/linux-trace-kernel/20260406072834.243491-2-abhijithsriram95@gmail.com/T/#u
I will do my best to be more careful in the future :)
--
Regards
Abhijith Sriram
^ permalink raw reply
* [PATCH v2] bootconfig: Skip printing early params to cmdline from bootconfig
From: Masami Hiramatsu (Google) @ 2026-04-06 7:49 UTC (permalink / raw)
To: Masami Hiramatsu, Steven Rostedt
Cc: Breno Leitao, linux-kernel, linux-trace-kernel
From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
If user configures `kernel.key` in bootconfig, the 'key' is shown
in kernel cmdline (/proc/cmdline) and kernel boot parameter
handler associated with 'key' is invoked. However, since the
bootconfig does not support the parameter defined with early_param,
those keys are shown in '/proc/cmdline' but not handled by kernel.
This could easily mislead users who expected to be able to specify
early parameters via the boot configuration, leading them to wonder
why it doesn't work.
Let's skip printing out early params to cmdline buffer, and warn
if there is such parameters in bootconfig.
Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
Changes in v2:
- Check if the parameter is defined only by early_param().
---
init/main.c | 35 +++++++++++++++++++++++++++++------
1 file changed, 29 insertions(+), 6 deletions(-)
diff --git a/init/main.c b/init/main.c
index 1cb395dd94e4..e4687c00e8fb 100644
--- a/init/main.c
+++ b/init/main.c
@@ -324,10 +324,26 @@ static void * __init get_boot_config_from_initrd(size_t *_size)
static char xbc_namebuf[XBC_KEYLEN_MAX] __initdata;
+/* Return true if the given param is only defined by early_param(). */
+static bool __init is_early_only_param(const char *param)
+{
+ const struct obs_kernel_param *p;
+ bool ret = false;
+
+ for (p = __setup_start; p < __setup_end; p++) {
+ if (parameq(param, p->str)) {
+ if (!p->early)
+ return false;
+ ret = true;
+ }
+ }
+ return ret;
+}
+
#define rest(dst, end) ((end) > (dst) ? (end) - (dst) : 0)
static int __init xbc_snprint_cmdline(char *buf, size_t size,
- struct xbc_node *root)
+ struct xbc_node *root, bool is_kernel)
{
struct xbc_node *knode, *vnode;
char *end = buf + size;
@@ -340,6 +356,13 @@ static int __init xbc_snprint_cmdline(char *buf, size_t size,
if (ret < 0)
return ret;
+ /* We will skip early params because it is not applied. */
+ if (is_kernel && is_early_only_param(xbc_namebuf)) {
+ pr_warn_once("bootconfig: early_param(e.g. %s.%s) can not be handled.\n",
+ xbc_node_get_data(root), xbc_namebuf);
+ continue;
+ }
+
vnode = xbc_node_get_child(knode);
if (!vnode) {
ret = snprintf(buf, rest(buf, end), "%s ", xbc_namebuf);
@@ -368,7 +391,7 @@ static int __init xbc_snprint_cmdline(char *buf, size_t size,
#undef rest
/* Make an extra command line under given key word */
-static char * __init xbc_make_cmdline(const char *key)
+static char * __init xbc_make_cmdline(const char *key, bool is_kernel)
{
struct xbc_node *root;
char *new_cmdline;
@@ -379,7 +402,7 @@ static char * __init xbc_make_cmdline(const char *key)
return NULL;
/* Count required buffer size */
- len = xbc_snprint_cmdline(NULL, 0, root);
+ len = xbc_snprint_cmdline(NULL, 0, root, is_kernel);
if (len <= 0)
return NULL;
@@ -389,7 +412,7 @@ static char * __init xbc_make_cmdline(const char *key)
return NULL;
}
- ret = xbc_snprint_cmdline(new_cmdline, len + 1, root);
+ ret = xbc_snprint_cmdline(new_cmdline, len + 1, root, is_kernel);
if (ret < 0 || ret > len) {
pr_err("Failed to print extra kernel cmdline.\n");
memblock_free(new_cmdline, len + 1);
@@ -465,9 +488,9 @@ static void __init setup_boot_config(void)
xbc_get_info(&ret, NULL);
pr_info("Load bootconfig: %ld bytes %d nodes\n", (long)size, ret);
/* keys starting with "kernel." are passed via cmdline */
- extra_command_line = xbc_make_cmdline("kernel");
+ extra_command_line = xbc_make_cmdline("kernel", true);
/* Also, "init." keys are init arguments */
- extra_init_args = xbc_make_cmdline("init");
+ extra_init_args = xbc_make_cmdline("init", false);
}
return;
}
^ permalink raw reply related
* [PATCH v16 0/5] ring-buffer: Making persistent ring buffers robust
From: Masami Hiramatsu (Google) @ 2026-04-06 10:24 UTC (permalink / raw)
To: Steven Rostedt, Catalin Marinas, Will Deacon
Cc: Masami Hiramatsu, Mathieu Desnoyers, linux-kernel,
linux-trace-kernel, Ian Rogers, linux-arm-kernel
Hi,
Here is the 16th version of improvement patches for making persistent
ring buffers robust to failures.
The previous version is here:
https://lore.kernel.org/all/177494615421.71933.3679132057004156013.stgit@mhiramat.tok.corp.google.com/
This version adds Catalin's Ack [1/5] and update description and
document[4/5][5/5]. Also, rebased on ring-buffer/for-next.
Thank you,
---
Masami Hiramatsu (Google) (5):
ring-buffer: Flush and stop persistent ring buffer on panic
ring-buffer: Skip invalid sub-buffers when validating persistent ring buffer
ring-buffer: Skip invalid sub-buffers when rewinding persistent ring buffer
ring-buffer: Add persistent ring buffer invalid-page inject test
ring-buffer: Show commit numbers in buffer_meta file
arch/alpha/include/asm/Kbuild | 1
arch/arc/include/asm/Kbuild | 1
arch/arm/include/asm/Kbuild | 1
arch/arm64/include/asm/ring_buffer.h | 10 +
arch/csky/include/asm/Kbuild | 1
arch/hexagon/include/asm/Kbuild | 1
arch/loongarch/include/asm/Kbuild | 1
arch/m68k/include/asm/Kbuild | 1
arch/microblaze/include/asm/Kbuild | 1
arch/mips/include/asm/Kbuild | 1
arch/nios2/include/asm/Kbuild | 1
arch/openrisc/include/asm/Kbuild | 1
arch/parisc/include/asm/Kbuild | 1
arch/powerpc/include/asm/Kbuild | 1
arch/riscv/include/asm/Kbuild | 1
arch/s390/include/asm/Kbuild | 1
arch/sh/include/asm/Kbuild | 1
arch/sparc/include/asm/Kbuild | 1
arch/um/include/asm/Kbuild | 1
arch/x86/include/asm/Kbuild | 1
arch/xtensa/include/asm/Kbuild | 1
include/asm-generic/ring_buffer.h | 13 ++
include/linux/ring_buffer.h | 1
kernel/trace/Kconfig | 34 ++++
kernel/trace/ring_buffer.c | 258 ++++++++++++++++++++++++++--------
kernel/trace/trace.c | 4 +
26 files changed, 276 insertions(+), 64 deletions(-)
create mode 100644 arch/arm64/include/asm/ring_buffer.h
create mode 100644 include/asm-generic/ring_buffer.h
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
^ permalink raw reply
* [PATCH v16 1/5] ring-buffer: Flush and stop persistent ring buffer on panic
From: Masami Hiramatsu (Google) @ 2026-04-06 10:24 UTC (permalink / raw)
To: Steven Rostedt, Catalin Marinas, Will Deacon
Cc: Masami Hiramatsu, Mathieu Desnoyers, linux-kernel,
linux-trace-kernel, Ian Rogers, linux-arm-kernel
In-Reply-To: <177547105523.259641.14385891517704197263.stgit@mhiramat.tok.corp.google.com>
From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
On real hardware, panic and machine reboot may not flush hardware cache
to memory. This means the persistent ring buffer, which relies on a
coherent state of memory, may not have its events written to the buffer
and they may be lost. Moreover, there may be inconsistency with the
counters which are used for validation of the integrity of the
persistent ring buffer which may cause all data to be discarded.
To avoid this issue, stop recording of the ring buffer on panic and
flush the cache of the ring buffer's memory.
Fixes: e645535a954a ("tracing: Add option to use memmapped memory for trace boot instance")
Cc: stable@vger.kernel.org
Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
---
Changes in v13:
- Fix a rebase conflict.
Changes in v11:
- Do nothing by default since flush_cache_vmap() does nothing on x86
but it can cause deadlock on some architectures via on_each_cpu()
because other CPUs will be stoppped when panic notifier is called.
Changes in v9:
- Fix typo of & to &&.
- Fix typo of "Generic"
Changes in v6:
- Introduce asm/ring_buffer.h for arch_ring_buffer_flush_range().
- Use flush_cache_vmap() instead of flush_cache_all().
Changes in v5:
- Use ring_buffer_record_off() instead of ring_buffer_record_disable().
- Use flush_cache_all() to ensure flush all cache.
Changes in v3:
- update patch description.
---
arch/alpha/include/asm/Kbuild | 1 +
arch/arc/include/asm/Kbuild | 1 +
arch/arm/include/asm/Kbuild | 1 +
arch/arm64/include/asm/ring_buffer.h | 10 ++++++++++
arch/csky/include/asm/Kbuild | 1 +
arch/hexagon/include/asm/Kbuild | 1 +
arch/loongarch/include/asm/Kbuild | 1 +
arch/m68k/include/asm/Kbuild | 1 +
arch/microblaze/include/asm/Kbuild | 1 +
arch/mips/include/asm/Kbuild | 1 +
arch/nios2/include/asm/Kbuild | 1 +
arch/openrisc/include/asm/Kbuild | 1 +
arch/parisc/include/asm/Kbuild | 1 +
arch/powerpc/include/asm/Kbuild | 1 +
arch/riscv/include/asm/Kbuild | 1 +
arch/s390/include/asm/Kbuild | 1 +
arch/sh/include/asm/Kbuild | 1 +
arch/sparc/include/asm/Kbuild | 1 +
arch/um/include/asm/Kbuild | 1 +
arch/x86/include/asm/Kbuild | 1 +
arch/xtensa/include/asm/Kbuild | 1 +
include/asm-generic/ring_buffer.h | 13 +++++++++++++
kernel/trace/ring_buffer.c | 22 ++++++++++++++++++++++
23 files changed, 65 insertions(+)
create mode 100644 arch/arm64/include/asm/ring_buffer.h
create mode 100644 include/asm-generic/ring_buffer.h
diff --git a/arch/alpha/include/asm/Kbuild b/arch/alpha/include/asm/Kbuild
index 483965c5a4de..b154b4e3dfa8 100644
--- a/arch/alpha/include/asm/Kbuild
+++ b/arch/alpha/include/asm/Kbuild
@@ -5,4 +5,5 @@ generic-y += agp.h
generic-y += asm-offsets.h
generic-y += kvm_para.h
generic-y += mcs_spinlock.h
+generic-y += ring_buffer.h
generic-y += text-patching.h
diff --git a/arch/arc/include/asm/Kbuild b/arch/arc/include/asm/Kbuild
index 4c69522e0328..483caacc6988 100644
--- a/arch/arc/include/asm/Kbuild
+++ b/arch/arc/include/asm/Kbuild
@@ -5,5 +5,6 @@ generic-y += extable.h
generic-y += kvm_para.h
generic-y += mcs_spinlock.h
generic-y += parport.h
+generic-y += ring_buffer.h
generic-y += user.h
generic-y += text-patching.h
diff --git a/arch/arm/include/asm/Kbuild b/arch/arm/include/asm/Kbuild
index 03657ff8fbe3..decad5f2c826 100644
--- a/arch/arm/include/asm/Kbuild
+++ b/arch/arm/include/asm/Kbuild
@@ -3,6 +3,7 @@ generic-y += early_ioremap.h
generic-y += extable.h
generic-y += flat.h
generic-y += parport.h
+generic-y += ring_buffer.h
generated-y += mach-types.h
generated-y += unistd-nr.h
diff --git a/arch/arm64/include/asm/ring_buffer.h b/arch/arm64/include/asm/ring_buffer.h
new file mode 100644
index 000000000000..62316c406888
--- /dev/null
+++ b/arch/arm64/include/asm/ring_buffer.h
@@ -0,0 +1,10 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+#ifndef _ASM_ARM64_RING_BUFFER_H
+#define _ASM_ARM64_RING_BUFFER_H
+
+#include <asm/cacheflush.h>
+
+/* Flush D-cache on persistent ring buffer */
+#define arch_ring_buffer_flush_range(start, end) dcache_clean_pop(start, end)
+
+#endif /* _ASM_ARM64_RING_BUFFER_H */
diff --git a/arch/csky/include/asm/Kbuild b/arch/csky/include/asm/Kbuild
index 3a5c7f6e5aac..7dca0c6cdc84 100644
--- a/arch/csky/include/asm/Kbuild
+++ b/arch/csky/include/asm/Kbuild
@@ -9,6 +9,7 @@ generic-y += qrwlock.h
generic-y += qrwlock_types.h
generic-y += qspinlock.h
generic-y += parport.h
+generic-y += ring_buffer.h
generic-y += user.h
generic-y += vmlinux.lds.h
generic-y += text-patching.h
diff --git a/arch/hexagon/include/asm/Kbuild b/arch/hexagon/include/asm/Kbuild
index 1efa1e993d4b..0f887d4238ed 100644
--- a/arch/hexagon/include/asm/Kbuild
+++ b/arch/hexagon/include/asm/Kbuild
@@ -5,4 +5,5 @@ generic-y += extable.h
generic-y += iomap.h
generic-y += kvm_para.h
generic-y += mcs_spinlock.h
+generic-y += ring_buffer.h
generic-y += text-patching.h
diff --git a/arch/loongarch/include/asm/Kbuild b/arch/loongarch/include/asm/Kbuild
index 9034b583a88a..7e92957baf6a 100644
--- a/arch/loongarch/include/asm/Kbuild
+++ b/arch/loongarch/include/asm/Kbuild
@@ -10,5 +10,6 @@ generic-y += qrwlock.h
generic-y += user.h
generic-y += ioctl.h
generic-y += mmzone.h
+generic-y += ring_buffer.h
generic-y += statfs.h
generic-y += text-patching.h
diff --git a/arch/m68k/include/asm/Kbuild b/arch/m68k/include/asm/Kbuild
index b282e0dd8dc1..62543bf305ff 100644
--- a/arch/m68k/include/asm/Kbuild
+++ b/arch/m68k/include/asm/Kbuild
@@ -3,5 +3,6 @@ generated-y += syscall_table.h
generic-y += extable.h
generic-y += kvm_para.h
generic-y += mcs_spinlock.h
+generic-y += ring_buffer.h
generic-y += spinlock.h
generic-y += text-patching.h
diff --git a/arch/microblaze/include/asm/Kbuild b/arch/microblaze/include/asm/Kbuild
index 7178f990e8b3..0030309b47ad 100644
--- a/arch/microblaze/include/asm/Kbuild
+++ b/arch/microblaze/include/asm/Kbuild
@@ -5,6 +5,7 @@ generic-y += extable.h
generic-y += kvm_para.h
generic-y += mcs_spinlock.h
generic-y += parport.h
+generic-y += ring_buffer.h
generic-y += syscalls.h
generic-y += tlb.h
generic-y += user.h
diff --git a/arch/mips/include/asm/Kbuild b/arch/mips/include/asm/Kbuild
index 684569b2ecd6..9771c3d85074 100644
--- a/arch/mips/include/asm/Kbuild
+++ b/arch/mips/include/asm/Kbuild
@@ -12,5 +12,6 @@ generic-y += mcs_spinlock.h
generic-y += parport.h
generic-y += qrwlock.h
generic-y += qspinlock.h
+generic-y += ring_buffer.h
generic-y += user.h
generic-y += text-patching.h
diff --git a/arch/nios2/include/asm/Kbuild b/arch/nios2/include/asm/Kbuild
index 28004301c236..0a2530964413 100644
--- a/arch/nios2/include/asm/Kbuild
+++ b/arch/nios2/include/asm/Kbuild
@@ -5,6 +5,7 @@ generic-y += cmpxchg.h
generic-y += extable.h
generic-y += kvm_para.h
generic-y += mcs_spinlock.h
+generic-y += ring_buffer.h
generic-y += spinlock.h
generic-y += user.h
generic-y += text-patching.h
diff --git a/arch/openrisc/include/asm/Kbuild b/arch/openrisc/include/asm/Kbuild
index cef49d60d74c..8aa34621702d 100644
--- a/arch/openrisc/include/asm/Kbuild
+++ b/arch/openrisc/include/asm/Kbuild
@@ -8,4 +8,5 @@ generic-y += spinlock_types.h
generic-y += spinlock.h
generic-y += qrwlock_types.h
generic-y += qrwlock.h
+generic-y += ring_buffer.h
generic-y += user.h
diff --git a/arch/parisc/include/asm/Kbuild b/arch/parisc/include/asm/Kbuild
index 4fb596d94c89..d48d158f7241 100644
--- a/arch/parisc/include/asm/Kbuild
+++ b/arch/parisc/include/asm/Kbuild
@@ -4,4 +4,5 @@ generated-y += syscall_table_64.h
generic-y += agp.h
generic-y += kvm_para.h
generic-y += mcs_spinlock.h
+generic-y += ring_buffer.h
generic-y += user.h
diff --git a/arch/powerpc/include/asm/Kbuild b/arch/powerpc/include/asm/Kbuild
index 2e23533b67e3..805b5aeebb6f 100644
--- a/arch/powerpc/include/asm/Kbuild
+++ b/arch/powerpc/include/asm/Kbuild
@@ -5,4 +5,5 @@ generated-y += syscall_table_spu.h
generic-y += agp.h
generic-y += mcs_spinlock.h
generic-y += qrwlock.h
+generic-y += ring_buffer.h
generic-y += early_ioremap.h
diff --git a/arch/riscv/include/asm/Kbuild b/arch/riscv/include/asm/Kbuild
index bd5fc9403295..7721b63642f4 100644
--- a/arch/riscv/include/asm/Kbuild
+++ b/arch/riscv/include/asm/Kbuild
@@ -14,5 +14,6 @@ generic-y += ticket_spinlock.h
generic-y += qrwlock.h
generic-y += qrwlock_types.h
generic-y += qspinlock.h
+generic-y += ring_buffer.h
generic-y += user.h
generic-y += vmlinux.lds.h
diff --git a/arch/s390/include/asm/Kbuild b/arch/s390/include/asm/Kbuild
index 80bad7de7a04..0c1fc47c3ba0 100644
--- a/arch/s390/include/asm/Kbuild
+++ b/arch/s390/include/asm/Kbuild
@@ -7,3 +7,4 @@ generated-y += unistd_nr.h
generic-y += asm-offsets.h
generic-y += mcs_spinlock.h
generic-y += mmzone.h
+generic-y += ring_buffer.h
diff --git a/arch/sh/include/asm/Kbuild b/arch/sh/include/asm/Kbuild
index 4d3f10ed8275..f0403d3ee8ab 100644
--- a/arch/sh/include/asm/Kbuild
+++ b/arch/sh/include/asm/Kbuild
@@ -3,4 +3,5 @@ generated-y += syscall_table.h
generic-y += kvm_para.h
generic-y += mcs_spinlock.h
generic-y += parport.h
+generic-y += ring_buffer.h
generic-y += text-patching.h
diff --git a/arch/sparc/include/asm/Kbuild b/arch/sparc/include/asm/Kbuild
index 17ee8a273aa6..49c6bb326b75 100644
--- a/arch/sparc/include/asm/Kbuild
+++ b/arch/sparc/include/asm/Kbuild
@@ -4,4 +4,5 @@ generated-y += syscall_table_64.h
generic-y += agp.h
generic-y += kvm_para.h
generic-y += mcs_spinlock.h
+generic-y += ring_buffer.h
generic-y += text-patching.h
diff --git a/arch/um/include/asm/Kbuild b/arch/um/include/asm/Kbuild
index 1b9b82bbe322..2a1629ba8140 100644
--- a/arch/um/include/asm/Kbuild
+++ b/arch/um/include/asm/Kbuild
@@ -17,6 +17,7 @@ generic-y += module.lds.h
generic-y += parport.h
generic-y += percpu.h
generic-y += preempt.h
+generic-y += ring_buffer.h
generic-y += runtime-const.h
generic-y += softirq_stack.h
generic-y += switch_to.h
diff --git a/arch/x86/include/asm/Kbuild b/arch/x86/include/asm/Kbuild
index 4566000e15c4..078fd2c0d69d 100644
--- a/arch/x86/include/asm/Kbuild
+++ b/arch/x86/include/asm/Kbuild
@@ -14,3 +14,4 @@ generic-y += early_ioremap.h
generic-y += fprobe.h
generic-y += mcs_spinlock.h
generic-y += mmzone.h
+generic-y += ring_buffer.h
diff --git a/arch/xtensa/include/asm/Kbuild b/arch/xtensa/include/asm/Kbuild
index 13fe45dea296..e57af619263a 100644
--- a/arch/xtensa/include/asm/Kbuild
+++ b/arch/xtensa/include/asm/Kbuild
@@ -6,5 +6,6 @@ generic-y += mcs_spinlock.h
generic-y += parport.h
generic-y += qrwlock.h
generic-y += qspinlock.h
+generic-y += ring_buffer.h
generic-y += user.h
generic-y += text-patching.h
diff --git a/include/asm-generic/ring_buffer.h b/include/asm-generic/ring_buffer.h
new file mode 100644
index 000000000000..201d2aee1005
--- /dev/null
+++ b/include/asm-generic/ring_buffer.h
@@ -0,0 +1,13 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Generic arch dependent ring_buffer macros.
+ */
+#ifndef __ASM_GENERIC_RING_BUFFER_H__
+#define __ASM_GENERIC_RING_BUFFER_H__
+
+#include <linux/cacheflush.h>
+
+/* Flush cache on ring buffer range if needed. Do nothing by default. */
+#define arch_ring_buffer_flush_range(start, end) do { } while (0)
+
+#endif /* __ASM_GENERIC_RING_BUFFER_H__ */
diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
index 2caa5d3d0ae9..4d5817286791 100644
--- a/kernel/trace/ring_buffer.c
+++ b/kernel/trace/ring_buffer.c
@@ -7,6 +7,7 @@
#include <linux/ring_buffer_types.h>
#include <linux/sched/isolation.h>
#include <linux/trace_recursion.h>
+#include <linux/panic_notifier.h>
#include <linux/trace_events.h>
#include <linux/ring_buffer.h>
#include <linux/trace_clock.h>
@@ -31,6 +32,7 @@
#include <linux/oom.h>
#include <linux/mm.h>
+#include <asm/ring_buffer.h>
#include <asm/local64.h>
#include <asm/local.h>
#include <asm/setup.h>
@@ -559,6 +561,7 @@ struct trace_buffer {
unsigned long range_addr_start;
unsigned long range_addr_end;
+ struct notifier_block flush_nb;
struct ring_buffer_meta *meta;
@@ -2520,6 +2523,16 @@ static void rb_free_cpu_buffer(struct ring_buffer_per_cpu *cpu_buffer)
kfree(cpu_buffer);
}
+/* Stop recording on a persistent buffer and flush cache if needed. */
+static int rb_flush_buffer_cb(struct notifier_block *nb, unsigned long event, void *data)
+{
+ struct trace_buffer *buffer = container_of(nb, struct trace_buffer, flush_nb);
+
+ ring_buffer_record_off(buffer);
+ arch_ring_buffer_flush_range(buffer->range_addr_start, buffer->range_addr_end);
+ return NOTIFY_DONE;
+}
+
static struct trace_buffer *alloc_buffer(unsigned long size, unsigned flags,
int order, unsigned long start,
unsigned long end,
@@ -2650,6 +2663,12 @@ static struct trace_buffer *alloc_buffer(unsigned long size, unsigned flags,
mutex_init(&buffer->mutex);
+ /* Persistent ring buffer needs to flush cache before reboot. */
+ if (start && end) {
+ buffer->flush_nb.notifier_call = rb_flush_buffer_cb;
+ atomic_notifier_chain_register(&panic_notifier_list, &buffer->flush_nb);
+ }
+
return_ptr(buffer);
fail_free_buffers:
@@ -2748,6 +2767,9 @@ ring_buffer_free(struct trace_buffer *buffer)
{
int cpu;
+ if (buffer->range_addr_start && buffer->range_addr_end)
+ atomic_notifier_chain_unregister(&panic_notifier_list, &buffer->flush_nb);
+
cpuhp_state_remove_instance(CPUHP_TRACE_RB_PREPARE, &buffer->node);
irq_work_sync(&buffer->irq_work.work);
^ permalink raw reply related
* [PATCH v16 2/5] ring-buffer: Skip invalid sub-buffers when validating persistent ring buffer
From: Masami Hiramatsu (Google) @ 2026-04-06 10:24 UTC (permalink / raw)
To: Steven Rostedt, Catalin Marinas, Will Deacon
Cc: Masami Hiramatsu, Mathieu Desnoyers, linux-kernel,
linux-trace-kernel, Ian Rogers, linux-arm-kernel
In-Reply-To: <177547105523.259641.14385891517704197263.stgit@mhiramat.tok.corp.google.com>
From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Skip invalid sub-buffers when validating the persistent ring buffer
instead of discarding the entire ring buffer. Only skipped buffers
are invalidated (cleared).
If the cache data in memory fails to be synchronized during a reboot,
the persistent ring buffer may become partially corrupted, but other
sub-buffers may still contain readable event data. Only discard the
subbuffers that are found to be corrupted.
Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
Changes in v15:
- Skip reader_page loop check on persistent ring buffer because
there can be contiguous empty(invalidated) pages.
- Do not show discarded page number information if it is 0.
Changes in v11:
- Fix a typo.
Changes in v9:
- Add meta->subbuf_size check.
- Fix a typo.
- Handle invalid reader_page case.
Changes in v8:
- Add comment in rb_valudate_buffer()
- Clear the RB_MISSED_* flags in rb_valudate_buffer() instead of
skipping subbuf.
- Remove unused subbuf local variable from rb_cpu_meta_valid().
Changes in v7:
- Combined with Handling RB_MISSED_* flags patch, focus on validation at boot.
- Remove checking subbuffer data when validating metadata, because it should be done
later.
- Do not mark the discarded sub buffer page but just reset it.
Changes in v6:
- Show invalid page detection message once per CPU.
Changes in v5:
- Instead of showing errors for each page, just show the number
of discarded pages at last.
Changes in v3:
- Record missed data event on commit.
---
kernel/trace/ring_buffer.c | 109 ++++++++++++++++++++++++++------------------
1 file changed, 65 insertions(+), 44 deletions(-)
diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
index 4d5817286791..0c284094f7d0 100644
--- a/kernel/trace/ring_buffer.c
+++ b/kernel/trace/ring_buffer.c
@@ -370,6 +370,12 @@ static __always_inline unsigned int rb_page_commit(struct buffer_page *bpage)
return local_read(&bpage->page->commit);
}
+/* Size is determined by what has been committed */
+static __always_inline unsigned int rb_page_size(struct buffer_page *bpage)
+{
+ return rb_page_commit(bpage) & ~RB_MISSED_MASK;
+}
+
static void free_buffer_page(struct buffer_page *bpage)
{
/* Range pages are not to be freed */
@@ -1762,7 +1768,6 @@ static bool rb_cpu_meta_valid(struct ring_buffer_cpu_meta *meta, int cpu,
unsigned long *subbuf_mask)
{
int subbuf_size = PAGE_SIZE;
- struct buffer_data_page *subbuf;
unsigned long buffers_start;
unsigned long buffers_end;
int i;
@@ -1770,6 +1775,11 @@ static bool rb_cpu_meta_valid(struct ring_buffer_cpu_meta *meta, int cpu,
if (!subbuf_mask)
return false;
+ if (meta->subbuf_size != PAGE_SIZE) {
+ pr_info("Ring buffer boot meta [%d] invalid subbuf_size\n", cpu);
+ return false;
+ }
+
buffers_start = meta->first_buffer;
buffers_end = meta->first_buffer + (subbuf_size * meta->nr_subbufs);
@@ -1786,11 +1796,12 @@ static bool rb_cpu_meta_valid(struct ring_buffer_cpu_meta *meta, int cpu,
return false;
}
- subbuf = rb_subbufs_from_meta(meta);
-
bitmap_clear(subbuf_mask, 0, meta->nr_subbufs);
- /* Is the meta buffers and the subbufs themselves have correct data? */
+ /*
+ * Ensure the meta::buffers array has correct data. The data in each subbufs
+ * are checked later in rb_meta_validate_events().
+ */
for (i = 0; i < meta->nr_subbufs; i++) {
if (meta->buffers[i] < 0 ||
meta->buffers[i] >= meta->nr_subbufs) {
@@ -1798,18 +1809,12 @@ static bool rb_cpu_meta_valid(struct ring_buffer_cpu_meta *meta, int cpu,
return false;
}
- if ((unsigned)local_read(&subbuf->commit) > subbuf_size) {
- pr_info("Ring buffer boot meta [%d] buffer invalid commit\n", cpu);
- return false;
- }
-
if (test_bit(meta->buffers[i], subbuf_mask)) {
pr_info("Ring buffer boot meta [%d] array has duplicates\n", cpu);
return false;
}
set_bit(meta->buffers[i], subbuf_mask);
- subbuf = (void *)subbuf + subbuf_size;
}
return true;
@@ -1873,13 +1878,22 @@ static int rb_read_data_buffer(struct buffer_data_page *dpage, int tail, int cpu
return events;
}
-static int rb_validate_buffer(struct buffer_data_page *dpage, int cpu)
+static int rb_validate_buffer(struct buffer_data_page *dpage, int cpu,
+ struct ring_buffer_cpu_meta *meta)
{
unsigned long long ts;
+ unsigned long tail;
u64 delta;
- int tail;
- tail = local_read(&dpage->commit);
+ /*
+ * When a sub-buffer is recovered from a read, the commit value may
+ * have RB_MISSED_* bits set, as these bits are reset on reuse.
+ * Even after clearing these bits, a commit value greater than the
+ * subbuf_size is considered invalid.
+ */
+ tail = local_read(&dpage->commit) & ~RB_MISSED_MASK;
+ if (tail > meta->subbuf_size)
+ return -1;
return rb_read_data_buffer(dpage, tail, cpu, &ts, &delta);
}
@@ -1890,6 +1904,7 @@ static void rb_meta_validate_events(struct ring_buffer_per_cpu *cpu_buffer)
struct buffer_page *head_page, *orig_head;
unsigned long entry_bytes = 0;
unsigned long entries = 0;
+ int discarded = 0;
int ret;
u64 ts;
int i;
@@ -1900,14 +1915,19 @@ static void rb_meta_validate_events(struct ring_buffer_per_cpu *cpu_buffer)
orig_head = head_page = cpu_buffer->head_page;
/* Do the reader page first */
- ret = rb_validate_buffer(cpu_buffer->reader_page->page, cpu_buffer->cpu);
+ ret = rb_validate_buffer(cpu_buffer->reader_page->page, cpu_buffer->cpu, meta);
if (ret < 0) {
- pr_info("Ring buffer reader page is invalid\n");
- goto invalid;
+ pr_info("Ring buffer meta [%d] invalid reader page detected\n",
+ cpu_buffer->cpu);
+ discarded++;
+ /* Instead of discard whole ring buffer, discard only this sub-buffer. */
+ local_set(&cpu_buffer->reader_page->entries, 0);
+ local_set(&cpu_buffer->reader_page->page->commit, 0);
+ } else {
+ entries += ret;
+ entry_bytes += rb_page_size(cpu_buffer->reader_page);
+ local_set(&cpu_buffer->reader_page->entries, ret);
}
- entries += ret;
- entry_bytes += local_read(&cpu_buffer->reader_page->page->commit);
- local_set(&cpu_buffer->reader_page->entries, ret);
ts = head_page->page->time_stamp;
@@ -1935,7 +1955,7 @@ static void rb_meta_validate_events(struct ring_buffer_per_cpu *cpu_buffer)
break;
/* Stop rewind if the page is invalid. */
- ret = rb_validate_buffer(head_page->page, cpu_buffer->cpu);
+ ret = rb_validate_buffer(head_page->page, cpu_buffer->cpu, meta);
if (ret < 0)
break;
@@ -2014,21 +2034,24 @@ static void rb_meta_validate_events(struct ring_buffer_per_cpu *cpu_buffer)
if (head_page == cpu_buffer->reader_page)
continue;
- ret = rb_validate_buffer(head_page->page, cpu_buffer->cpu);
+ ret = rb_validate_buffer(head_page->page, cpu_buffer->cpu, meta);
if (ret < 0) {
- pr_info("Ring buffer meta [%d] invalid buffer page\n",
- cpu_buffer->cpu);
- goto invalid;
- }
-
- /* If the buffer has content, update pages_touched */
- if (ret)
- local_inc(&cpu_buffer->pages_touched);
-
- entries += ret;
- entry_bytes += local_read(&head_page->page->commit);
- local_set(&head_page->entries, ret);
+ if (!discarded)
+ pr_info("Ring buffer meta [%d] invalid buffer page detected\n",
+ cpu_buffer->cpu);
+ discarded++;
+ /* Instead of discard whole ring buffer, discard only this sub-buffer. */
+ local_set(&head_page->entries, 0);
+ local_set(&head_page->page->commit, 0);
+ } else {
+ /* If the buffer has content, update pages_touched */
+ if (ret)
+ local_inc(&cpu_buffer->pages_touched);
+ entries += ret;
+ entry_bytes += rb_page_size(head_page);
+ local_set(&head_page->entries, ret);
+ }
if (head_page == cpu_buffer->commit_page)
break;
}
@@ -2042,7 +2065,10 @@ static void rb_meta_validate_events(struct ring_buffer_per_cpu *cpu_buffer)
local_set(&cpu_buffer->entries, entries);
local_set(&cpu_buffer->entries_bytes, entry_bytes);
- pr_info("Ring buffer meta [%d] is from previous boot!\n", cpu_buffer->cpu);
+ pr_info("Ring buffer meta [%d] is from previous boot!", cpu_buffer->cpu);
+ if (discarded)
+ pr_cont(" (%d pages discarded)", discarded);
+ pr_cont("\n");
return;
invalid:
@@ -3329,12 +3355,6 @@ rb_iter_head_event(struct ring_buffer_iter *iter)
return NULL;
}
-/* Size is determined by what has been committed */
-static __always_inline unsigned rb_page_size(struct buffer_page *bpage)
-{
- return rb_page_commit(bpage) & ~RB_MISSED_MASK;
-}
-
static __always_inline unsigned
rb_commit_index(struct ring_buffer_per_cpu *cpu_buffer)
{
@@ -5647,11 +5667,12 @@ __rb_get_reader_page(struct ring_buffer_per_cpu *cpu_buffer)
again:
/*
* This should normally only loop twice. But because the
- * start of the reader inserts an empty page, it causes
- * a case where we will loop three times. There should be no
- * reason to loop four times (that I know of).
+ * start of the reader inserts an empty page, it causes a
+ * case where we will loop three times. There should be no
+ * reason to loop four times unless the ring buffer is a
+ * recovered persistent ring buffer.
*/
- if (RB_WARN_ON(cpu_buffer, ++nr_loops > 3)) {
+ if (RB_WARN_ON(cpu_buffer, ++nr_loops > 3 && !cpu_buffer->ring_meta)) {
reader = NULL;
goto out;
}
^ permalink raw reply related
* [PATCH v16 3/5] ring-buffer: Skip invalid sub-buffers when rewinding persistent ring buffer
From: Masami Hiramatsu (Google) @ 2026-04-06 10:24 UTC (permalink / raw)
To: Steven Rostedt, Catalin Marinas, Will Deacon
Cc: Masami Hiramatsu, Mathieu Desnoyers, linux-kernel,
linux-trace-kernel, Ian Rogers, linux-arm-kernel
In-Reply-To: <177547105523.259641.14385891517704197263.stgit@mhiramat.tok.corp.google.com>
From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Skip invalid sub-buffers when rewinding the persistent ring buffer
instead of stopping the rewinding the ring buffer. The skipped
buffers are cleared.
To ensure the rewinding stops at the unused page, this also clears
buffer_data_page::time_stamp when tracing resets the buffer. This
allows us to identify unused pages and empty pages.
Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
Changes in v12:
- Fix build error.
Changes in v11:
- Reset timestamp when the buffer is invalid.
- When rewinding, skip subbuf page if timestamp is wrong and
check timestamp after validating buffer data page.
Changes in v10:
- Newly added.
---
kernel/trace/ring_buffer.c | 76 +++++++++++++++++++++++++-------------------
1 file changed, 43 insertions(+), 33 deletions(-)
diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
index 0c284094f7d0..518a05df6ef7 100644
--- a/kernel/trace/ring_buffer.c
+++ b/kernel/trace/ring_buffer.c
@@ -363,6 +363,7 @@ struct buffer_page {
static void rb_init_page(struct buffer_data_page *bpage)
{
local_set(&bpage->commit, 0);
+ bpage->time_stamp = 0;
}
static __always_inline unsigned int rb_page_commit(struct buffer_page *bpage)
@@ -1878,12 +1879,14 @@ static int rb_read_data_buffer(struct buffer_data_page *dpage, int tail, int cpu
return events;
}
-static int rb_validate_buffer(struct buffer_data_page *dpage, int cpu,
+static int rb_validate_buffer(struct buffer_page *bpage, int cpu,
struct ring_buffer_cpu_meta *meta)
{
+ struct buffer_data_page *dpage = bpage->page;
unsigned long long ts;
unsigned long tail;
u64 delta;
+ int ret = -1;
/*
* When a sub-buffer is recovered from a read, the commit value may
@@ -1892,9 +1895,17 @@ static int rb_validate_buffer(struct buffer_data_page *dpage, int cpu,
* subbuf_size is considered invalid.
*/
tail = local_read(&dpage->commit) & ~RB_MISSED_MASK;
- if (tail > meta->subbuf_size)
- return -1;
- return rb_read_data_buffer(dpage, tail, cpu, &ts, &delta);
+ if (tail <= meta->subbuf_size)
+ ret = rb_read_data_buffer(dpage, tail, cpu, &ts, &delta);
+
+ if (ret < 0) {
+ local_set(&bpage->entries, 0);
+ local_set(&bpage->page->commit, 0);
+ } else {
+ local_set(&bpage->entries, ret);
+ }
+
+ return ret;
}
/* If the meta data has been validated, now validate the events */
@@ -1915,18 +1926,14 @@ static void rb_meta_validate_events(struct ring_buffer_per_cpu *cpu_buffer)
orig_head = head_page = cpu_buffer->head_page;
/* Do the reader page first */
- ret = rb_validate_buffer(cpu_buffer->reader_page->page, cpu_buffer->cpu, meta);
+ ret = rb_validate_buffer(cpu_buffer->reader_page, cpu_buffer->cpu, meta);
if (ret < 0) {
pr_info("Ring buffer meta [%d] invalid reader page detected\n",
cpu_buffer->cpu);
discarded++;
- /* Instead of discard whole ring buffer, discard only this sub-buffer. */
- local_set(&cpu_buffer->reader_page->entries, 0);
- local_set(&cpu_buffer->reader_page->page->commit, 0);
} else {
entries += ret;
entry_bytes += rb_page_size(cpu_buffer->reader_page);
- local_set(&cpu_buffer->reader_page->entries, ret);
}
ts = head_page->page->time_stamp;
@@ -1945,26 +1952,33 @@ static void rb_meta_validate_events(struct ring_buffer_per_cpu *cpu_buffer)
if (head_page == cpu_buffer->tail_page)
break;
- /* Ensure the page has older data than head. */
- if (ts < head_page->page->time_stamp)
- break;
-
- ts = head_page->page->time_stamp;
- /* Ensure the page has correct timestamp and some data. */
- if (!ts || rb_page_commit(head_page) == 0)
- break;
-
- /* Stop rewind if the page is invalid. */
- ret = rb_validate_buffer(head_page->page, cpu_buffer->cpu, meta);
- if (ret < 0)
+ /* Rewind until unused page (no timestamp, no commit). */
+ if (!head_page->page->time_stamp && rb_page_commit(head_page) == 0)
break;
- /* Recover the number of entries and update stats. */
- local_set(&head_page->entries, ret);
- if (ret)
- local_inc(&cpu_buffer->pages_touched);
- entries += ret;
- entry_bytes += rb_page_commit(head_page);
+ /*
+ * Skip if the page is invalid, or its timestamp is newer than the
+ * previous valid page.
+ */
+ ret = rb_validate_buffer(head_page, cpu_buffer->cpu, meta);
+ if (ret >= 0 && ts < head_page->page->time_stamp) {
+ local_set(&head_page->entries, 0);
+ local_set(&head_page->page->commit, 0);
+ head_page->page->time_stamp = ts;
+ ret = -1;
+ }
+ if (ret < 0) {
+ if (!discarded)
+ pr_info("Ring buffer meta [%d] invalid buffer page detected\n",
+ cpu_buffer->cpu);
+ discarded++;
+ } else {
+ entries += ret;
+ entry_bytes += rb_page_size(head_page);
+ if (ret > 0)
+ local_inc(&cpu_buffer->pages_touched);
+ ts = head_page->page->time_stamp;
+ }
}
if (i)
pr_info("Ring buffer [%d] rewound %d pages\n", cpu_buffer->cpu, i);
@@ -2034,15 +2048,12 @@ static void rb_meta_validate_events(struct ring_buffer_per_cpu *cpu_buffer)
if (head_page == cpu_buffer->reader_page)
continue;
- ret = rb_validate_buffer(head_page->page, cpu_buffer->cpu, meta);
+ ret = rb_validate_buffer(head_page, cpu_buffer->cpu, meta);
if (ret < 0) {
if (!discarded)
pr_info("Ring buffer meta [%d] invalid buffer page detected\n",
cpu_buffer->cpu);
discarded++;
- /* Instead of discard whole ring buffer, discard only this sub-buffer. */
- local_set(&head_page->entries, 0);
- local_set(&head_page->page->commit, 0);
} else {
/* If the buffer has content, update pages_touched */
if (ret)
@@ -2050,7 +2061,6 @@ static void rb_meta_validate_events(struct ring_buffer_per_cpu *cpu_buffer)
entries += ret;
entry_bytes += rb_page_size(head_page);
- local_set(&head_page->entries, ret);
}
if (head_page == cpu_buffer->commit_page)
break;
@@ -2083,7 +2093,7 @@ static void rb_meta_validate_events(struct ring_buffer_per_cpu *cpu_buffer)
/* Reset all the subbuffers */
for (i = 0; i < meta->nr_subbufs - 1; i++, rb_inc_page(&head_page)) {
local_set(&head_page->entries, 0);
- local_set(&head_page->page->commit, 0);
+ rb_init_page(head_page->page);
}
}
^ permalink raw reply related
* [PATCH v16 4/5] ring-buffer: Add persistent ring buffer invalid-page inject test
From: Masami Hiramatsu (Google) @ 2026-04-06 10:24 UTC (permalink / raw)
To: Steven Rostedt, Catalin Marinas, Will Deacon
Cc: Masami Hiramatsu, Mathieu Desnoyers, linux-kernel,
linux-trace-kernel, Ian Rogers, linux-arm-kernel
In-Reply-To: <177547105523.259641.14385891517704197263.stgit@mhiramat.tok.corp.google.com>
From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Add a self-corrupting test for the persistent ring buffer.
This will inject an erroneous value to some sub-buffer pages (where
the index is even or multiples of 5) in the persistent ring buffer
when the kernel panics, and checks whether the number of detected
invalid pages and the total entry_bytes are the same as the recorded
values after reboot.
This ensures that the kernel can correctly recover a partially
corrupted persistent ring buffer after a reboot or panic.
The test only runs on the persistent ring buffer whose name is
"ptracingtest". The user has to fill it with events before a
kernel panic.
To run the test, enable CONFIG_RING_BUFFER_PERSISTENT_INJECT
and add the following kernel cmdline:
reserve_mem=20M:2M:trace trace_instance=ptracingtest^traceoff@trace
panic=1
Run the following commands after the 1st boot:
cd /sys/kernel/tracing/instances/ptracingtest
echo 1 > tracing_on
echo 1 > events/enable
sleep 3
echo c > /proc/sysrq-trigger
After panic message, the kernel will reboot and run the verification
on the persistent ring buffer, e.g.
Ring buffer meta [2] invalid buffer page detected
Ring buffer meta [2] is from previous boot! (318 pages discarded)
Ring buffer testing [2] invalid pages: PASSED (318/318)
Ring buffer testing [2] entry_bytes: PASSED (1300476/1300476)
Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
Changes in v16:
- Update description and comments according to review comments.
Changes in v15:
- Use pr_warn() for test result.
- Inject errors on the page index is multiples of 5 so that
this can reproduce contiguous empty pages.
Changes in v14:
- Rename config to CONFIG_RING_BUFFER_PERSISTENT_INJECT.
- Clear meta->nr_invalid/entry_bytes after testing.
- Add test commands in config comment.
Changes in v10:
- Add entry_bytes test.
- Do not compile test code if CONFIG_RING_BUFFER_PERSISTENT_SELFTEST=n.
Changes in v9:
- Test also reader pages.
---
include/linux/ring_buffer.h | 1 +
kernel/trace/Kconfig | 34 ++++++++++++++++++++
kernel/trace/ring_buffer.c | 74 +++++++++++++++++++++++++++++++++++++++++++
kernel/trace/trace.c | 4 ++
4 files changed, 113 insertions(+)
diff --git a/include/linux/ring_buffer.h b/include/linux/ring_buffer.h
index 994f52b34344..0670742b2d60 100644
--- a/include/linux/ring_buffer.h
+++ b/include/linux/ring_buffer.h
@@ -238,6 +238,7 @@ int ring_buffer_subbuf_size_get(struct trace_buffer *buffer);
enum ring_buffer_flags {
RB_FL_OVERWRITE = 1 << 0,
+ RB_FL_TESTING = 1 << 1,
};
#ifdef CONFIG_RING_BUFFER
diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
index e130da35808f..084f34dc6c9f 100644
--- a/kernel/trace/Kconfig
+++ b/kernel/trace/Kconfig
@@ -1202,6 +1202,40 @@ config RING_BUFFER_VALIDATE_TIME_DELTAS
Only say Y if you understand what this does, and you
still want it enabled. Otherwise say N
+config RING_BUFFER_PERSISTENT_INJECT
+ bool "Enable persistent ring buffer error injection test"
+ depends on RING_BUFFER
+ help
+ This option will have the kernel check if the persistent ring
+ buffer is named "ptracingtest". and if so, it will corrupt some
+ of its pages on a kernel panic. This is used to test if the
+ persistent ring buffer can recover from some of its sub-buffers
+ being corrupted.
+ To use this, boot a kernel with a "ptracingtest" persistent
+ ring buffer, e.g.
+
+ reserve_mem=20M:2M:trace trace_instance=ptracingtest@trace panic=1
+
+ And after the 1st boot, run the following commands:
+
+ cd /sys/kernel/tracing/instances/ptracingtest
+ echo 1 > events/enable
+ echo 1 > tracing_on
+ sleep 3
+ echo c > /proc/sysrq-trigger
+
+ After the panic message, the kernel will reboot and will show
+ the test results in the console output.
+
+ Note that events for the test ring buffer needs to be enabled
+ prior to crashing the kernel so that the ring buffer has content
+ that the test will corrupt.
+ As the test will corrupt events in the "ptracingtest" persistent
+ ring buffer, it should not be used for any other purpose other
+ than this test.
+
+ If unsure, say N
+
config MMIOTRACE_TEST
tristate "Test module for mmiotrace"
depends on MMIOTRACE && m
diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
index 518a05df6ef7..e56fe9dcc7d7 100644
--- a/kernel/trace/ring_buffer.c
+++ b/kernel/trace/ring_buffer.c
@@ -64,6 +64,10 @@ struct ring_buffer_cpu_meta {
unsigned long commit_buffer;
__u32 subbuf_size;
__u32 nr_subbufs;
+#ifdef CONFIG_RING_BUFFER_PERSISTENT_INJECT
+ __u32 nr_invalid;
+ __u32 entry_bytes;
+#endif
int buffers[];
};
@@ -2079,6 +2083,21 @@ static void rb_meta_validate_events(struct ring_buffer_per_cpu *cpu_buffer)
if (discarded)
pr_cont(" (%d pages discarded)", discarded);
pr_cont("\n");
+
+#ifdef CONFIG_RING_BUFFER_PERSISTENT_INJECT
+ if (meta->nr_invalid)
+ pr_warn("Ring buffer testing [%d] invalid pages: %s (%d/%d)\n",
+ cpu_buffer->cpu,
+ (discarded == meta->nr_invalid) ? "PASSED" : "FAILED",
+ discarded, meta->nr_invalid);
+ if (meta->entry_bytes)
+ pr_warn("Ring buffer testing [%d] entry_bytes: %s (%ld/%ld)\n",
+ cpu_buffer->cpu,
+ (entry_bytes == meta->entry_bytes) ? "PASSED" : "FAILED",
+ (long)entry_bytes, (long)meta->entry_bytes);
+ meta->nr_invalid = 0;
+ meta->entry_bytes = 0;
+#endif
return;
invalid:
@@ -2559,12 +2578,67 @@ static void rb_free_cpu_buffer(struct ring_buffer_per_cpu *cpu_buffer)
kfree(cpu_buffer);
}
+#ifdef CONFIG_RING_BUFFER_PERSISTENT_INJECT
+static void rb_test_inject_invalid_pages(struct trace_buffer *buffer)
+{
+ struct ring_buffer_per_cpu *cpu_buffer;
+ struct ring_buffer_cpu_meta *meta;
+ struct buffer_data_page *dpage;
+ u32 entry_bytes = 0;
+ unsigned long ptr;
+ int subbuf_size;
+ int invalid = 0;
+ int cpu;
+ int i;
+
+ if (!(buffer->flags & RB_FL_TESTING))
+ return;
+
+ guard(preempt)();
+ cpu = smp_processor_id();
+
+ cpu_buffer = buffer->buffers[cpu];
+ meta = cpu_buffer->ring_meta;
+ ptr = (unsigned long)rb_subbufs_from_meta(meta);
+ subbuf_size = meta->subbuf_size;
+
+ for (i = 0; i < meta->nr_subbufs; i++) {
+ int idx = meta->buffers[i];
+
+ dpage = (void *)(ptr + idx * subbuf_size);
+ /* Skip unused pages */
+ if (!local_read(&dpage->commit))
+ continue;
+
+ /*
+ * Invalidate even pages or multiples of 5. This will cause 3
+ * contiguous invalidated(empty) pages.
+ */
+ if (!(i & 0x1) || !(i % 5)) {
+ local_add(subbuf_size + 1, &dpage->commit);
+ invalid++;
+ } else {
+ /* Count total commit bytes. */
+ entry_bytes += local_read(&dpage->commit);
+ }
+ }
+
+ pr_info("Inject invalidated %d pages on CPU%d, total size: %ld\n",
+ invalid, cpu, (long)entry_bytes);
+ meta->nr_invalid = invalid;
+ meta->entry_bytes = entry_bytes;
+}
+#else /* !CONFIG_RING_BUFFER_PERSISTENT_INJECT */
+#define rb_test_inject_invalid_pages(buffer) do { } while (0)
+#endif
+
/* Stop recording on a persistent buffer and flush cache if needed. */
static int rb_flush_buffer_cb(struct notifier_block *nb, unsigned long event, void *data)
{
struct trace_buffer *buffer = container_of(nb, struct trace_buffer, flush_nb);
ring_buffer_record_off(buffer);
+ rb_test_inject_invalid_pages(buffer);
arch_ring_buffer_flush_range(buffer->range_addr_start, buffer->range_addr_end);
return NOTIFY_DONE;
}
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index e9455d46ec16..96101d276d13 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -9436,6 +9436,8 @@ static void setup_trace_scratch(struct trace_array *tr,
memset(tscratch, 0, size);
}
+#define TRACE_TEST_PTRACING_NAME "ptracingtest"
+
static int
allocate_trace_buffer(struct trace_array *tr, struct array_buffer *buf, unsigned long size)
{
@@ -9448,6 +9450,8 @@ allocate_trace_buffer(struct trace_array *tr, struct array_buffer *buf, unsigned
buf->tr = tr;
if (tr->range_addr_start && tr->range_addr_size) {
+ if (!strcmp(tr->name, TRACE_TEST_PTRACING_NAME))
+ rb_flags |= RB_FL_TESTING;
/* Add scratch buffer to handle 128 modules */
buf->buffer = ring_buffer_alloc_range(size, rb_flags, 0,
tr->range_addr_start,
^ permalink raw reply related
* [PATCH v16 5/5] ring-buffer: Show commit numbers in buffer_meta file
From: Masami Hiramatsu (Google) @ 2026-04-06 10:24 UTC (permalink / raw)
To: Steven Rostedt, Catalin Marinas, Will Deacon
Cc: Masami Hiramatsu, Mathieu Desnoyers, linux-kernel,
linux-trace-kernel, Ian Rogers, linux-arm-kernel
In-Reply-To: <177547105523.259641.14385891517704197263.stgit@mhiramat.tok.corp.google.com>
From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
In addition to the index number, show the commit numbers of
each data page in the per_cpu buffer_meta file.
This is useful for understanding the current status of the
persistent ring buffer. (Note that this file is shown
only for persistent ring buffer and its backup instance)
Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
Changes in v16:
- update description.
---
kernel/trace/ring_buffer.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
index e56fe9dcc7d7..4bf83b7805da 100644
--- a/kernel/trace/ring_buffer.c
+++ b/kernel/trace/ring_buffer.c
@@ -2209,6 +2209,7 @@ static int rbm_show(struct seq_file *m, void *v)
struct ring_buffer_per_cpu *cpu_buffer = m->private;
struct ring_buffer_cpu_meta *meta = cpu_buffer->ring_meta;
unsigned long val = (unsigned long)v;
+ struct buffer_data_page *dpage;
if (val == 1) {
seq_printf(m, "head_buffer: %d\n",
@@ -2221,7 +2222,9 @@ static int rbm_show(struct seq_file *m, void *v)
}
val -= 2;
- seq_printf(m, "buffer[%ld]: %d\n", val, meta->buffers[val]);
+ dpage = rb_range_buffer(cpu_buffer, val);
+ seq_printf(m, "buffer[%ld]: %d (commit: %ld)\n",
+ val, meta->buffers[val], local_read(&dpage->commit));
return 0;
}
^ 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