tracing/mmiotrace: Fix some bugs in mmiotrace

PATCH In review Masami Hiramatsu (Google) 2 participants 8 messages Jul 28 – Aug 8

Subsystems (2)

Tracing Tracing MMIO accesses (mmiotrace)

lore

Share & tools

lore

Masami Hiramatsu (Google)

author maintainer

2026-07-28 12:49 UTC #0 lore raw

cover tracing/mmiotrace: Fix some bugs in mmiotrace

Hi,

Here are some patches to fix bugs in mmiotrace. I used Antigravity to scan the code and found some issues.
The first 2 patches are fixing some kind of timing issues. Other patches are a kind of cleanups.

Thank you,

---
base-commit: 62cc90241548d5570ee68e01aaba6506964e9811

Masami Hiramatsu (Google) (4):
tracing/mmiotrace: Reset dropped_count in mmio_reset_data() tracing/mmiotrace: Add NULL check for mmio_trace_array in logging functions tracing/mmiotrace: Use trace_assign_type() in mmio_print_mark() tracing/mmiotrace: Clean up coding style and redundant debug logs

 kernel/trace/trace_mmiotrace.c |   36 +++++++++++++++++++++++-------------
 1 file changed, 23 insertions(+), 13 deletions(-)

--
Masami Hiramatsu (Google) <[email protected]>

Masami Hiramatsu (Google)

author maintainer

2026-07-28 12:49 UTC #1 lore raw

PATCH 1/4 tracing/mmiotrace: Reset dropped_count in mmio_reset_data()

From: Masami Hiramatsu (Google) <[email protected]>

mmio_reset_data() is called during tracer initialization, reset, and start. While it resets overrun_detected and prev_overruns, it neglects to reset dropped_count. Consequently, dropped event counts from prior tracing sessions persist in dropped_count and corrupt overrun reports in subsequent runs.

Fix this by explicitly calling atomic_set(&dropped_count, 0) in mmio_reset_data().

Fixes 173ed24ee2d6 ("mmiotrace: count events lost due to not recording")

Assisted-by Antigravity:gemini-3.6-flash

Signed-off-by Masami Hiramatsu (Google) <[email protected]>

---
 kernel/trace/trace_mmiotrace.c |    1 +
 1 file changed, 1 insertion(+)

kernel/trace/trace_mmiotrace.c +1 -0

index b88b8d9923ad..e064ba3f28cb 100644
@@ -29,6 +29,7 @@ static void mmio_reset_data(struct trace_array *tr)
 {
 	overrun_detected = false;
 	prev_overruns = 0;
+	atomic_set(&dropped_count, 0);
 
 	tracing_reset_online_cpus(&tr->array_buffer);
 }

Masami Hiramatsu (Google)

author maintainer

2026-07-28 12:50 UTC #2 lore raw

PATCH 2/4 tracing/mmiotrace: Add NULL check for mmio_trace_array in logging functions

In reply to Masami Hiramatsu (Google)

From: Masami Hiramatsu (Google) <[email protected]>

mmio_trace_rw() and mmio_trace_mapping() retrieve mmio_trace_array into tr and pass it to __trace_mmiotrace_rw() and __trace_mmiotrace_map(). If these functions are invoked while mmio_trace_array is NULL (e.g. before initialization or after disabled), accessing tr->array_buffer.buffer will result in a NULL pointer dereference crash.

Fix this by adding an explicit NULL check for tr at the beginning of __trace_mmiotrace_rw() and __trace_mmiotrace_map().

Fixes f984b51e0779 ("ftrace: add mmiotrace plugin")

Assisted-by Antigravity:gemini-3.6-flash

Signed-off-by Masami Hiramatsu (Google) <[email protected]>

---
 kernel/trace/trace_mmiotrace.c |   12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

kernel/trace/trace_mmiotrace.c +10 -2

