[PATCH 0/4] tracing/mmiotrace: Fix some bugs in mmiotrace

[PATCH 0/4] tracing/mmiotrace: Fix some bugs in mmiotrace

[Masami Hiramatsu (Google)]

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) <mhiramat@kernel.org>

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

[Masami Hiramatsu (Google)]

From: Masami Hiramatsu (Google) <mhiramat@kernel.org>

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) <mhiramat@kernel.org>
---
 kernel/trace/trace_mmiotrace.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/kernel/trace/trace_mmiotrace.c b/kernel/trace/trace_mmiotrace.c
index b88b8d9923ad..e064ba3f28cb 100644
--- a/kernel/trace/trace_mmiotrace.c
+++ b/kernel/trace/trace_mmiotrace.c
@@ -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);
 }

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

[Masami Hiramatsu (Google)]

From: Masami Hiramatsu (Google) <mhiramat@kernel.org>

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) <mhiramat@kernel.org>
---
 kernel/trace/trace_mmiotrace.c |   12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/kernel/trace/trace_mmiotrace.c b/kernel/trace/trace_mmiotrace.c
index e064ba3f28cb..df8692c2dea8 100644
--- a/kernel/trace/trace_mmiotrace.c
+++ b/kernel/trace/trace_mmiotrace.c
@@ -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);

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

[Masami Hiramatsu (Google)]

From: Masami Hiramatsu (Google) <mhiramat@kernel.org>

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) <mhiramat@kernel.org>
---
 kernel/trace/trace_mmiotrace.c |    7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/kernel/trace/trace_mmiotrace.c b/kernel/trace/trace_mmiotrace.c
index df8692c2dea8..77120d467e11 100644
--- a/kernel/trace/trace_mmiotrace.c
+++ b/kernel/trace/trace_mmiotrace.c
@@ -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);
 

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

[Masami Hiramatsu (Google)]

From: Masami Hiramatsu (Google) <mhiramat@kernel.org>

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) <mhiramat@kernel.org>
---
 kernel/trace/trace_mmiotrace.c |   16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/kernel/trace/trace_mmiotrace.c b/kernel/trace/trace_mmiotrace.c
index 77120d467e11..ce16e1c53d12 100644
--- a/kernel/trace/trace_mmiotrace.c
+++ b/kernel/trace/trace_mmiotrace.c
@@ -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);
 }
 

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

[Steven Rostedt]

On Tue, 28 Jul 2026 21:50:19 +0900
"Masami Hiramatsu (Google)" <mhiramat@kernel.org> wrote:

> From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> 
> 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) <mhiramat@kernel.org>
> ---
>  kernel/trace/trace_mmiotrace.c |   16 +++++++---------
>  1 file changed, 7 insertions(+), 9 deletions(-)
> 
> diff --git a/kernel/trace/trace_mmiotrace.c b/kernel/trace/trace_mmiotrace.c
> index 77120d467e11..ce16e1c53d12 100644
> --- a/kernel/trace/trace_mmiotrace.c
> +++ b/kernel/trace/trace_mmiotrace.c
> @@ -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?

>  	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;
>  }

This is fine.

> @@ -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.

>  	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 = {

OK.

>  	.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);
>  }

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

-- Steve

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

[Masami Hiramatsu]

On Fri, 7 Aug 2026 20:50:53 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:

> On Tue, 28 Jul 2026 21:50:19 +0900
> "Masami Hiramatsu (Google)" <mhiramat@kernel.org> wrote:
> 
> > From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> > 
> > 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) <mhiramat@kernel.org>
> > ---
> >  kernel/trace/trace_mmiotrace.c |   16 +++++++---------
> >  1 file changed, 7 insertions(+), 9 deletions(-)
> > 
> > diff --git a/kernel/trace/trace_mmiotrace.c b/kernel/trace/trace_mmiotrace.c
> > index 77120d467e11..ce16e1c53d12 100644
> > --- a/kernel/trace/trace_mmiotrace.c
> > +++ b/kernel/trace/trace_mmiotrace.c
> > @@ -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?

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

[...]
> > @@ -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);
> >  }
> 
> Honestly, for one line functions like the above, I think it looks better
> without that blank 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,

-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>

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

[Steven Rostedt]

On Sat, 8 Aug 2026 13:16:34 +0900
Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote:

> 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