Re: [PATCH v14 02/17] splice: Add a func to do a splice from a buffered file without ITER_PIPE

Ming Lei
2023-02-18T02:43:11Z

On Tue, Feb 14, 2023 at 05:13:15PM +0000, David Howells wrote:
> Provide a function to do splice read from a buffered file, pulling the
> folios out of the pagecache directly by calling filemap_get_pages() to do
> any required reading and then pasting the returned folios into the pipe.
>
> A helper function is provided to do the actual folio pasting and will
> handle multipage folios by splicing as many of the relevant subpages as
> will fit into the pipe.
>
> The code is loosely based on filemap_read() and might belong in
> mm/filemap.c with that as it needs to use filemap_get_pages().
>
> 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)
> - Rename to filemap_splice_read().
> - Create a helper, pipe_head_buf(), to get the head buffer.
> - Use init_sync_kiocb().
> - Move to mm/filemap.c.
> - Split the implementation of filemap_splice_read() from the patch to
> make generic_file_splice_read() use it and direct_splice_read().
>
> include/linux/fs.h | 3 ++
> mm/filemap.c | 128 +++++++++++++++++++++++++++++++++++++++++++++
> mm/internal.h | 6 +++
> 3 files changed, 137 insertions(+)
>
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index c1769a2c5d70..28743e38df91 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -3163,6 +3163,9 @@ ssize_t vfs_iocb_iter_write(struct file *file, struct kiocb *iocb,
> struct iov_iter *iter);
>
> /* fs/splice.c */
> +ssize_t filemap_splice_read(struct file *in, loff_t *ppos,
> + struct pipe_inode_info *pipe,
> + size_t len, unsigned int flags);
> extern ssize_t generic_file_splice_read(struct file *, loff_t *,
> struct pipe_inode_info *, size_t, unsigned int);
> extern ssize_t iter_file_splice_write(struct pipe_inode_info *,
> diff --git a/mm/filemap.c b/mm/filemap.c
> index 876e77278d2a..8c7b135c8e23 100644
> --- a/mm/filemap.c
> +++ b/mm/filemap.c
> @@ -42,6 +42,8 @@
> #include <linux/ramfs.h>
> #include <linux/page_idle.h>
> #include <linux/migrate.h>
> +#include <linux/pipe_fs_i.h>
> +#include <linux/splice.h>
> #include <asm/pgalloc.h>
> #include <asm/tlbflush.h>
> #include "internal.h"
> @@ -2842,6 +2844,132 @@ generic_file_read_iter(struct kiocb *iocb, struct iov_iter *iter)
> }
> EXPORT_SYMBOL(generic_file_read_iter);
>
> +/*
> + * Splice subpages from a folio into a pipe.
> + */
> +size_t splice_folio_into_pipe(struct pipe_inode_info *pipe,
> + struct folio *folio, loff_t fpos, size_t size)
> +{
> + struct page *page;
> + size_t spliced = 0, offset = offset_in_folio(folio, fpos);
> +
> + page = folio_page(folio, offset / PAGE_SIZE);
> + size = min(size, folio_size(folio) - offset);
> + offset %= PAGE_SIZE;
> +
> + while (spliced < size &&
> + !pipe_full(pipe->head, pipe->tail, pipe->max_usage)) {
> + struct pipe_buffer *buf = pipe_head_buf(pipe);
> + size_t part = min_t(size_t, PAGE_SIZE - offset, size - spliced);
> +
> + *buf = (struct pipe_buffer) {
> + .ops = &page_cache_pipe_buf_ops,
> + .page = page,
> + .offset = offset,
> + .len = part,
> + };
> + folio_get(folio);
> + pipe->head++;
> + page++;
> + spliced += part;
> + offset = 0;

It should be better to replace above with add_to_pipe().

> + }
> +
> + return spliced;
> +}
> +
> +/*
> + * Splice folios from the pagecache of a buffered (ie. non-O_DIRECT) file into
> + * a pipe.
> + */
> +ssize_t filemap_splice_read(struct file *in, loff_t *ppos,
> + struct pipe_inode_info *pipe,
> + size_t len, unsigned int flags)
> +{
> + struct folio_batch fbatch;
> + struct kiocb iocb;
> + size_t total_spliced = 0, used, npages;
> + loff_t isize, end_offset;
> + bool writably_mapped;
> + int i, error = 0;
> +
> + init_sync_kiocb(&iocb, in);
> + iocb.ki_pos = *ppos;
> +
> + /* Work out how much data we can actually add into the pipe */
> + used = pipe_occupancy(pipe->head, pipe->tail);
> + npages = max_t(ssize_t, pipe->max_usage - used, 0);
> + len = min_t(size_t, len, npages * PAGE_SIZE);

Do we need to consider offset in 1st page here?

thanks,
Ming