index e064ba3f28cb..df8692c2dea8 100644
@@ -294,11 +294,15 @@ device_initcall(init_mmio_trace);
 static void __trace_mmiotrace_rw(struct trace_array *tr,
 				struct mmiotrace_rw *rw)
 {
-	struct trace_buffer *buffer = tr->array_buffer.buffer;
+	struct trace_buffer *buffer;
 	struct ring_buffer_event *event;
 	struct trace_mmiotrace_rw *entry;
 	unsigned int trace_ctx;
 
+	if (!tr)
+		return;
+
+	buffer = tr->array_buffer.buffer;
 	trace_ctx = tracing_gen_ctx_flags(0);
 	event = trace_buffer_lock_reserve(buffer, TRACE_MMIO_RW,
 					  sizeof(*entry), trace_ctx);

@@ -321,11 +325,15 @@ void mmio_trace_rw(struct mmiotrace_rw *rw)
 static void __trace_mmiotrace_map(struct trace_array *tr,
 				struct mmiotrace_map *map)
 {
-	struct trace_buffer *buffer = tr->array_buffer.buffer;
+	struct trace_buffer *buffer;
 	struct ring_buffer_event *event;
 	struct trace_mmiotrace_map *entry;
 	unsigned int trace_ctx;
 
+	if (!tr)
+		return;
+
+	buffer = tr->array_buffer.buffer;
 	trace_ctx = tracing_gen_ctx_flags(0);
 	event = trace_buffer_lock_reserve(buffer, TRACE_MMIO_MAP,
 					  sizeof(*entry), trace_ctx);

Masami Hiramatsu (Google)

author maintainer

2026-07-28 12:50 UTC #3 lore raw

PATCH 3/4 tracing/mmiotrace: Use trace_assign_type() in mmio_print_mark()

In reply to Masami Hiramatsu (Google)

From: Masami Hiramatsu (Google) <[email protected]>

In mmio_print_mark(), a raw C cast (struct print_entry *)entry is used to obtain the print_entry pointer.

Use the standard trace_assign_type() macro instead, matching the usage in mmio_print_rw() and mmio_print_map().

Assisted-by Antigravity:gemini-3.6-flash

Signed-off-by Masami Hiramatsu (Google) <[email protected]>

---
 kernel/trace/trace_mmiotrace.c |    7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

kernel/trace/trace_mmiotrace.c +5 -2

index df8692c2dea8..77120d467e11 100644
@@ -244,13 +244,16 @@ static enum print_line_t mmio_print_map(struct trace_iterator *iter)
 static enum print_line_t mmio_print_mark(struct trace_iterator *iter)
 {
 	struct trace_entry *entry = iter->ent;
-	struct print_entry *print = (struct print_entry *)entry;
-	const char *msg		= print->buf;
+	struct print_entry *print;
+	const char *msg;
 	struct trace_seq *s	= &iter->seq;
 	unsigned long long t	= ns2usecs(iter->ts);
 	unsigned long usec_rem	= do_div(t, USEC_PER_SEC);
 	unsigned secs		= (unsigned long)t;
 
+	trace_assign_type(print, entry);
+	msg = print->buf;
+
 	/* The trailing newline must be in the message. */
 	trace_seq_printf(s, "MARK %u.%06lu %s", secs, usec_rem, msg);

Masami Hiramatsu (Google)

author maintainer

2026-07-28 12:50 UTC #4 lore raw

PATCH 4/4 tracing/mmiotrace: Clean up coding style and redundant debug logs

In reply to Masami Hiramatsu (Google)

From: Masami Hiramatsu (Google) <[email protected]>

Clean up coding style issues in trace_mmiotrace.c:
- Remove redundant pr_debug() entries in tracer callbacks.
- Fix opening brace placement for mmio_tracer.
- Prefer 'unsigned int' to bare 'unsigned'.
- Add missing blank lines after local variable declarations.

Assisted-by Antigravity:gemini-3.6-flash

Signed-off-by Masami Hiramatsu (Google) <[email protected]>

---
 kernel/trace/trace_mmiotrace.c |   16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)

kernel/trace/trace_mmiotrace.c +7 -9

index 77120d467e11..ce16e1c53d12 100644
@@ -36,7 +36,6 @@ static void mmio_reset_data(struct trace_array *tr)
 
 static int mmio_trace_init(struct trace_array *tr)
 {
-	pr_debug("in %s\n", __func__);
 	mmio_trace_array = tr;
 
 	mmio_reset_data(tr);

@@ -46,8 +45,6 @@ static int mmio_trace_init(struct trace_array *tr)
 
 static void mmio_trace_reset(struct trace_array *tr)
 {
-	pr_debug("in %s\n", __func__);
-
 	disable_mmiotrace();
 	mmio_reset_data(tr);
 	mmio_trace_array = NULL;

@@ -55,7 +52,6 @@ static void mmio_trace_reset(struct trace_array *tr)
 
 static void mmio_trace_start(struct trace_array *tr)
 {
-	pr_debug("in %s\n", __func__);
 	mmio_reset_data(tr);
 }

@@ -113,6 +109,7 @@ static void mmio_pipe_open(struct trace_iterator *iter)
 static void mmio_close(struct trace_iterator *iter)
 {
 	struct header_iter *hiter = iter->private;
+
 	destroy_header_iter(hiter);
 	iter->private = NULL;
 }

@@ -170,7 +167,7 @@ static enum print_line_t mmio_print_rw(struct trace_iterator *iter)
 	struct trace_seq *s	= &iter->seq;
 	unsigned long long t	= ns2usecs(iter->ts);
 	unsigned long usec_rem	= do_div(t, USEC_PER_SEC);
-	unsigned secs		= (unsigned long)t;
+	unsigned int secs	= (unsigned long)t;
 
 	trace_assign_type(field, entry);
 	rw = &field->rw;

@@ -215,7 +212,7 @@ static enum print_line_t mmio_print_map(struct trace_iterator *iter)
 	struct trace_seq *s	= &iter->seq;
 	unsigned long long t	= ns2usecs(iter->ts);
 	unsigned long usec_rem	= do_div(t, USEC_PER_SEC);
-	unsigned secs		= (unsigned long)t;
+	unsigned int secs	= (unsigned long)t;
 
 	trace_assign_type(field, entry);
 	m = &field->map;

@@ -249,7 +246,7 @@ static enum print_line_t mmio_print_mark(struct trace_iterator *iter)
 	struct trace_seq *s	= &iter->seq;
 	unsigned long long t	= ns2usecs(iter->ts);
 	unsigned long usec_rem	= do_div(t, USEC_PER_SEC);
-	unsigned secs		= (unsigned long)t;
+	unsigned int secs	= (unsigned long)t;
 
 	trace_assign_type(print, entry);
 	msg = print->buf;

@@ -274,8 +271,7 @@ static enum print_line_t mmio_print_line(struct trace_iterator *iter)
 	}
 }
 
-static struct tracer mmio_tracer __read_mostly =
-{
+static struct tracer mmio_tracer __read_mostly = {
 	.name		= "mmiotrace",
 	.init		= mmio_trace_init,
 	.reset		= mmio_trace_reset,

@@ -322,6 +318,7 @@ static void __trace_mmiotrace_rw(struct trace_array *tr,
 void mmio_trace_rw(struct mmiotrace_rw *rw)
 {
 	struct trace_array *tr = mmio_trace_array;
+
 	__trace_mmiotrace_rw(tr, rw);
 }

@@ -353,6 +350,7 @@ static void __trace_mmiotrace_map(struct trace_array *tr,
 void mmio_trace_mapping(struct mmiotrace_map *map)
 {
 	struct trace_array *tr = mmio_trace_array;
+
 	__trace_mmiotrace_map(tr, map);
 }

Steven Rostedt

maintainer

2026-08-08 00:50 UTC #5 lore raw

Re: [PATCH 4/4] tracing/mmiotrace: Clean up coding style and redundant debug logs

Quoted 19 lines · On Tue, 28 Jul 2026 21:50:19 +0900 "Masami Hiramatsu (Google)"…

From: Masami Hiramatsu (Google) <[email protected]>
Clean up coding style issues in trace_mmiotrace.c:
- Remove redundant pr_debug() entries in tracer callbacks.
- Fix opening brace placement for mmio_tracer.
- Prefer 'unsigned int' to bare 'unsigned'.
- Add missing blank lines after local variable declarations.
Assisted-by Antigravity:gemini-3.6-flash
Signed-off-by Masami Hiramatsu (Google) <[email protected]>
---
 kernel/trace/trace_mmiotrace.c |   16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)
kernel/trace/trace_mmiotrace.c +0 -0
index 77120d467e11..ce16e1c53d12 100644

kernel/trace/trace_mmiotrace.c static void mmio_reset_data(struct trace_array *tr) · line 39

Quoted 4 lines

kernel/trace/trace_mmiotrace.c +0 -1

@@ -36,7 +36,6 @@ static void mmio_reset_data(struct trace_array *tr)
 
 static int mmio_trace_init(struct trace_array *tr)
 {
-	pr_debug("in %s\n", __func__);

So how are these redundant?

Are there pr_debug() prints elsewhere?

Quoted 21 lines

kernel/trace/trace_mmiotrace.c +0 -3

 	mmio_trace_array = tr;
 
 	mmio_reset_data(tr);

@@ -46,8 +45,6 @@ static int mmio_trace_init(struct trace_array *tr)
 
 static void mmio_trace_reset(struct trace_array *tr)
 {
-	pr_debug("in %s\n", __func__);
-
 	disable_mmiotrace();
 	mmio_reset_data(tr);
 	mmio_trace_array = NULL;

@@ -55,7 +52,6 @@ static void mmio_trace_reset(struct trace_array *tr)
 
 static void mmio_trace_start(struct trace_array *tr)
 {
-	pr_debug("in %s\n", __func__);
 	mmio_reset_data(tr);
 }

kernel/trace/trace_mmiotrace.c static void mmio_pipe_open(struct trace_iterator *iter) · line 115

Quoted 7 lines

kernel/trace/trace_mmiotrace.c +1 -0

@@ -113,6 +109,7 @@ static void mmio_pipe_open(struct trace_iterator *iter)
 static void mmio_close(struct trace_iterator *iter)
 {
 	struct header_iter *hiter = iter->private;
+
 	destroy_header_iter(hiter);
 	iter->private = NULL;
 }

This is fine.

kernel/trace/trace_mmiotrace.c static enum print_line_t mmio_print_rw(struct trace_iterator *iter) · line 171

Quoted 7 lines

kernel/trace/trace_mmiotrace.c +1 -1

@@ -170,7 +167,7 @@ static enum print_line_t mmio_print_rw(struct trace_iterator *iter)
 	struct trace_seq *s	= &iter->seq;
 	unsigned long long t	= ns2usecs(iter->ts);
 	unsigned long usec_rem	= do_div(t, USEC_PER_SEC);
-	unsigned secs		= (unsigned long)t;
+	unsigned int secs	= (unsigned long)t;

OK.

Quoted 21 lines

kernel/trace/trace_mmiotrace.c +2 -2

 	trace_assign_type(field, entry);
 	rw = &field->rw;

@@ -215,7 +212,7 @@ static enum print_line_t mmio_print_map(struct trace_iterator *iter)
 	struct trace_seq *s	= &iter->seq;
 	unsigned long long t	= ns2usecs(iter->ts);
 	unsigned long usec_rem	= do_div(t, USEC_PER_SEC);
-	unsigned secs		= (unsigned long)t;
+	unsigned int secs	= (unsigned long)t;
 
 	trace_assign_type(field, entry);
 	m = &field->map;

@@ -249,7 +246,7 @@ static enum print_line_t mmio_print_mark(struct trace_iterator *iter)
 	struct trace_seq *s	= &iter->seq;
 	unsigned long long t	= ns2usecs(iter->ts);
 	unsigned long usec_rem	= do_div(t, USEC_PER_SEC);
-	unsigned secs		= (unsigned long)t;
+	unsigned int secs	= (unsigned long)t;
 
 	trace_assign_type(print, entry);
 	msg = print->buf;

kernel/trace/trace_mmiotrace.c static enum print_line_t mmio_print_line(struct trace_iterator *iter) · line 274

Quoted 6 lines

kernel/trace/trace_mmiotrace.c +1 -2

@@ -274,8 +271,7 @@ static enum print_line_t mmio_print_line(struct trace_iterator *iter)
 	}
 }
 
-static struct tracer mmio_tracer __read_mostly =
-{
+static struct tracer mmio_tracer __read_mostly = {

OK.

Quoted 12 lines

kernel/trace/trace_mmiotrace.c +1 -0

 	.name		= "mmiotrace",
 	.init		= mmio_trace_init,
 	.reset		= mmio_trace_reset,

@@ -322,6 +318,7 @@ static void __trace_mmiotrace_rw(struct trace_array *tr,
 void mmio_trace_rw(struct mmiotrace_rw *rw)
 {
 	struct trace_array *tr = mmio_trace_array;
+
 	__trace_mmiotrace_rw(tr, rw);
 }

kernel/trace/trace_mmiotrace.c static void __trace_mmiotrace_map(struct trace_array *tr, · line 355

Quoted 6 lines

kernel/trace/trace_mmiotrace.c +1 -0

@@ -353,6 +350,7 @@ static void __trace_mmiotrace_map(struct trace_array *tr,
 void mmio_trace_mapping(struct mmiotrace_map *map)
 {
 	struct trace_array *tr = mmio_trace_array;
+
 	__trace_mmiotrace_map(tr, map);
 }

Honestly, for one line functions like the above, I think it looks better without that blank line.

-- Steve

Masami Hiramatsu (Google)

author maintainer

2026-08-08 04:16 UTC #6 lore raw

Re: [PATCH 4/4] tracing/mmiotrace: Clean up coding style and redundant debug logs

Quoted 23 lines · On Fri, 7 Aug 2026 20:50:53 -0400 Steven Rostedt <[email protected]> wrote:

Quoted 20 lines · On Tue, 28 Jul 2026 21:50:19 +0900 "Masami Hiramatsu (Google)"…
From: Masami Hiramatsu (Google) <[email protected]>
Clean up coding style issues in trace_mmiotrace.c:
- Remove redundant pr_debug() entries in tracer callbacks.
- Fix opening brace placement for mmio_tracer.
- Prefer 'unsigned int' to bare 'unsigned'.
- Add missing blank lines after local variable declarations.
Assisted-by Antigravity:gemini-3.6-flash
Signed-off-by Masami Hiramatsu (Google) <[email protected]>
---
 kernel/trace/trace_mmiotrace.c |   16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)
kernel/trace/trace_mmiotrace.c +0 -0
index 77120d467e11..ce16e1c53d12 100644
@@ -36,7 +36,6 @@ static void mmio_reset_data(struct trace_array *tr)
 

Quoted 7 lines

kernel/trace/trace_mmiotrace.c static void mmio_reset_data(struct trace_array *tr) · line 39
Quoted 3 lines
kernel/trace/trace_mmiotrace.c +0 -1
 static int mmio_trace_init(struct trace_array *tr)
 {
-	pr_debug("in %s\n", __func__);
So how are these redundant?
Are there pr_debug() prints elsewhere?

No, that is not redundant, but just like only for debugging mmiotrace. Maybe we can leave it.

[...]

Quoted 19 lines

Quoted 9 lines
kernel/trace/trace_mmiotrace.c +1 -0
@@ -322,6 +318,7 @@ static void __trace_mmiotrace_rw(struct trace_array *tr,
 void mmio_trace_rw(struct mmiotrace_rw *rw)
 {
 	struct trace_array *tr = mmio_trace_array;
+
 	__trace_mmiotrace_rw(tr, rw);
 }
 
kernel/trace/trace_mmiotrace.c static void __trace_mmiotrace_map(struct trace_array *tr, · line 355
Quoted 6 lines
kernel/trace/trace_mmiotrace.c +1 -0
@@ -353,6 +350,7 @@ static void __trace_mmiotrace_map(struct trace_array *tr,
 void mmio_trace_mapping(struct mmiotrace_map *map)
 {
 	struct trace_array *tr = mmio_trace_array;
+
 	__trace_mmiotrace_map(tr, map);
 }
Honestly, for one line functions like the above, I think it looks better without that blank line.

Quoted 1 line

-- Steve

Yeah, this is somewhat machinary fix against the checkpatch.pl. Or, maybe it is better to just pass mmio_trace_array directly?

Thanks,

Signature · 1 line

Masami Hiramatsu (Google) <[email protected]>

Steven Rostedt

maintainer

2026-08-08 13:12 UTC #7 lore raw

Re: [PATCH 4/4] tracing/mmiotrace: Clean up coding style and redundant debug logs

Quoted 2 lines · On Sat, 8 Aug 2026 13:16:34 +0900 Masami Hiramatsu (Google)…

Yeah, this is somewhat machinary fix against the checkpatch.pl. Or, maybe it is better to just pass mmio_trace_array directly?

checkpatch.pl isn't for existing code. It is for new patches. And is also more of a guideline than a rule.

Those two functions don't need to be changed to satisfy checkpatch. They are fine as is.

-- Steve