[PATCH v2] erofs: support splice() in inode_share mode

[PATCH v2] erofs: support splice() in inode_share mode

[Zhan Xusheng]

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:
it does init_sync_kiocb(&iocb, in), and filemap_get_pages() then takes
iocb->ki_filp->f_mapping.

So splice() and sendfile() fill the per-inode page cache rather than the
shared one.  The content is the same either way, since erofs_fill_inode()
sets a_ops on that mapping too, which is why this went unnoticed.  Two
identical 8 MiB files under inode_share, reading one of them with splice(2)
alone, in pages:

  before   own 2048   shared 0
  after    own 0      shared 2048

"own" is cachestat(fd), which reports the file's own mapping; "shared" is
mmap()+mincore(), which erofs_ishare_mmap() redirects to the backing file.

Read through the backing file, as read_iter already does.

Link: https://lore.kernel.org/all/b7dc7192-d586-45a2-bc4a-b41dc681c9bb@linux.alibaba.com/
Signed-off-by: Zhan Xusheng <zhanxusheng@xiaomi.com>
---
v1->v2:
- Retitled and dropped the Fixes tag: the sharing is best effort, so
  this is an enhancement rather than a fix.
- Added the measured before/after.
- Left file_accessed() alone, since it is a no-op under SB_NOATIME.
- An erofs-utils test is sent separately against experimental-tests.

v1: https://lore.kernel.org/all/20260820064441.1083470-1-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);
+}
+
 static int erofs_ishare_fadvise(struct file *file, loff_t offset,
 				loff_t len, int advice)
 {
@@ -161,7 +168,7 @@ const struct file_operations erofs_ishare_fops = {
 	.mmap		= erofs_ishare_mmap,
 	.release	= erofs_ishare_file_release,
 	.get_unmapped_area = thp_get_unmapped_area,
-	.splice_read	= filemap_splice_read,
+	.splice_read	= erofs_ishare_splice_read,
 	.fadvise	= erofs_ishare_fadvise,
 };
 
-- 
2.43.0

Re: [PATCH v2] erofs: support splice() in inode_share mode

[Jingbo Xu]

On 8/21/26 9:56 AM, 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:
> it does init_sync_kiocb(&iocb, in), and filemap_get_pages() then takes
> iocb->ki_filp->f_mapping.
> 
> So splice() and sendfile() fill the per-inode page cache rather than the
> shared one.  The content is the same either way, since erofs_fill_inode()
> sets a_ops on that mapping too, which is why this went unnoticed.  Two
> identical 8 MiB files under inode_share, reading one of them with splice(2)
> alone, in pages:
> 
>   before   own 2048   shared 0
>   after    own 0      shared 2048
> 
> "own" is cachestat(fd), which reports the file's own mapping; "shared" is
> mmap()+mincore(), which erofs_ishare_mmap() redirects to the backing file.
> 
> Read through the backing file, as read_iter already does.
> 
> Link: https://lore.kernel.org/all/b7dc7192-d586-45a2-bc4a-b41dc681c9bb@linux.alibaba.com/
> Signed-off-by: Zhan Xusheng <zhanxusheng@xiaomi.com>

LGTM.

Reviewed-by: Jingbo Xu \<jefflexu@linux.alibaba.com\>

> ---
> v1->v2:
> - Retitled and dropped the Fixes tag: the sharing is best effort, so
>   this is an enhancement rather than a fix.
> - Added the measured before/after.
> - Left file_accessed() alone, since it is a no-op under SB_NOATIME.
> - An erofs-utils test is sent separately against experimental-tests.
> 
> v1: https://lore.kernel.org/all/20260820064441.1083470-1-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);
> +}
> +
>  static int erofs_ishare_fadvise(struct file *file, loff_t offset,
>  				loff_t len, int advice)
>  {
> @@ -161,7 +168,7 @@ const struct file_operations erofs_ishare_fops = {
>  	.mmap		= erofs_ishare_mmap,
>  	.release	= erofs_ishare_file_release,
>  	.get_unmapped_area = thp_get_unmapped_area,
> -	.splice_read	= filemap_splice_read,
> +	.splice_read	= erofs_ishare_splice_read,
>  	.fadvise	= erofs_ishare_fadvise,
>  };
>  

-- 
Thanks,
Jingbo

Re: [PATCH v2] erofs: support splice() in inode_share mode

[Chao Yu]

On 8/21/26 09:56, 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:
> it does init_sync_kiocb(&iocb, in), and filemap_get_pages() then takes
> iocb->ki_filp->f_mapping.
> 
> So splice() and sendfile() fill the per-inode page cache rather than the
> shared one.  The content is the same either way, since erofs_fill_inode()
> sets a_ops on that mapping too, which is why this went unnoticed.  Two
> identical 8 MiB files under inode_share, reading one of them with splice(2)
> alone, in pages:
> 
>    before   own 2048   shared 0
>    after    own 0      shared 2048
> 
> "own" is cachestat(fd), which reports the file's own mapping; "shared" is
> mmap()+mincore(), which erofs_ishare_mmap() redirects to the backing file.
> 
> Read through the backing file, as read_iter already does.
> 
> Link: https://lore.kernel.org/all/b7dc7192-d586-45a2-bc4a-b41dc681c9bb@linux.alibaba.com/
> Signed-off-by: Zhan Xusheng <zhanxusheng@xiaomi.com>

Reviewed-by: Chao Yu <chao@kernel.org>

Thanks,