[PATCH] tracing: Do not clean up hiter in mmiotrace read function

[PATCH] tracing: Do not clean up hiter in mmiotrace read function

[Steven Rostedt]

From: Steven Rostedt <rostedt@goodmis.org>

When the mmiotrace trace was first created, it allocated a descriptor in
its pipe_open() method. Since there was no pipe_close() method when it was
created (in May of 2008, and pipe_close() was added in December of 2009),
it cleaned up the allocated descriptors in the read.

Now that the clean up is in the pipe_close() method that now exists,
remove the clean up from the read as it is no longer needed.

Also simplify the code by inverting the early exit conditional into a
conditional to perform the logic and get rid of the goto.

Link: https://lore.kernel.org/all/20260715143604.14481-1-gaikwad.dcg@gmail.com/
Link: https://lore.kernel.org/all/20260721211143.36dbd559@gandalf.local.home/

Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 kernel/trace/trace_mmiotrace.c | 14 +++-----------
 1 file changed, 3 insertions(+), 11 deletions(-)

diff --git a/kernel/trace/trace_mmiotrace.c b/kernel/trace/trace_mmiotrace.c
index b88b8d9923ad..ba604c22d2d2 100644
--- a/kernel/trace/trace_mmiotrace.c
+++ b/kernel/trace/trace_mmiotrace.c
@@ -142,21 +142,13 @@ static ssize_t mmio_read(struct trace_iterator *iter, struct file *filp,
 		if (!overrun_detected)
 			pr_warn("mmiotrace has lost events\n");
 		overrun_detected = true;
-		goto print_out;
 	}
 
-	if (!hiter || !hiter->dev)
-		return 0;
-
-	mmio_print_pcidev(s, hiter->dev);
-	hiter->dev = pci_get_device(PCI_ANY_ID, PCI_ANY_ID, hiter->dev);
-
-	if (!hiter->dev) {
-		destroy_header_iter(hiter);
-		iter->private = NULL;
+	if (hiter && hiter->dev) {
+		mmio_print_pcidev(s, hiter->dev);
+		hiter->dev = pci_get_device(PCI_ANY_ID, PCI_ANY_ID, hiter->dev);
 	}
 
-print_out:
 	ret = trace_seq_to_user(s, ubuf, cnt);
 	return (ret == -EBUSY) ? 0 : ret;
 }
-- 
2.53.0

Re: [PATCH] tracing: Do not clean up hiter in mmiotrace read function

[Masami Hiramatsu]

On Tue, 21 Jul 2026 21:20:10 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:

> From: Steven Rostedt <rostedt@goodmis.org>
> 
> When the mmiotrace trace was first created, it allocated a descriptor in
> its pipe_open() method. Since there was no pipe_close() method when it was
> created (in May of 2008, and pipe_close() was added in December of 2009),
> it cleaned up the allocated descriptors in the read.
> 
> Now that the clean up is in the pipe_close() method that now exists,
> remove the clean up from the read as it is no longer needed.
> 
> Also simplify the code by inverting the early exit conditional into a
> conditional to perform the logic and get rid of the goto.
> 
> Link: https://lore.kernel.org/all/20260715143604.14481-1-gaikwad.dcg@gmail.com/
> Link: https://lore.kernel.org/all/20260721211143.36dbd559@gandalf.local.home/
> 
> Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
> ---
>  kernel/trace/trace_mmiotrace.c | 14 +++-----------
>  1 file changed, 3 insertions(+), 11 deletions(-)
> 
> diff --git a/kernel/trace/trace_mmiotrace.c b/kernel/trace/trace_mmiotrace.c
> index b88b8d9923ad..ba604c22d2d2 100644
> --- a/kernel/trace/trace_mmiotrace.c
> +++ b/kernel/trace/trace_mmiotrace.c
> @@ -142,21 +142,13 @@ static ssize_t mmio_read(struct trace_iterator *iter, struct file *filp,
>  		if (!overrun_detected)
>  			pr_warn("mmiotrace has lost events\n");
>  		overrun_detected = true;
> -		goto print_out;

Is this intentional change? Removing this goto means we will change
the hiter->dev even if overrun happens. Previously we can resume
output in the next read for current hiter->dev, but this will skip
the current hiter->dev?

Thanks,

>  	}
>  
> -	if (!hiter || !hiter->dev)
> -		return 0;
> -
> -	mmio_print_pcidev(s, hiter->dev);
> -	hiter->dev = pci_get_device(PCI_ANY_ID, PCI_ANY_ID, hiter->dev);
> -
> -	if (!hiter->dev) {
> -		destroy_header_iter(hiter);
> -		iter->private = NULL;
> +	if (hiter && hiter->dev) {
> +		mmio_print_pcidev(s, hiter->dev);
> +		hiter->dev = pci_get_device(PCI_ANY_ID, PCI_ANY_ID, hiter->dev);
>  	}
>  
> -print_out:
>  	ret = trace_seq_to_user(s, ubuf, cnt);
>  	return (ret == -EBUSY) ? 0 : ret;
>  }
> -- 
> 2.53.0
> 


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