Re: [PATCH v21 26/30] splice: Convert trace/seq to use copy_splice_read()

[Google]

Hi David,

On Sat, 20 May 2023 01:00:45 +0100
David Howells <dhowells@xxxxxxxxxx> wrote:

> For the splice from the trace seq buffer, just use copy_splice_read().

So this is because you will remove generic_file_splice_read() (since
it's buggy), right?

> 
> In the future, something better can probably be done by gifting pages from
> seq->buf into the pipe, but that would require changing seq->buf into a
> vmap over an array of pages.

So what we need is to introduce a vmap? We introduced splice support for
avoiding copy ringbuffer pages, but this drops it. Thus this will drop
performance of splice on ring buffer (trace file). If it is correct,
can you also add a note about that?

Thank you,

> 
> Signed-off-by: David Howells <dhowells@xxxxxxxxxx>
> cc: Christoph Hellwig <hch@xxxxxx>
> cc: Al Viro <viro@xxxxxxxxxxxxxxxxxx>
> cc: Jens Axboe <axboe@xxxxxxxxx>
> cc: Steven Rostedt <rostedt@xxxxxxxxxxx>
> cc: Masami Hiramatsu <mhiramat@xxxxxxxxxx>
> cc: linux-kernel@xxxxxxxxxxxxxxx
> cc: linux-trace-kernel@xxxxxxxxxxxxxxx
> cc: linux-fsdevel@xxxxxxxxxxxxxxx
> cc: linux-block@xxxxxxxxxxxxxxx
> cc: linux-mm@xxxxxxxxx
> ---
>  kernel/trace/trace.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> index ebc59781456a..c210d02fac97 100644
> --- a/kernel/trace/trace.c
> +++ b/kernel/trace/trace.c
> @@ -5171,7 +5171,7 @@ static const struct file_operations tracing_fops = {
>  	.open		= tracing_open,
>  	.read		= seq_read,
>  	.read_iter	= seq_read_iter,
> -	.splice_read	= generic_file_splice_read,
> +	.splice_read	= copy_splice_read,
>  	.write		= tracing_write_stub,
>  	.llseek		= tracing_lseek,
>  	.release	= tracing_release,
> 


-- 
Masami Hiramatsu (Google) <mhiramat@xxxxxxxxxx>