Re: [PATCH] erofs: use the shared page cache for splice in inode_share mode

[Jingbo Xu <jefflexu@linux.alibaba.com>]

On 8/20/26 2:44 PM, Zhan Xusheng wrote:
> From: Zhan Xusheng <zhanxusheng1024@gmail.com>
> 
> From: Zhan Xusheng <zhanxusheng@xiaomi.com>
> 
> erofs_ishare_fops routes everything that touches the page cache to the
> backing file in ->private_data: read_iter clones the iocb onto it, mmap
> does vma_set_file(), fadvise calls vfs_fadvise() on it.  splice_read was
> left as filemap_splice_read(), which works on the user file's own mapping.
> 
> filemap_splice_read() does init_sync_kiocb(&iocb, in), and
> filemap_get_pages() then takes iocb->ki_filp->f_mapping, so splice() and
> sendfile() populate the per-file page cache from disk instead of using the
> shared one.  The data is correct, since erofs_fill_inode() sets that
> mapping's a_ops either way, but the same content ends up cached twice,
> which is what inode_share is there to avoid.
> 
> Pass the backing file, as read_iter already does.
> 
> Fixes: 5ef3208e3be5 ("erofs: introduce the page cache share feature")
> Link: https://lore.kernel.org/all/b7dc7192-d586-45a2-bc4a-b41dc681c9bb@linux.alibaba.com/
> Signed-off-by: Zhan Xusheng <zhanxusheng@xiaomi.com>
> ---
>  fs/erofs/ishare.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/erofs/ishare.c b/fs/erofs/ishare.c
> index fa7d4112dec5..01dc53e9e3ad 100644
> --- a/fs/erofs/ishare.c
> +++ b/fs/erofs/ishare.c
> @@ -148,6 +148,13 @@ static int erofs_ishare_mmap(struct file *file, struct vm_area_struct *vma)
>  	return generic_file_readonly_mmap(file, vma);
>  }
>  
> +static ssize_t erofs_ishare_splice_read(struct file *in, loff_t *ppos,
> +					struct pipe_inode_info *pipe,
> +					size_t len, unsigned int flags)
> +{
> +	return filemap_splice_read(in->private_data, ppos, pipe, len, flags);

Please refer to backing_file_splice_read() called from
ovl_splice_read(), file_accessed() needs to be called on the original
file (just as what .read_iter() i.e. filemap_read() does), and the input
@ppos needs to be updated accordingly.


-- 
Thanks,
Jingbo