Re: [PATCH 2/3] ovl: support cachestat() syscall on overlayfs files

On 6/23/26 19:12, Nhat Pham wrote:
> On Tue, Jun 23, 2026 at 4:15 AM Pavel Tikhomirov
> <[email protected]> wrote:
>>
>> Overlayfs forwards data I/O to the real (upper/lower) file, so the page
>> cache lives in the real inode's mapping and cachestat() on an overlay
>> fd returned all zeroes.
>>
>> Implement the ->cachestat() file operation by forwarding to the real
>> file via vfs_cachestat(), the same way ovl_fadvise() forwards
>> for fadvise.
>>
>> Signed-off-by: Pavel Tikhomirov <[email protected]>
>> ---
>>  fs/overlayfs/file.c | 18 ++++++++++++++++++
>>  1 file changed, 18 insertions(+)
>>
>> diff --git a/fs/overlayfs/file.c b/fs/overlayfs/file.c
>> index 27cc07738f33b..a7e252a91ea43 100644
>> --- a/fs/overlayfs/file.c
>> +++ b/fs/overlayfs/file.c
>> @@ -518,6 +518,21 @@ static int ovl_fadvise(struct file *file, loff_t offset, loff_t len, int advice)
>>                 return vfs_fadvise(realfile, offset, len, advice);
>>  }
>>
>> +#ifdef CONFIG_CACHESTAT_SYSCALL
>> +static int ovl_cachestat(struct file *file, struct cachestat_range *csr,
>> +                        struct cachestat *cs)
>> +{
>> +       struct file *realfile;
>> +
>> +       realfile = ovl_real_file(file);
>> +       if (IS_ERR(realfile))
>> +               return PTR_ERR(realfile);
> 
> We're propagating the error of ovl_real_file() all the way to
> userspace right? I think we need to handle this.
> 
> For example, we might get -EIO here, which is unexpected and
> undocumented from cachestat's POV.
> 
> Maybe handle it and just return -EBADF or sth like that (with some
> updated documentations, etc.)
> 
> The rest LGTM, but I'll let overlayfs maintainers check the
> overlayfs-specific bits :)

Yeh, we probably can use EBADF here instead of propagating:

Man cachestat(2) says:

  EBADF  Invalid file descriptor.

not really a bad fd here, but probably close enough not to rewrite man.

I'm a bit hesitant though, since in other overlayfs operations we already
propagate, maybe that was by design?

> 
>> +
>> +       with_ovl_creds(file_inode(file)->i_sb)
>> +               return vfs_cachestat(realfile, csr, cs);
>> +}

-- 
Best regards, Pavel Tikhomirov
Senior Software Developer, Virtuozzo.