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

Masami Hiramatsu (Google) <[email protected]>
Newsgroups org.kernel.vger.linux-trace-kernel,org.kernel.vger.linux-kernel
Message-ID <[email protected]>
On Tue, 21 Jul 2026 21:20:10 -0400
Steven Rostedt <[email protected]> wrote:

> From: Steven Rostedt <[email protected]>
> 
> 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/[email protected]/
> Link: https://lore.kernel.org/all/[email protected]/
> 
> Signed-off-by: Steven Rostedt <[email protected]>
> ---
>  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) <[email protected]>
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.