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

On 8/20/26 9:49 PM, Gao Xiang wrote:
> On Thu, Aug 20, 2026 at 09:34:49PM +0800, Jingbo Xu wrote:
>>
>>
>> On 8/20/26 8:37 PM, Zhan Xusheng wrote:
>>> On Thu, 20 Aug 2026 17:53:20 +0800, Jingbo Xu wrote:
>>>> 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.
>>>
>>> Taking the file_accessed() one, thanks.  filemap_splice_read() calls it at
>>> mm/filemap.c:3155 on whatever file it was handed, so on the backing file,
>>> whereas backing_file_splice_read() ends in ctx->accessed(iocb->ki_filp),
>>> which for ovl_splice_read() is the original.  v2 adds file_accessed(in).
>>>
>>> @ppos looks already handled to me.  filemap_splice_read() takes a loff_t *
>>> and advances it itself, at mm/filemap.c:3144; its internal kiocb is seeded
>>> from *ppos at 3083 and 3098, not the other way round.  ovl_splice_read()
>>> has to copy iocb.ki_pos back because backing_file_splice_read() takes a
>>> struct kiocb and hands &iocb->ki_pos to vfs_splice_read().  Say if I have
>>> that wrong.
>>
>> Make sense.
>>
>>
>>>
>>> One you may want for read_iter too: it clones the kiocb onto the backing
>>> file, so filemap_read() marks that one accessed rather than the user's
>>> file, which is the shape splice_read had.  Neither is observable today,
>>> since erofs_fc_fill_super() sets SB_RDONLY | SB_NOATIME and the backing
>>> file is opened O_NOATIME, so both reach a no-op.  That is why I left
>>> read_iter alone here.
>>
>> Okay, it seems that file_accessed() shall also be added to
>> erofs_ishare_file_read_iter()?
> 
> I think file_accessed() is a no-op for erofs?

Right. erofs unconditionally sets SB_NOATIME (sb->s_flags |= SB_RDONLY |
SB_NOATIME).  Please ignore the noise..

-- 
Thanks,
Jingbo