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