Re: [PATCH v14 08/17] splice: Do splice read from a file without using ITER_PIPE

From: Ming Lei <ming.lei@redhat.com>
To: David Howells <dhowells@redhat.com>
Cc: Jens Axboe <axboe@kernel.dk>, Al Viro <viro@zeniv.linux.org.uk>,
	Christoph Hellwig <hch@infradead.org>,
	Matthew Wilcox <willy@infradead.org>, Jan Kara <jack@suse.cz>,
	Jeff Layton <jlayton@kernel.org>,
	David Hildenbrand <david@redhat.com>,
	Jason Gunthorpe <jgg@nvidia.com>,
	Logan Gunthorpe <logang@deltatee.com>,
	Hillf Danton <hdanton@sina.com>,
	linux-fsdevel@vger.kernel.org, linux-block@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-mm@kvack.org,
	Christoph Hellwig <hch@lst.de>,
	John Hubbard <jhubbard@nvidia.com>,
	ming.lei@redhat.com
Subject: Re: [PATCH v14 08/17] splice: Do splice read from a file without using ITER_PIPE
Date: Fri, 17 Feb 2023 16:22:14 +0800	[thread overview]
Message-ID: <Y+85Ni9CH/7ajQga@T590> (raw)
In-Reply-To: <20230214171330.2722188-9-dhowells@redhat.com>

On Tue, Feb 14, 2023 at 05:13:21PM +0000, David Howells wrote:
> Make generic_file_splice_read() use filemap_splice_read() and
> direct_splice_read() rather than using an ITER_PIPE and call_read_iter().
> 
> With this, ITER_PIPE is no longer used.
> 
> Signed-off-by: David Howells <dhowells@redhat.com>
> cc: Jens Axboe <axboe@kernel.dk>
> cc: Christoph Hellwig <hch@lst.de>
> cc: Al Viro <viro@zeniv.linux.org.uk>
> cc: David Hildenbrand <david@redhat.com>
> cc: John Hubbard <jhubbard@nvidia.com>
> cc: linux-mm@kvack.org
> cc: linux-block@vger.kernel.org
> cc: linux-fsdevel@vger.kernel.org
> ---
> 
> Notes:
>     ver #14)
>     - Split out filemap_splice_read() into a separate patch.
> 
>  fs/splice.c | 30 +++++++-----------------------
>  1 file changed, 7 insertions(+), 23 deletions(-)
> 
> diff --git a/fs/splice.c b/fs/splice.c
> index 4c6332854b63..a93478338cec 100644
> --- a/fs/splice.c
> +++ b/fs/splice.c
> @@ -391,29 +391,13 @@ ssize_t generic_file_splice_read(struct file *in, loff_t *ppos,
>  				 struct pipe_inode_info *pipe, size_t len,
>  				 unsigned int flags)
>  {
> -	struct iov_iter to;
> -	struct kiocb kiocb;
> -	int ret;
> -
> -	iov_iter_pipe(&to, ITER_DEST, pipe, len);
> -	init_sync_kiocb(&kiocb, in);
> -	kiocb.ki_pos = *ppos;
> -	ret = call_read_iter(in, &kiocb, &to);
> -	if (ret > 0) {
> -		*ppos = kiocb.ki_pos;
> -		file_accessed(in);
> -	} else if (ret < 0) {
> -		/* free what was emitted */
> -		pipe_discard_from(pipe, to.start_head);
> -		/*
> -		 * callers of ->splice_read() expect -EAGAIN on
> -		 * "can't put anything in there", rather than -EFAULT.
> -		 */
> -		if (ret == -EFAULT)
> -			ret = -EAGAIN;
> -	}
> -
> -	return ret;
> +	if (unlikely(*ppos >= file_inode(in)->i_sb->s_maxbytes))
> +		return 0;
> +	if (unlikely(!len))
> +		return 0;
> +	if (in->f_flags & O_DIRECT)
> +		return direct_splice_read(in, ppos, pipe, len, flags);

Hello David,

I have one question, for dio, pages need to map to userspace
memory, but direct_splice_read() just allocates pages and not
see when the user mapping is setup, can you give one hint?

Thanks, 
Ming