All of lore.kernel.org
 help / color / mirror / Atom feed
From: Gao Xiang <xiang@kernel.org>
To: Zhan Xusheng <zhanxusheng1024@gmail.com>
Cc: Gao Xiang <xiang@kernel.org>, Chao Yu <chao@kernel.org>,
	Jingbo Xu <jefflexu@linux.alibaba.com>,
	zhanxusheng@xiaomi.com, linux-erofs@lists.ozlabs.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] erofs: use the shared page cache for splice in inode_share mode
Date: Thu, 20 Aug 2026 21:29:23 +0800	[thread overview]
Message-ID: <aocBM-43e9Ea_vDK@XiangdeMacBook-Pro.local> (raw)
In-Reply-To: <20260820064441.1083470-1-zhanxusheng@xiaomi.com>

Hi Xusheng,

On Thu, Aug 20, 2026 at 02:44:41PM +0800, Zhan Xusheng wrote:
> From: Zhan Xusheng <zhanxusheng1024@gmail.com>
> 
> From: Zhan Xusheng <zhanxusheng@xiaomi.com>
> 

the subject can be improved as "erofs: support splice() in inode_share mode"

> 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>

I observed the sashiko's report too:
https://sashiko.dev/#/patchset/20260818113713.116849-1-jefflexu%40linux.alibaba.com

Regardless of the implementation details, I don't think it is a fix
since this feature should be considered as a best-effort approach.

If it's an enhancement, please also write an erofs-utils test and
drop the fixes tag.

Thanks,
Gao Xiang


      parent reply	other threads:[~2026-08-20 13:29 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-20  6:44 [PATCH] erofs: use the shared page cache for splice in inode_share mode Zhan Xusheng
2026-08-20  9:53 ` Jingbo Xu
2026-08-20 12:37   ` Zhan Xusheng
2026-08-20 13:34     ` Jingbo Xu
2026-08-20 13:49       ` Gao Xiang
2026-08-20 13:55         ` Jingbo Xu
2026-08-20 13:29 ` Gao Xiang [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aocBM-43e9Ea_vDK@XiangdeMacBook-Pro.local \
    --to=xiang@kernel.org \
    --cc=chao@kernel.org \
    --cc=jefflexu@linux.alibaba.com \
    --cc=linux-erofs@lists.ozlabs.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=zhanxusheng1024@gmail.com \
    --cc=zhanxusheng@xiaomi.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.