On Wed, Jun 24, 2026 at 7:16 AM Amir Goldstein <[email protected]> wrote: > > On Wed, Jun 24, 2026 at 1:45 PM Pavel Tikhomirov > <[email protected]> wrote: > > > > > > > > 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. > > Please don't do that. > > Re-read what you just wrote - it is ridiculous > Because of being lazy to update man page, > we are going to send a confusing error to user which tells them > that their fd is wrong, which it is not.
I don't think we're being lazy here. It's technically more work to handle errors and updating documentations :) I'm more concerned with undocumented/unexpected behavior (error type in this case). -EIO was an example that I saw in ovl_real_file() itself, but I'm not familiar enough with overlayfs to know if that's the extent of it. But I'm OK with just updating the documentation with a simple note that other error maybe propagated from the underlying fs, if no one else thinks it's a problem :) > > > > > I'm a bit hesitant though, since in other overlayfs operations we already > > propagate, maybe that was by design? > > > > Exactly, plenty of overlayfs operations return EIO for unexpected > conditions, often accompanied with some assertion as is the case > with ovl_real_file(). > > Even though many man pages don't document an explicit EIO error > code, it is obvious to any experienced sys admin that if EIO is observed > they should look at the kernel logs, because an underlying subsystem > may have reported critical errors. > > But in general, man pages follow development, not the other way around. Fair point. > > Thanks, > Amir.
