* [PATCH 1/3] fs, mm: add ->cachestat() file operation
2026-06-23 11:14 [PATCH 0/3] ovl: make cachestat() work Pavel Tikhomirov
@ 2026-06-23 11:14 ` Pavel Tikhomirov
2026-06-23 13:48 ` Johannes Weiner
2026-06-23 16:01 ` Nhat Pham
2026-06-23 11:14 ` [PATCH 2/3] ovl: support cachestat() syscall on overlayfs files Pavel Tikhomirov
` (2 subsequent siblings)
3 siblings, 2 replies; 25+ messages in thread
From: Pavel Tikhomirov @ 2026-06-23 11:14 UTC (permalink / raw)
To: Miklos Szeredi, Amir Goldstein
Cc: Alexander Viro, Christian Brauner, Jan Kara,
Matthew Wilcox (Oracle),
Andrew Morton, Nhat Pham, Johannes Weiner, Shuah Khan,
linux-unionfs, linux-kernel, linux-fsdevel, linux-mm,
linux-kselftest, Pavel Tikhomirov
The cachestat() syscall reads page cache statistics straight from the
file's f_mapping. Stackable filesystems such as overlayfs keep the data
pages in an underlying inode's mapping rather than in the overlay
inode's, so cachestat() reports all zeroes for them.
Add a ->cachestat() file operation and route the syscall through a new
vfs_cachestat() helper that calls it when present, falling back to
file's f_mapping otherwise. This lets stackable filesystems forward the
query to the file that actually owns the page cache. No behaviour change
for regular files.
Signed-off-by: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>
---
Note: Memset change might be a bit tricky, I moved it to no
->cachestat() path to avoid multiple memset on nested overlayfs, that
means that ->cachestat() is expected to be able to handle unitialized
cs.
---
include/linux/fs.h | 10 ++++++++++
mm/filemap.c | 43 +++++++++++++++++++++++++++++++++++--------
2 files changed, 45 insertions(+), 8 deletions(-)
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 6da44573ce450..966b6564707e4 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -53,6 +53,8 @@
struct bdi_writeback;
struct bio;
+struct cachestat_range;
+struct cachestat;
struct io_comp_batch;
struct fiemap_extent_info;
struct kiocb;
@@ -1963,6 +1965,8 @@ struct file_operations {
struct file *file_out, loff_t pos_out,
loff_t len, unsigned int remap_flags);
int (*fadvise)(struct file *, loff_t, loff_t, int);
+ int (*cachestat)(struct file *file, struct cachestat_range *csr,
+ struct cachestat *cs);
int (*uring_cmd)(struct io_uring_cmd *ioucmd, unsigned int issue_flags);
int (*uring_cmd_iopoll)(struct io_uring_cmd *, struct io_comp_batch *,
unsigned int poll_flags);
@@ -3633,6 +3637,12 @@ extern int vfs_fadvise(struct file *file, loff_t offset, loff_t len,
extern int generic_fadvise(struct file *file, loff_t offset, loff_t len,
int advice);
+/* mm/filemap.c */
+#ifdef CONFIG_CACHESTAT_SYSCALL
+int vfs_cachestat(struct file *file, struct cachestat_range *csr,
+ struct cachestat *cs);
+#endif
+
static inline bool vfs_empty_path(int dfd, const char __user *path)
{
char c;
diff --git a/mm/filemap.c b/mm/filemap.c
index 7e467c81d2138..90608c6b1ce55 100644
--- a/mm/filemap.c
+++ b/mm/filemap.c
@@ -4714,6 +4714,37 @@ static inline bool can_do_cachestat(struct file *f)
return file_permission(f, MAY_WRITE) == 0;
}
+/**
+ * vfs_cachestat() - query page cache statistics of a file
+ * @file: file to query
+ * @csr: byte range to query
+ * @cs: output statistics
+ *
+ * Compute the page cache statistics for the given byte range of @file.
+ *
+ * Stackable filesystems (e.g. overlayfs) keep the data pages in the
+ * mapping of an underlying file rather than in @file->f_mapping. Such
+ * filesystems provide a ->cachestat() file operation that forwards the
+ * query to the file that actually owns the page cache; otherwise the
+ * statistics are computed from @file->f_mapping directly.
+ */
+int vfs_cachestat(struct file *file, struct cachestat_range *csr,
+ struct cachestat *cs)
+{
+ pgoff_t first_index, last_index;
+
+ if (file->f_op->cachestat)
+ return file->f_op->cachestat(file, csr, cs);
+
+ first_index = csr->off >> PAGE_SHIFT;
+ last_index =
+ csr->len == 0 ? ULONG_MAX : (csr->off + csr->len - 1) >> PAGE_SHIFT;
+ memset(cs, 0, sizeof(struct cachestat));
+ filemap_cachestat(file->f_mapping, first_index, last_index, cs);
+ return 0;
+}
+EXPORT_SYMBOL(vfs_cachestat);
+
/*
* The cachestat(2) system call.
*
@@ -4753,10 +4784,9 @@ SYSCALL_DEFINE4(cachestat, unsigned int, fd,
struct cachestat __user *, cstat, unsigned int, flags)
{
CLASS(fd, f)(fd);
- struct address_space *mapping;
struct cachestat_range csr;
struct cachestat cs;
- pgoff_t first_index, last_index;
+ int ret;
if (fd_empty(f))
return -EBADF;
@@ -4775,12 +4805,9 @@ SYSCALL_DEFINE4(cachestat, unsigned int, fd,
if (flags != 0)
return -EINVAL;
- first_index = csr.off >> PAGE_SHIFT;
- last_index =
- csr.len == 0 ? ULONG_MAX : (csr.off + csr.len - 1) >> PAGE_SHIFT;
- memset(&cs, 0, sizeof(struct cachestat));
- mapping = fd_file(f)->f_mapping;
- filemap_cachestat(mapping, first_index, last_index, &cs);
+ ret = vfs_cachestat(fd_file(f), &csr, &cs);
+ if (ret)
+ return ret;
if (copy_to_user(cstat, &cs, sizeof(struct cachestat)))
return -EFAULT;
--
2.54.0
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH 1/3] fs, mm: add ->cachestat() file operation
2026-06-23 11:14 ` [PATCH 1/3] fs, mm: add ->cachestat() file operation Pavel Tikhomirov
@ 2026-06-23 13:48 ` Johannes Weiner
2026-06-23 14:55 ` Pavel Tikhomirov
2026-06-23 16:01 ` Nhat Pham
1 sibling, 1 reply; 25+ messages in thread
From: Johannes Weiner @ 2026-06-23 13:48 UTC (permalink / raw)
To: Pavel Tikhomirov
Cc: Miklos Szeredi, Amir Goldstein, Alexander Viro,
Christian Brauner, Jan Kara, Matthew Wilcox (Oracle),
Andrew Morton, Nhat Pham, Shuah Khan, linux-unionfs,
linux-kernel, linux-fsdevel, linux-mm, linux-kselftest
On Tue, Jun 23, 2026 at 01:14:48PM +0200, Pavel Tikhomirov wrote:
> The cachestat() syscall reads page cache statistics straight from the
> file's f_mapping. Stackable filesystems such as overlayfs keep the data
> pages in an underlying inode's mapping rather than in the overlay
> inode's, so cachestat() reports all zeroes for them.
>
> Add a ->cachestat() file operation and route the syscall through a new
> vfs_cachestat() helper that calls it when present, falling back to
> file's f_mapping otherwise. This lets stackable filesystems forward the
> query to the file that actually owns the page cache. No behaviour change
> for regular files.
>
> Signed-off-by: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>
> ---
> Note: Memset change might be a bit tricky, I moved it to no
> ->cachestat() path to avoid multiple memset on nested overlayfs, that
> means that ->cachestat() is expected to be able to handle unitialized
> cs.
> ---
> include/linux/fs.h | 10 ++++++++++
> mm/filemap.c | 43 +++++++++++++++++++++++++++++++++++--------
> 2 files changed, 45 insertions(+), 8 deletions(-)
>
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 6da44573ce450..966b6564707e4 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -53,6 +53,8 @@
>
> struct bdi_writeback;
> struct bio;
> +struct cachestat_range;
> +struct cachestat;
> struct io_comp_batch;
> struct fiemap_extent_info;
> struct kiocb;
> @@ -1963,6 +1965,8 @@ struct file_operations {
> struct file *file_out, loff_t pos_out,
> loff_t len, unsigned int remap_flags);
> int (*fadvise)(struct file *, loff_t, loff_t, int);
> + int (*cachestat)(struct file *file, struct cachestat_range *csr,
> + struct cachestat *cs);
I suppose you can't just have it return the real file because of the
with_ovl_creds() scope you need during access? That would make things
a bit easier. But short of that, this looks good to me.
Acked-by: Johannes Weiner <hannes@cmpxchg.org>
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH 1/3] fs, mm: add ->cachestat() file operation
2026-06-23 13:48 ` Johannes Weiner
@ 2026-06-23 14:55 ` Pavel Tikhomirov
2026-06-23 15:34 ` Amir Goldstein
0 siblings, 1 reply; 25+ messages in thread
From: Pavel Tikhomirov @ 2026-06-23 14:55 UTC (permalink / raw)
To: Johannes Weiner
Cc: Miklos Szeredi, Amir Goldstein, Alexander Viro,
Christian Brauner, Jan Kara, Matthew Wilcox (Oracle),
Andrew Morton, Nhat Pham, Shuah Khan, linux-unionfs,
linux-kernel, linux-fsdevel, linux-mm, linux-kselftest
On 6/23/26 15:48, Johannes Weiner wrote:
> On Tue, Jun 23, 2026 at 01:14:48PM +0200, Pavel Tikhomirov wrote:
>> The cachestat() syscall reads page cache statistics straight from the
>> file's f_mapping. Stackable filesystems such as overlayfs keep the data
>> pages in an underlying inode's mapping rather than in the overlay
>> inode's, so cachestat() reports all zeroes for them.
>>
>> Add a ->cachestat() file operation and route the syscall through a new
>> vfs_cachestat() helper that calls it when present, falling back to
>> file's f_mapping otherwise. This lets stackable filesystems forward the
>> query to the file that actually owns the page cache. No behaviour change
>> for regular files.
>>
>> Signed-off-by: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>
>> ---
>> Note: Memset change might be a bit tricky, I moved it to no
>> ->cachestat() path to avoid multiple memset on nested overlayfs, that
>> means that ->cachestat() is expected to be able to handle unitialized
>> cs.
>> ---
>> include/linux/fs.h | 10 ++++++++++
>> mm/filemap.c | 43 +++++++++++++++++++++++++++++++++++--------
>> 2 files changed, 45 insertions(+), 8 deletions(-)
>>
>> diff --git a/include/linux/fs.h b/include/linux/fs.h
>> index 6da44573ce450..966b6564707e4 100644
>> --- a/include/linux/fs.h
>> +++ b/include/linux/fs.h
>> @@ -53,6 +53,8 @@
>>
>> struct bdi_writeback;
>> struct bio;
>> +struct cachestat_range;
>> +struct cachestat;
>> struct io_comp_batch;
>> struct fiemap_extent_info;
>> struct kiocb;
>> @@ -1963,6 +1965,8 @@ struct file_operations {
>> struct file *file_out, loff_t pos_out,
>> loff_t len, unsigned int remap_flags);
>> int (*fadvise)(struct file *, loff_t, loff_t, int);
>> + int (*cachestat)(struct file *file, struct cachestat_range *csr,
>> + struct cachestat *cs);
>
> I suppose you can't just have it return the real file because of the
> with_ovl_creds() scope you need during access?
Yes, AFAIU in overlay when we use realfile we should always use it
with_ovl_creds(), even though I don't think there is anything cred related
in filemap_cachestat(), I still think we should follow the common pattern
other overlay helpers use (similar to ovl_fadvise() and ovl_flush()).
note: Actually some places get ovl_real_file() and use it without
with_ovl_creds(), e.g.: ovl_read_iter, ovl_write_iter, ovl_splice_read,
ovl_splice_write. But those look more of an exception than the general
rule. All other instances use with_ovl_creds().
Also there are simingly no other file_operations which return "realfile"
for further processing, mostly the operation from fsops simply replaces
general operation with its own logic completely.
Thanks for your review!
ps: Hope overlay maintainers will correctly if I'm getting this wrong.
> That would make things
> a bit easier. But short of that, this looks good to me.
>
> Acked-by: Johannes Weiner <hannes@cmpxchg.org>
--
Best regards, Pavel Tikhomirov
Senior Software Developer, Virtuozzo.
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH 1/3] fs, mm: add ->cachestat() file operation
2026-06-23 14:55 ` Pavel Tikhomirov
@ 2026-06-23 15:34 ` Amir Goldstein
2026-06-24 11:59 ` Pavel Tikhomirov
2026-06-25 10:36 ` Christian Brauner
0 siblings, 2 replies; 25+ messages in thread
From: Amir Goldstein @ 2026-06-23 15:34 UTC (permalink / raw)
To: Pavel Tikhomirov
Cc: Johannes Weiner, Miklos Szeredi, Alexander Viro,
Christian Brauner, Jan Kara, Matthew Wilcox (Oracle),
Andrew Morton, Nhat Pham, Shuah Khan, linux-unionfs,
linux-kernel, linux-fsdevel, linux-mm, linux-kselftest
On Tue, Jun 23, 2026 at 4:55 PM Pavel Tikhomirov
<ptikhomirov@virtuozzo.com> wrote:
>
>
>
> On 6/23/26 15:48, Johannes Weiner wrote:
> > On Tue, Jun 23, 2026 at 01:14:48PM +0200, Pavel Tikhomirov wrote:
> >> The cachestat() syscall reads page cache statistics straight from the
> >> file's f_mapping. Stackable filesystems such as overlayfs keep the data
> >> pages in an underlying inode's mapping rather than in the overlay
> >> inode's, so cachestat() reports all zeroes for them.
> >>
> >> Add a ->cachestat() file operation and route the syscall through a new
> >> vfs_cachestat() helper that calls it when present, falling back to
> >> file's f_mapping otherwise. This lets stackable filesystems forward the
> >> query to the file that actually owns the page cache. No behaviour change
> >> for regular files.
> >>
> >> Signed-off-by: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>
> >> ---
> >> Note: Memset change might be a bit tricky, I moved it to no
> >> ->cachestat() path to avoid multiple memset on nested overlayfs, that
> >> means that ->cachestat() is expected to be able to handle unitialized
> >> cs.
> >> ---
> >> include/linux/fs.h | 10 ++++++++++
> >> mm/filemap.c | 43 +++++++++++++++++++++++++++++++++++--------
> >> 2 files changed, 45 insertions(+), 8 deletions(-)
> >>
> >> diff --git a/include/linux/fs.h b/include/linux/fs.h
> >> index 6da44573ce450..966b6564707e4 100644
> >> --- a/include/linux/fs.h
> >> +++ b/include/linux/fs.h
> >> @@ -53,6 +53,8 @@
> >>
> >> struct bdi_writeback;
> >> struct bio;
> >> +struct cachestat_range;
> >> +struct cachestat;
> >> struct io_comp_batch;
> >> struct fiemap_extent_info;
> >> struct kiocb;
> >> @@ -1963,6 +1965,8 @@ struct file_operations {
> >> struct file *file_out, loff_t pos_out,
> >> loff_t len, unsigned int remap_flags);
> >> int (*fadvise)(struct file *, loff_t, loff_t, int);
> >> + int (*cachestat)(struct file *file, struct cachestat_range *csr,
> >> + struct cachestat *cs);
> >
> > I suppose you can't just have it return the real file because of the
> > with_ovl_creds() scope you need during access?
>
> Yes, AFAIU in overlay when we use realfile we should always use it
> with_ovl_creds(), even though I don't think there is anything cred related
> in filemap_cachestat(), I still think we should follow the common pattern
> other overlay helpers use (similar to ovl_fadvise() and ovl_flush()).
>
> note: Actually some places get ovl_real_file() and use it without
> with_ovl_creds(), e.g.: ovl_read_iter, ovl_write_iter, ovl_splice_read,
> ovl_splice_write. But those look more of an exception than the general
> rule. All other instances use with_ovl_creds().
Use with_ovl_creds() is a good practice to keep the mental security model,
but it is useless if the security check (can_do_cachestat) is not in the
vfs helper (vfs_cachestat), so please move it there.
Also it kind of makes more sense to check (flags != 0) in sys_cachestats
before checking permissions.
>
> Also there are simingly no other file_operations which return "realfile"
> for further processing, mostly the operation from fsops simply replaces
> general operation with its own logic completely.
>
> Thanks for your review!
>
> ps: Hope overlay maintainers will correctly if I'm getting this wrong.
>
I don't think this is wrong per-se, except for can_do_cachestat().
Just be aware that the real file could change from one cachestat
call to the next (i.e. due to copy up).
Thanks,
Amir.
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH 1/3] fs, mm: add ->cachestat() file operation
2026-06-23 15:34 ` Amir Goldstein
@ 2026-06-24 11:59 ` Pavel Tikhomirov
2026-06-24 14:11 ` Amir Goldstein
2026-06-25 10:36 ` Christian Brauner
1 sibling, 1 reply; 25+ messages in thread
From: Pavel Tikhomirov @ 2026-06-24 11:59 UTC (permalink / raw)
To: Amir Goldstein
Cc: Johannes Weiner, Miklos Szeredi, Alexander Viro,
Christian Brauner, Jan Kara, Matthew Wilcox (Oracle),
Andrew Morton, Nhat Pham, Shuah Khan, linux-unionfs,
linux-kernel, linux-fsdevel, linux-mm, linux-kselftest
On 6/23/26 17:34, Amir Goldstein wrote:
> On Tue, Jun 23, 2026 at 4:55 PM Pavel Tikhomirov
> <ptikhomirov@virtuozzo.com> wrote:
>>
>>
>>
>> On 6/23/26 15:48, Johannes Weiner wrote:
>>> On Tue, Jun 23, 2026 at 01:14:48PM +0200, Pavel Tikhomirov wrote:
>>>> The cachestat() syscall reads page cache statistics straight from the
>>>> file's f_mapping. Stackable filesystems such as overlayfs keep the data
>>>> pages in an underlying inode's mapping rather than in the overlay
>>>> inode's, so cachestat() reports all zeroes for them.
>>>>
>>>> Add a ->cachestat() file operation and route the syscall through a new
>>>> vfs_cachestat() helper that calls it when present, falling back to
>>>> file's f_mapping otherwise. This lets stackable filesystems forward the
>>>> query to the file that actually owns the page cache. No behaviour change
>>>> for regular files.
>>>>
>>>> Signed-off-by: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>
>>>> ---
>>>> Note: Memset change might be a bit tricky, I moved it to no
>>>> ->cachestat() path to avoid multiple memset on nested overlayfs, that
>>>> means that ->cachestat() is expected to be able to handle unitialized
>>>> cs.
>>>> ---
>>>> include/linux/fs.h | 10 ++++++++++
>>>> mm/filemap.c | 43 +++++++++++++++++++++++++++++++++++--------
>>>> 2 files changed, 45 insertions(+), 8 deletions(-)
>>>>
>>>> diff --git a/include/linux/fs.h b/include/linux/fs.h
>>>> index 6da44573ce450..966b6564707e4 100644
>>>> --- a/include/linux/fs.h
>>>> +++ b/include/linux/fs.h
>>>> @@ -53,6 +53,8 @@
>>>>
>>>> struct bdi_writeback;
>>>> struct bio;
>>>> +struct cachestat_range;
>>>> +struct cachestat;
>>>> struct io_comp_batch;
>>>> struct fiemap_extent_info;
>>>> struct kiocb;
>>>> @@ -1963,6 +1965,8 @@ struct file_operations {
>>>> struct file *file_out, loff_t pos_out,
>>>> loff_t len, unsigned int remap_flags);
>>>> int (*fadvise)(struct file *, loff_t, loff_t, int);
>>>> + int (*cachestat)(struct file *file, struct cachestat_range *csr,
>>>> + struct cachestat *cs);
>>>
>>> I suppose you can't just have it return the real file because of the
>>> with_ovl_creds() scope you need during access?
>>
>> Yes, AFAIU in overlay when we use realfile we should always use it
>> with_ovl_creds(), even though I don't think there is anything cred related
>> in filemap_cachestat(), I still think we should follow the common pattern
>> other overlay helpers use (similar to ovl_fadvise() and ovl_flush()).
>>
>> note: Actually some places get ovl_real_file() and use it without
>> with_ovl_creds(), e.g.: ovl_read_iter, ovl_write_iter, ovl_splice_read,
>> ovl_splice_write. But those look more of an exception than the general
>> rule. All other instances use with_ovl_creds().
>
> Use with_ovl_creds() is a good practice to keep the mental security model,
> but it is useless if the security check (can_do_cachestat) is not in the
> vfs helper (vfs_cachestat), so please move it there.
Totally, I feel stupid now that I missed it originally, will move it in v2.
>
> Also it kind of makes more sense to check (flags != 0) in sys_cachestats
> before checking permissions.
>
>>
>> Also there are simingly no other file_operations which return "realfile"
>> for further processing, mostly the operation from fsops simply replaces
>> general operation with its own logic completely.
>>
>> Thanks for your review!
>>
>> ps: Hope overlay maintainers will correctly if I'm getting this wrong.
>>
>
> I don't think this is wrong per-se, except for can_do_cachestat().
>
> Just be aware that the real file could change from one cachestat
> call to the next (i.e. due to copy up).
I guess the copy-up between two cachestat calls is an equivalent of cache pages
being dropped/reclaimed between the calls, right? (I assume old pages will not
be used anymore and will be reclaimed in due time.) So userspace should not be
overly confused by such a cache usage change.
>
> Thanks,
> Amir.
--
Best regards, Pavel Tikhomirov
Senior Software Developer, Virtuozzo.
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH 1/3] fs, mm: add ->cachestat() file operation
2026-06-24 11:59 ` Pavel Tikhomirov
@ 2026-06-24 14:11 ` Amir Goldstein
0 siblings, 0 replies; 25+ messages in thread
From: Amir Goldstein @ 2026-06-24 14:11 UTC (permalink / raw)
To: Pavel Tikhomirov
Cc: Johannes Weiner, Miklos Szeredi, Alexander Viro,
Christian Brauner, Jan Kara, Matthew Wilcox (Oracle),
Andrew Morton, Nhat Pham, Shuah Khan, linux-unionfs,
linux-kernel, linux-fsdevel, linux-mm, linux-kselftest
On Wed, Jun 24, 2026 at 1:59 PM Pavel Tikhomirov
<ptikhomirov@virtuozzo.com> wrote:
>
>
>
> On 6/23/26 17:34, Amir Goldstein wrote:
> > On Tue, Jun 23, 2026 at 4:55 PM Pavel Tikhomirov
> > <ptikhomirov@virtuozzo.com> wrote:
> >>
> >>
> >>
> >> On 6/23/26 15:48, Johannes Weiner wrote:
> >>> On Tue, Jun 23, 2026 at 01:14:48PM +0200, Pavel Tikhomirov wrote:
> >>>> The cachestat() syscall reads page cache statistics straight from the
> >>>> file's f_mapping. Stackable filesystems such as overlayfs keep the data
> >>>> pages in an underlying inode's mapping rather than in the overlay
> >>>> inode's, so cachestat() reports all zeroes for them.
> >>>>
> >>>> Add a ->cachestat() file operation and route the syscall through a new
> >>>> vfs_cachestat() helper that calls it when present, falling back to
> >>>> file's f_mapping otherwise. This lets stackable filesystems forward the
> >>>> query to the file that actually owns the page cache. No behaviour change
> >>>> for regular files.
> >>>>
> >>>> Signed-off-by: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>
> >>>> ---
> >>>> Note: Memset change might be a bit tricky, I moved it to no
> >>>> ->cachestat() path to avoid multiple memset on nested overlayfs, that
> >>>> means that ->cachestat() is expected to be able to handle unitialized
> >>>> cs.
> >>>> ---
> >>>> include/linux/fs.h | 10 ++++++++++
> >>>> mm/filemap.c | 43 +++++++++++++++++++++++++++++++++++--------
> >>>> 2 files changed, 45 insertions(+), 8 deletions(-)
> >>>>
> >>>> diff --git a/include/linux/fs.h b/include/linux/fs.h
> >>>> index 6da44573ce450..966b6564707e4 100644
> >>>> --- a/include/linux/fs.h
> >>>> +++ b/include/linux/fs.h
> >>>> @@ -53,6 +53,8 @@
> >>>>
> >>>> struct bdi_writeback;
> >>>> struct bio;
> >>>> +struct cachestat_range;
> >>>> +struct cachestat;
> >>>> struct io_comp_batch;
> >>>> struct fiemap_extent_info;
> >>>> struct kiocb;
> >>>> @@ -1963,6 +1965,8 @@ struct file_operations {
> >>>> struct file *file_out, loff_t pos_out,
> >>>> loff_t len, unsigned int remap_flags);
> >>>> int (*fadvise)(struct file *, loff_t, loff_t, int);
> >>>> + int (*cachestat)(struct file *file, struct cachestat_range *csr,
> >>>> + struct cachestat *cs);
> >>>
> >>> I suppose you can't just have it return the real file because of the
> >>> with_ovl_creds() scope you need during access?
> >>
> >> Yes, AFAIU in overlay when we use realfile we should always use it
> >> with_ovl_creds(), even though I don't think there is anything cred related
> >> in filemap_cachestat(), I still think we should follow the common pattern
> >> other overlay helpers use (similar to ovl_fadvise() and ovl_flush()).
> >>
> >> note: Actually some places get ovl_real_file() and use it without
> >> with_ovl_creds(), e.g.: ovl_read_iter, ovl_write_iter, ovl_splice_read,
> >> ovl_splice_write. But those look more of an exception than the general
> >> rule. All other instances use with_ovl_creds().
> >
> > Use with_ovl_creds() is a good practice to keep the mental security model,
> > but it is useless if the security check (can_do_cachestat) is not in the
> > vfs helper (vfs_cachestat), so please move it there.
>
> Totally, I feel stupid now that I missed it originally, will move it in v2.
>
> >
> > Also it kind of makes more sense to check (flags != 0) in sys_cachestats
> > before checking permissions.
> >
> >>
> >> Also there are simingly no other file_operations which return "realfile"
> >> for further processing, mostly the operation from fsops simply replaces
> >> general operation with its own logic completely.
> >>
> >> Thanks for your review!
> >>
> >> ps: Hope overlay maintainers will correctly if I'm getting this wrong.
> >>
> >
> > I don't think this is wrong per-se, except for can_do_cachestat().
> >
> > Just be aware that the real file could change from one cachestat
> > call to the next (i.e. due to copy up).
>
> I guess the copy-up between two cachestat calls is an equivalent of cache pages
> being dropped/reclaimed between the calls, right?
I guess so.
> (I assume old pages will not be used anymore and will be reclaimed in due time.)
TBH as long as this overlayfs fd is kept open both real lower
and real upper (post copy up) files are kept open, but the lower
pages will not be accessed by overlayfs
expect for the case ofMAP_SHARED mmap
if this overlayfs file was mmaped before copy up, the
lower inode pages will remain mapped to process address space
and then cachestat will return stats which are disassociated from the
pages accessed by mmap.
This is documented in the `Non-standard behavior` section of overlayfs.rst
and not a specific issue to cachestat. Just wanted to be aware of this.
> So userspace should not be
> overly confused by such a cache usage change.
>
Agree.
Thanks,
Amir.
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 1/3] fs, mm: add ->cachestat() file operation
2026-06-23 15:34 ` Amir Goldstein
2026-06-24 11:59 ` Pavel Tikhomirov
@ 2026-06-25 10:36 ` Christian Brauner
2026-06-26 15:18 ` Amir Goldstein
1 sibling, 1 reply; 25+ messages in thread
From: Christian Brauner @ 2026-06-25 10:36 UTC (permalink / raw)
To: Amir Goldstein
Cc: Pavel Tikhomirov, Johannes Weiner, Miklos Szeredi,
Alexander Viro, Christian Brauner, Jan Kara,
Matthew Wilcox (Oracle),
Andrew Morton, Nhat Pham, Shuah Khan, linux-unionfs,
linux-kernel, linux-fsdevel, linux-mm, linux-kselftest
On 2026-06-23 17:34:47+02:00, Amir Goldstein wrote:
> On Tue, Jun 23, 2026 at 4:55 PM Pavel Tikhomirov
> <ptikhomirov@virtuozzo.com> wrote:
>
> > On 6/23/26 15:48, Johannes Weiner wrote:
> >
> > Yes, AFAIU in overlay when we use realfile we should always use it
> > with_ovl_creds(), even though I don't think there is anything cred related
> > in filemap_cachestat(), I still think we should follow the common pattern
> > other overlay helpers use (similar to ovl_fadvise() and ovl_flush()).
> >
> > note: Actually some places get ovl_real_file() and use it without
> > with_ovl_creds(), e.g.: ovl_read_iter, ovl_write_iter, ovl_splice_read,
> > ovl_splice_write. But those look more of an exception than the general
> > rule. All other instances use with_ovl_creds().
>
> Use with_ovl_creds() is a good practice to keep the mental security model,
> but it is useless if the security check (can_do_cachestat) is not in the
> vfs helper (vfs_cachestat), so please move it there.
>
> Also it kind of makes more sense to check (flags != 0) in sys_cachestats
> before checking permissions.
>
> > Also there are simingly no other file_operations which return "realfile"
> > for further processing, mostly the operation from fsops simply replaces
> > general operation with its own logic completely.
> >
> > Thanks for your review!
> >
> > ps: Hope overlay maintainers will correctly if I'm getting this wrong.
>
> I don't think this is wrong per-se, except for can_do_cachestat().
>
> Just be aware that the real file could change from one cachestat
> call to the next (i.e. due to copy up).
I'm really grump about adding a new file operation just for a
special-sauce system call which is under a CONFIG_* option even. We're
not going to set the precedent of piling on custom file operations for a
single filesystem everytime someone adds a new system call unless
absolutely necessary. This looks like it could just use a new helper in
fs/backing_file.c that the cachestat thing can call to use the correct
file.
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 1/3] fs, mm: add ->cachestat() file operation
2026-06-25 10:36 ` Christian Brauner
@ 2026-06-26 15:18 ` Amir Goldstein
2026-06-30 7:55 ` Christian Brauner
0 siblings, 1 reply; 25+ messages in thread
From: Amir Goldstein @ 2026-06-26 15:18 UTC (permalink / raw)
To: Christian Brauner
Cc: Pavel Tikhomirov, Johannes Weiner, Miklos Szeredi,
Alexander Viro, Jan Kara, Matthew Wilcox (Oracle),
Andrew Morton, Nhat Pham, Shuah Khan, linux-unionfs,
linux-kernel, linux-fsdevel, linux-mm, linux-kselftest
On Thu, Jun 25, 2026 at 12:36 PM Christian Brauner <brauner@kernel.org> wrote:
>
> On 2026-06-23 17:34:47+02:00, Amir Goldstein wrote:
> > On Tue, Jun 23, 2026 at 4:55 PM Pavel Tikhomirov
> > <ptikhomirov@virtuozzo.com> wrote:
> >
> > > On 6/23/26 15:48, Johannes Weiner wrote:
> > >
> > > Yes, AFAIU in overlay when we use realfile we should always use it
> > > with_ovl_creds(), even though I don't think there is anything cred related
> > > in filemap_cachestat(), I still think we should follow the common pattern
> > > other overlay helpers use (similar to ovl_fadvise() and ovl_flush()).
> > >
> > > note: Actually some places get ovl_real_file() and use it without
> > > with_ovl_creds(), e.g.: ovl_read_iter, ovl_write_iter, ovl_splice_read,
> > > ovl_splice_write. But those look more of an exception than the general
> > > rule. All other instances use with_ovl_creds().
> >
> > Use with_ovl_creds() is a good practice to keep the mental security model,
> > but it is useless if the security check (can_do_cachestat) is not in the
> > vfs helper (vfs_cachestat), so please move it there.
> >
> > Also it kind of makes more sense to check (flags != 0) in sys_cachestats
> > before checking permissions.
> >
> > > Also there are simingly no other file_operations which return "realfile"
> > > for further processing, mostly the operation from fsops simply replaces
> > > general operation with its own logic completely.
> > >
> > > Thanks for your review!
> > >
> > > ps: Hope overlay maintainers will correctly if I'm getting this wrong.
> >
> > I don't think this is wrong per-se, except for can_do_cachestat().
> >
> > Just be aware that the real file could change from one cachestat
> > call to the next (i.e. due to copy up).
>
> I'm really grump about adding a new file operation just for a
> special-sauce system call which is under a CONFIG_* option even. We're
> not going to set the precedent of piling on custom file operations for a
> single filesystem everytime someone adds a new system call unless
> absolutely necessary.
I had a similar reaction.
> This looks like it could just use a new helper in
> fs/backing_file.c that the cachestat thing can call to use the correct
> file.
>
I was considering suggesting this as well.
Having f_real_file() complement
but technically, can_do_cachestat() should be checked against
the overlayfs file/inode AND also against the real file/inode with
ovl_creds.
I'd love to be able to provide a backing_file "template" for
operations, but I don't have a good idea how to do that.
Do you?
Thanks,
Amir.
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 1/3] fs, mm: add ->cachestat() file operation
2026-06-26 15:18 ` Amir Goldstein
@ 2026-06-30 7:55 ` Christian Brauner
2026-06-30 9:45 ` Pavel Tikhomirov
2026-06-30 10:56 ` Amir Goldstein
0 siblings, 2 replies; 25+ messages in thread
From: Christian Brauner @ 2026-06-30 7:55 UTC (permalink / raw)
To: Amir Goldstein
Cc: Christian Brauner, Pavel Tikhomirov, Johannes Weiner,
Miklos Szeredi, Alexander Viro, Jan Kara, Matthew Wilcox (Oracle),
Andrew Morton, Nhat Pham, Shuah Khan, linux-unionfs,
linux-kernel, linux-fsdevel, linux-mm, linux-kselftest
On 2026-06-26 17:18 +0200, Amir Goldstein wrote:
> On Thu, Jun 25, 2026 at 12:36 PM Christian Brauner <brauner@kernel.org> wrote:
> >
> > On 2026-06-23 17:34:47+02:00, Amir Goldstein wrote:
> > > On Tue, Jun 23, 2026 at 4:55 PM Pavel Tikhomirov
> > > <ptikhomirov@virtuozzo.com> wrote:
> > >
> > > > On 6/23/26 15:48, Johannes Weiner wrote:
> > > >
> > > > Yes, AFAIU in overlay when we use realfile we should always use it
> > > > with_ovl_creds(), even though I don't think there is anything cred related
> > > > in filemap_cachestat(), I still think we should follow the common pattern
> > > > other overlay helpers use (similar to ovl_fadvise() and ovl_flush()).
> > > >
> > > > note: Actually some places get ovl_real_file() and use it without
> > > > with_ovl_creds(), e.g.: ovl_read_iter, ovl_write_iter, ovl_splice_read,
> > > > ovl_splice_write. But those look more of an exception than the general
> > > > rule. All other instances use with_ovl_creds().
> > >
> > > Use with_ovl_creds() is a good practice to keep the mental security model,
> > > but it is useless if the security check (can_do_cachestat) is not in the
> > > vfs helper (vfs_cachestat), so please move it there.
> > >
> > > Also it kind of makes more sense to check (flags != 0) in sys_cachestats
> > > before checking permissions.
> > >
> > > > Also there are simingly no other file_operations which return "realfile"
> > > > for further processing, mostly the operation from fsops simply replaces
> > > > general operation with its own logic completely.
> > > >
> > > > Thanks for your review!
> > > >
> > > > ps: Hope overlay maintainers will correctly if I'm getting this wrong.
> > >
> > > I don't think this is wrong per-se, except for can_do_cachestat().
> > >
> > > Just be aware that the real file could change from one cachestat
> > > call to the next (i.e. due to copy up).
> >
> > I'm really grump about adding a new file operation just for a
> > special-sauce system call which is under a CONFIG_* option even. We're
> > not going to set the precedent of piling on custom file operations for a
> > single filesystem everytime someone adds a new system call unless
> > absolutely necessary.
>
> I had a similar reaction.
>
> > This looks like it could just use a new helper in
> > fs/backing_file.c that the cachestat thing can call to use the correct
> > file.
> >
>
> I was considering suggesting this as well.
> Having f_real_file() complement
>
> but technically, can_do_cachestat() should be checked against
> the overlayfs file/inode AND also against the real file/inode with
> ovl_creds.
>
> I'd love to be able to provide a backing_file "template" for
> operations, but I don't have a good idea how to do that.
> Do you?
diff --git a/fs/file_table.c b/fs/file_table.c
index c68b8c0a4097..d93059d53e88 100644
--- a/fs/file_table.c
+++ b/fs/file_table.c
@@ -57,6 +57,8 @@ struct backing_file {
#ifdef CONFIG_SECURITY
void *security;
#endif
+ unsigned int flags;
+ struct cred *backing_cred;
};
diff --git a/fs/overlayfs/file.c b/fs/overlayfs/file.c
index 27cc07738f33..64e07ebc43b0 100644
--- a/fs/overlayfs/file.c
+++ b/fs/overlayfs/file.c
@@ -50,6 +50,12 @@ static struct file *ovl_open_realfile(const struct file *file,
realfile = backing_file_open(file,
flags, realpath, current_cred());
+
+ // Raises ->flags |= BACKING_FILE_CREDS and sets ->creds to ->backing_cred.
+ backing_file_set_creds(file, ovl_creds(file_inode(file)->i_sb));
+ // Now add a helper that performs permission checks on
+ // both creds for that cachestat() thing and returns
+ // the actual file to operate on.
}
}
something like that?
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH 1/3] fs, mm: add ->cachestat() file operation
2026-06-30 7:55 ` Christian Brauner
@ 2026-06-30 9:45 ` Pavel Tikhomirov
2026-06-30 10:56 ` Amir Goldstein
1 sibling, 0 replies; 25+ messages in thread
From: Pavel Tikhomirov @ 2026-06-30 9:45 UTC (permalink / raw)
To: Christian Brauner, Amir Goldstein
Cc: Johannes Weiner, Miklos Szeredi, Alexander Viro, Jan Kara,
Matthew Wilcox (Oracle),
Andrew Morton, Nhat Pham, Shuah Khan, linux-unionfs,
linux-kernel, linux-fsdevel, linux-mm, linux-kselftest
On 6/30/26 09:55, Christian Brauner wrote:
> On 2026-06-26 17:18 +0200, Amir Goldstein wrote:
>> On Thu, Jun 25, 2026 at 12:36 PM Christian Brauner <brauner@kernel.org> wrote:
>>>
>>> On 2026-06-23 17:34:47+02:00, Amir Goldstein wrote:
>>>> On Tue, Jun 23, 2026 at 4:55 PM Pavel Tikhomirov
>>>> <ptikhomirov@virtuozzo.com> wrote:
>>>>
>>>>> On 6/23/26 15:48, Johannes Weiner wrote:
>>>>>
>>>>> Yes, AFAIU in overlay when we use realfile we should always use it
>>>>> with_ovl_creds(), even though I don't think there is anything cred related
>>>>> in filemap_cachestat(), I still think we should follow the common pattern
>>>>> other overlay helpers use (similar to ovl_fadvise() and ovl_flush()).
>>>>>
>>>>> note: Actually some places get ovl_real_file() and use it without
>>>>> with_ovl_creds(), e.g.: ovl_read_iter, ovl_write_iter, ovl_splice_read,
>>>>> ovl_splice_write. But those look more of an exception than the general
>>>>> rule. All other instances use with_ovl_creds().
>>>>
>>>> Use with_ovl_creds() is a good practice to keep the mental security model,
>>>> but it is useless if the security check (can_do_cachestat) is not in the
>>>> vfs helper (vfs_cachestat), so please move it there.
>>>>
>>>> Also it kind of makes more sense to check (flags != 0) in sys_cachestats
>>>> before checking permissions.
>>>>
>>>>> Also there are simingly no other file_operations which return "realfile"
>>>>> for further processing, mostly the operation from fsops simply replaces
>>>>> general operation with its own logic completely.
>>>>>
>>>>> Thanks for your review!
>>>>>
>>>>> ps: Hope overlay maintainers will correctly if I'm getting this wrong.
>>>>
>>>> I don't think this is wrong per-se, except for can_do_cachestat().
>>>>
>>>> Just be aware that the real file could change from one cachestat
>>>> call to the next (i.e. due to copy up).
>>>
>>> I'm really grump about adding a new file operation just for a
>>> special-sauce system call which is under a CONFIG_* option even. We're
>>> not going to set the precedent of piling on custom file operations for a
>>> single filesystem everytime someone adds a new system call unless
>>> absolutely necessary.
>>
>> I had a similar reaction.
>>
>>> This looks like it could just use a new helper in
>>> fs/backing_file.c that the cachestat thing can call to use the correct
>>> file.
>>>
>>
>> I was considering suggesting this as well.
>> Having f_real_file() complement
>>
>> but technically, can_do_cachestat() should be checked against
>> the overlayfs file/inode AND also against the real file/inode with
>> ovl_creds.
>>
>> I'd love to be able to provide a backing_file "template" for
>> operations, but I don't have a good idea how to do that.
>> Do you?
>
> diff --git a/fs/file_table.c b/fs/file_table.c
> index c68b8c0a4097..d93059d53e88 100644
> --- a/fs/file_table.c
> +++ b/fs/file_table.c
> @@ -57,6 +57,8 @@ struct backing_file {
> #ifdef CONFIG_SECURITY
> void *security;
> #endif
> + unsigned int flags;
> + struct cred *backing_cred;
> };
>
> diff --git a/fs/overlayfs/file.c b/fs/overlayfs/file.c
> index 27cc07738f33..64e07ebc43b0 100644
> --- a/fs/overlayfs/file.c
> +++ b/fs/overlayfs/file.c
> @@ -50,6 +50,12 @@ static struct file *ovl_open_realfile(const struct file *file,
>
> realfile = backing_file_open(file,
> flags, realpath, current_cred());
> +
> + // Raises ->flags |= BACKING_FILE_CREDS and sets ->creds to ->backing_cred.
> + backing_file_set_creds(file, ovl_creds(file_inode(file)->i_sb));
> + // Now add a helper that performs permission checks on
> + // both creds for that cachestat() thing and returns
> + // the actual file to operate on.
I'm not sure I fully understand it, sorry.
The idea seems to be to replace file operation customization helper ->cachestat() with some way for cachestat syscall to access a backing file directly. At the same time providing the credentials from overlay with which to check permissions on this backing file.
I think the inherent recursiveness of overlay (e.g. one can mount overlayfs where lower fs is again an overlay) is effectively making us check permissions recursively for each level of backing file, so it would be hard to have just one backing file. But maybe I miss something.
> }
> }
>
> something like that?
>
--
Best regards, Pavel Tikhomirov
Senior Software Developer, Virtuozzo.
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH 1/3] fs, mm: add ->cachestat() file operation
2026-06-30 7:55 ` Christian Brauner
2026-06-30 9:45 ` Pavel Tikhomirov
@ 2026-06-30 10:56 ` Amir Goldstein
1 sibling, 0 replies; 25+ messages in thread
From: Amir Goldstein @ 2026-06-30 10:56 UTC (permalink / raw)
To: Christian Brauner
Cc: Pavel Tikhomirov, Johannes Weiner, Miklos Szeredi,
Alexander Viro, Jan Kara, Matthew Wilcox (Oracle),
Andrew Morton, Nhat Pham, Shuah Khan, linux-unionfs,
linux-kernel, linux-fsdevel, linux-mm, linux-kselftest
On Tue, Jun 30, 2026 at 9:55 AM Christian Brauner <brauner@kernel.org> wrote:
>
> On 2026-06-26 17:18 +0200, Amir Goldstein wrote:
> > On Thu, Jun 25, 2026 at 12:36 PM Christian Brauner <brauner@kernel.org> wrote:
> > >
> > > On 2026-06-23 17:34:47+02:00, Amir Goldstein wrote:
> > > > On Tue, Jun 23, 2026 at 4:55 PM Pavel Tikhomirov
> > > > <ptikhomirov@virtuozzo.com> wrote:
> > > >
> > > > > On 6/23/26 15:48, Johannes Weiner wrote:
> > > > >
> > > > > Yes, AFAIU in overlay when we use realfile we should always use it
> > > > > with_ovl_creds(), even though I don't think there is anything cred related
> > > > > in filemap_cachestat(), I still think we should follow the common pattern
> > > > > other overlay helpers use (similar to ovl_fadvise() and ovl_flush()).
> > > > >
> > > > > note: Actually some places get ovl_real_file() and use it without
> > > > > with_ovl_creds(), e.g.: ovl_read_iter, ovl_write_iter, ovl_splice_read,
> > > > > ovl_splice_write. But those look more of an exception than the general
> > > > > rule. All other instances use with_ovl_creds().
> > > >
> > > > Use with_ovl_creds() is a good practice to keep the mental security model,
> > > > but it is useless if the security check (can_do_cachestat) is not in the
> > > > vfs helper (vfs_cachestat), so please move it there.
> > > >
> > > > Also it kind of makes more sense to check (flags != 0) in sys_cachestats
> > > > before checking permissions.
> > > >
> > > > > Also there are simingly no other file_operations which return "realfile"
> > > > > for further processing, mostly the operation from fsops simply replaces
> > > > > general operation with its own logic completely.
> > > > >
> > > > > Thanks for your review!
> > > > >
> > > > > ps: Hope overlay maintainers will correctly if I'm getting this wrong.
> > > >
> > > > I don't think this is wrong per-se, except for can_do_cachestat().
> > > >
> > > > Just be aware that the real file could change from one cachestat
> > > > call to the next (i.e. due to copy up).
> > >
> > > I'm really grump about adding a new file operation just for a
> > > special-sauce system call which is under a CONFIG_* option even. We're
> > > not going to set the precedent of piling on custom file operations for a
> > > single filesystem everytime someone adds a new system call unless
> > > absolutely necessary.
> >
> > I had a similar reaction.
> >
> > > This looks like it could just use a new helper in
> > > fs/backing_file.c that the cachestat thing can call to use the correct
> > > file.
> > >
> >
> > I was considering suggesting this as well.
> > Having f_real_file() complement
> >
> > but technically, can_do_cachestat() should be checked against
> > the overlayfs file/inode AND also against the real file/inode with
> > ovl_creds.
> >
> > I'd love to be able to provide a backing_file "template" for
> > operations, but I don't have a good idea how to do that.
> > Do you?
>
> diff --git a/fs/file_table.c b/fs/file_table.c
> index c68b8c0a4097..d93059d53e88 100644
> --- a/fs/file_table.c
> +++ b/fs/file_table.c
> @@ -57,6 +57,8 @@ struct backing_file {
> #ifdef CONFIG_SECURITY
> void *security;
> #endif
> + unsigned int flags;
> + struct cred *backing_cred;
> };
>
> diff --git a/fs/overlayfs/file.c b/fs/overlayfs/file.c
> index 27cc07738f33..64e07ebc43b0 100644
> --- a/fs/overlayfs/file.c
> +++ b/fs/overlayfs/file.c
> @@ -50,6 +50,12 @@ static struct file *ovl_open_realfile(const struct file *file,
>
> realfile = backing_file_open(file,
> flags, realpath, current_cred());
> +
> + // Raises ->flags |= BACKING_FILE_CREDS and sets ->creds to ->backing_cred.
> + backing_file_set_creds(file, ovl_creds(file_inode(file)->i_sb));
> + // Now add a helper that performs permission checks on
> + // both creds for that cachestat() thing and returns
> + // the actual file to operate on.
> }
> }
>
> something like that?
>
Confused.
It seems strange to have backing_cred in this scope.
I would expect having user_cred same as user_path.
backing_file gets you from the real file to properties of the "user"
facing file.
The cachestat() syscall has a reference to the user facing file not to
the real file.
The helper would need something like f_real_file() to get to real file
from user facing file.
Anyway I am not seeing the big picture clearly how this could turn to
be nice code.
Maybe lack of imagination on my part.
Thanks,
Amir.
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 1/3] fs, mm: add ->cachestat() file operation
2026-06-23 11:14 ` [PATCH 1/3] fs, mm: add ->cachestat() file operation Pavel Tikhomirov
2026-06-23 13:48 ` Johannes Weiner
@ 2026-06-23 16:01 ` Nhat Pham
2026-06-24 11:46 ` Pavel Tikhomirov
1 sibling, 1 reply; 25+ messages in thread
From: Nhat Pham @ 2026-06-23 16:01 UTC (permalink / raw)
To: Pavel Tikhomirov
Cc: Miklos Szeredi, Amir Goldstein, Alexander Viro,
Christian Brauner, Jan Kara, Matthew Wilcox (Oracle),
Andrew Morton, Johannes Weiner, Shuah Khan, linux-unionfs,
linux-kernel, linux-fsdevel, linux-mm, linux-kselftest
On Tue, Jun 23, 2026 at 4:15 AM Pavel Tikhomirov
<ptikhomirov@virtuozzo.com> wrote:
>
> The cachestat() syscall reads page cache statistics straight from the
> file's f_mapping. Stackable filesystems such as overlayfs keep the data
> pages in an underlying inode's mapping rather than in the overlay
> inode's, so cachestat() reports all zeroes for them.
>
> Add a ->cachestat() file operation and route the syscall through a new
> vfs_cachestat() helper that calls it when present, falling back to
> file's f_mapping otherwise. This lets stackable filesystems forward the
> query to the file that actually owns the page cache. No behaviour change
> for regular files.
>
> Signed-off-by: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>
> ---
> Note: Memset change might be a bit tricky, I moved it to no
> ->cachestat() path to avoid multiple memset on nested overlayfs, that
> means that ->cachestat() is expected to be able to handle unitialized
> cs.
nit: maybe a comment somewhere to document this?
> ---
> include/linux/fs.h | 10 ++++++++++
> mm/filemap.c | 43 +++++++++++++++++++++++++++++++++++--------
> 2 files changed, 45 insertions(+), 8 deletions(-)
>
LGTM otherwise. With Amir's suggestions incorporated:
Acked-by: Nhat Pham <nphamcs@gmail.com>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 1/3] fs, mm: add ->cachestat() file operation
2026-06-23 16:01 ` Nhat Pham
@ 2026-06-24 11:46 ` Pavel Tikhomirov
0 siblings, 0 replies; 25+ messages in thread
From: Pavel Tikhomirov @ 2026-06-24 11:46 UTC (permalink / raw)
To: Nhat Pham
Cc: Miklos Szeredi, Amir Goldstein, Alexander Viro,
Christian Brauner, Jan Kara, Matthew Wilcox (Oracle),
Andrew Morton, Johannes Weiner, Shuah Khan, linux-unionfs,
linux-kernel, linux-fsdevel, linux-mm, linux-kselftest
On 6/23/26 18:01, Nhat Pham wrote:
> On Tue, Jun 23, 2026 at 4:15 AM Pavel Tikhomirov
> <ptikhomirov@virtuozzo.com> wrote:
>>
>> The cachestat() syscall reads page cache statistics straight from the
>> file's f_mapping. Stackable filesystems such as overlayfs keep the data
>> pages in an underlying inode's mapping rather than in the overlay
>> inode's, so cachestat() reports all zeroes for them.
>>
>> Add a ->cachestat() file operation and route the syscall through a new
>> vfs_cachestat() helper that calls it when present, falling back to
>> file's f_mapping otherwise. This lets stackable filesystems forward the
>> query to the file that actually owns the page cache. No behaviour change
>> for regular files.
>>
>> Signed-off-by: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>
>> ---
>> Note: Memset change might be a bit tricky, I moved it to no
>> ->cachestat() path to avoid multiple memset on nested overlayfs, that
>> means that ->cachestat() is expected to be able to handle unitialized
>> cs.
>
> nit: maybe a comment somewhere to document this?
Sure, will do in v2.
>
>> ---
>> include/linux/fs.h | 10 ++++++++++
>> mm/filemap.c | 43 +++++++++++++++++++++++++++++++++++--------
>> 2 files changed, 45 insertions(+), 8 deletions(-)
>>
>
> LGTM otherwise. With Amir's suggestions incorporated:
>
> Acked-by: Nhat Pham <nphamcs@gmail.com>
--
Best regards, Pavel Tikhomirov
Senior Software Developer, Virtuozzo.
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 2/3] ovl: support cachestat() syscall on overlayfs files
2026-06-23 11:14 [PATCH 0/3] ovl: make cachestat() work Pavel Tikhomirov
2026-06-23 11:14 ` [PATCH 1/3] fs, mm: add ->cachestat() file operation Pavel Tikhomirov
@ 2026-06-23 11:14 ` Pavel Tikhomirov
2026-06-23 17:12 ` Nhat Pham
2026-06-23 11:14 ` [PATCH 3/3] selftests/cachestat: add an overlayfs test case Pavel Tikhomirov
2026-06-24 20:50 ` [PATCH 0/3] ovl: make cachestat() work Andrew Morton
3 siblings, 1 reply; 25+ messages in thread
From: Pavel Tikhomirov @ 2026-06-23 11:14 UTC (permalink / raw)
To: Miklos Szeredi, Amir Goldstein
Cc: Alexander Viro, Christian Brauner, Jan Kara,
Matthew Wilcox (Oracle),
Andrew Morton, Nhat Pham, Johannes Weiner, Shuah Khan,
linux-unionfs, linux-kernel, linux-fsdevel, linux-mm,
linux-kselftest, Pavel Tikhomirov
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 <ptikhomirov@virtuozzo.com>
---
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);
+
+ with_ovl_creds(file_inode(file)->i_sb)
+ return vfs_cachestat(realfile, csr, cs);
+}
+#endif
+
enum ovl_copyop {
OVL_COPY,
OVL_CLONE,
@@ -642,6 +657,9 @@ const struct file_operations ovl_file_operations = {
.mmap = ovl_mmap,
.fallocate = ovl_fallocate,
.fadvise = ovl_fadvise,
+#ifdef CONFIG_CACHESTAT_SYSCALL
+ .cachestat = ovl_cachestat,
+#endif
.flush = ovl_flush,
.splice_read = ovl_splice_read,
.splice_write = ovl_splice_write,
--
2.54.0
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH 2/3] ovl: support cachestat() syscall on overlayfs files
2026-06-23 11:14 ` [PATCH 2/3] ovl: support cachestat() syscall on overlayfs files Pavel Tikhomirov
@ 2026-06-23 17:12 ` Nhat Pham
2026-06-24 11:45 ` Pavel Tikhomirov
0 siblings, 1 reply; 25+ messages in thread
From: Nhat Pham @ 2026-06-23 17:12 UTC (permalink / raw)
To: Pavel Tikhomirov
Cc: Miklos Szeredi, Amir Goldstein, Alexander Viro,
Christian Brauner, Jan Kara, Matthew Wilcox (Oracle),
Andrew Morton, Johannes Weiner, Shuah Khan, linux-unionfs,
linux-kernel, linux-fsdevel, linux-mm, linux-kselftest
On Tue, Jun 23, 2026 at 4:15 AM Pavel Tikhomirov
<ptikhomirov@virtuozzo.com> 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 <ptikhomirov@virtuozzo.com>
> ---
> 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 :)
> +
> + with_ovl_creds(file_inode(file)->i_sb)
> + return vfs_cachestat(realfile, csr, cs);
> +}
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH 2/3] ovl: support cachestat() syscall on overlayfs files
2026-06-23 17:12 ` Nhat Pham
@ 2026-06-24 11:45 ` Pavel Tikhomirov
2026-06-24 14:16 ` Amir Goldstein
0 siblings, 1 reply; 25+ messages in thread
From: Pavel Tikhomirov @ 2026-06-24 11:45 UTC (permalink / raw)
To: Nhat Pham
Cc: Miklos Szeredi, Amir Goldstein, Alexander Viro,
Christian Brauner, Jan Kara, Matthew Wilcox (Oracle),
Andrew Morton, Johannes Weiner, Shuah Khan, linux-unionfs,
linux-kernel, linux-fsdevel, linux-mm, linux-kselftest
On 6/23/26 19:12, Nhat Pham wrote:
> On Tue, Jun 23, 2026 at 4:15 AM Pavel Tikhomirov
> <ptikhomirov@virtuozzo.com> 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 <ptikhomirov@virtuozzo.com>
>> ---
>> 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.
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH 2/3] ovl: support cachestat() syscall on overlayfs files
2026-06-24 11:45 ` Pavel Tikhomirov
@ 2026-06-24 14:16 ` Amir Goldstein
2026-06-24 19:06 ` Nhat Pham
0 siblings, 1 reply; 25+ messages in thread
From: Amir Goldstein @ 2026-06-24 14:16 UTC (permalink / raw)
To: Pavel Tikhomirov
Cc: Nhat Pham, Miklos Szeredi, Alexander Viro, Christian Brauner,
Jan Kara, Matthew Wilcox (Oracle),
Andrew Morton, Johannes Weiner, Shuah Khan, linux-unionfs,
linux-kernel, linux-fsdevel, linux-mm, linux-kselftest
On Wed, Jun 24, 2026 at 1:45 PM Pavel Tikhomirov
<ptikhomirov@virtuozzo.com> wrote:
>
>
>
> On 6/23/26 19:12, Nhat Pham wrote:
> > On Tue, Jun 23, 2026 at 4:15 AM Pavel Tikhomirov
> > <ptikhomirov@virtuozzo.com> 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 <ptikhomirov@virtuozzo.com>
> >> ---
> >> 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'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.
Thanks,
Amir.
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH 2/3] ovl: support cachestat() syscall on overlayfs files
2026-06-24 14:16 ` Amir Goldstein
@ 2026-06-24 19:06 ` Nhat Pham
2026-06-25 12:53 ` Matthew Wilcox
0 siblings, 1 reply; 25+ messages in thread
From: Nhat Pham @ 2026-06-24 19:06 UTC (permalink / raw)
To: Amir Goldstein
Cc: Pavel Tikhomirov, Miklos Szeredi, Alexander Viro,
Christian Brauner, Jan Kara, Matthew Wilcox (Oracle),
Andrew Morton, Johannes Weiner, Shuah Khan, linux-unionfs,
linux-kernel, linux-fsdevel, linux-mm, linux-kselftest
On Wed, Jun 24, 2026 at 7:16 AM Amir Goldstein <amir73il@gmail.com> wrote:
>
> On Wed, Jun 24, 2026 at 1:45 PM Pavel Tikhomirov
> <ptikhomirov@virtuozzo.com> wrote:
> >
> >
> >
> > On 6/23/26 19:12, Nhat Pham wrote:
> > > On Tue, Jun 23, 2026 at 4:15 AM Pavel Tikhomirov
> > > <ptikhomirov@virtuozzo.com> 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 <ptikhomirov@virtuozzo.com>
> > >> ---
> > >> 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.
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH 2/3] ovl: support cachestat() syscall on overlayfs files
2026-06-24 19:06 ` Nhat Pham
@ 2026-06-25 12:53 ` Matthew Wilcox
2026-06-25 16:12 ` Nhat Pham
0 siblings, 1 reply; 25+ messages in thread
From: Matthew Wilcox @ 2026-06-25 12:53 UTC (permalink / raw)
To: Nhat Pham
Cc: Amir Goldstein, Pavel Tikhomirov, Miklos Szeredi, Alexander Viro,
Christian Brauner, Jan Kara, Andrew Morton, Johannes Weiner,
Shuah Khan, linux-unionfs, linux-kernel, linux-fsdevel, linux-mm,
linux-kselftest
On Wed, Jun 24, 2026 at 12:06:54PM -0700, Nhat Pham wrote:
> 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 :)
That's ALWAYS true. POSIX even says so explicitly in section 2.3:
Implementations may support additional errors not included in this list,
may generate errors included in this list under circumstances other
than those described here, or may contain extensions or limitations
that prevent some errors from occurring.
We don't generally bother to document that pretty much every syscall may
return -ENOMEM if it can't allocate memory. That's just ... expected.
open(2) documents the possibility, but read(2) doesn't. I think it's
the same for EIO. Any operation which accesses storage can return -EIO.
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 2/3] ovl: support cachestat() syscall on overlayfs files
2026-06-25 12:53 ` Matthew Wilcox
@ 2026-06-25 16:12 ` Nhat Pham
0 siblings, 0 replies; 25+ messages in thread
From: Nhat Pham @ 2026-06-25 16:12 UTC (permalink / raw)
To: Matthew Wilcox
Cc: Amir Goldstein, Pavel Tikhomirov, Miklos Szeredi, Alexander Viro,
Christian Brauner, Jan Kara, Andrew Morton, Johannes Weiner,
Shuah Khan, linux-unionfs, linux-kernel, linux-fsdevel, linux-mm,
linux-kselftest
On Thu, Jun 25, 2026 at 5:53 AM Matthew Wilcox <willy@infradead.org> wrote:
>
> On Wed, Jun 24, 2026 at 12:06:54PM -0700, Nhat Pham wrote:
> > 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 :)
>
> That's ALWAYS true. POSIX even says so explicitly in section 2.3:
>
> Implementations may support additional errors not included in this list,
> may generate errors included in this list under circumstances other
> than those described here, or may contain extensions or limitations
> that prevent some errors from occurring.
>
> We don't generally bother to document that pretty much every syscall may
> return -ENOMEM if it can't allocate memory. That's just ... expected.
> open(2) documents the possibility, but read(2) doesn't. I think it's
> the same for EIO. Any operation which accesses storage can return -EIO.
Ah I see! Admittedly, I'm not aware with this POSIX detail. Thanks for
the clarifications and pointers, Matthew!
In that case, I have no further comments on this patch, and defer to
overlayfs and fs folks for FS-specific correctness.
Reviewed-by: Nhat Pham <nphamcs@gmail.com>
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 3/3] selftests/cachestat: add an overlayfs test case
2026-06-23 11:14 [PATCH 0/3] ovl: make cachestat() work Pavel Tikhomirov
2026-06-23 11:14 ` [PATCH 1/3] fs, mm: add ->cachestat() file operation Pavel Tikhomirov
2026-06-23 11:14 ` [PATCH 2/3] ovl: support cachestat() syscall on overlayfs files Pavel Tikhomirov
@ 2026-06-23 11:14 ` Pavel Tikhomirov
2026-06-23 16:13 ` Nhat Pham
2026-06-24 20:50 ` [PATCH 0/3] ovl: make cachestat() work Andrew Morton
3 siblings, 1 reply; 25+ messages in thread
From: Pavel Tikhomirov @ 2026-06-23 11:14 UTC (permalink / raw)
To: Miklos Szeredi, Amir Goldstein
Cc: Alexander Viro, Christian Brauner, Jan Kara,
Matthew Wilcox (Oracle),
Andrew Morton, Nhat Pham, Johannes Weiner, Shuah Khan,
linux-unionfs, linux-kernel, linux-fsdevel, linux-mm,
linux-kselftest, Pavel Tikhomirov
Mount an overlayfs, create and write a file in the merged directory, and
run cachestat() on it, reusing the existing test_cachestat() helper.
Also bump NR_TESTS to the actual number of tests run: it was 9 while
ten tests were already executed, and this adds an eleventh.
Signed-off-by: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>
---
.../selftests/cachestat/test_cachestat.c | 75 ++++++++++++++++++-
1 file changed, 74 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/cachestat/test_cachestat.c b/tools/testing/selftests/cachestat/test_cachestat.c
index 542cd09cb4434..1662d9817c50b 100644
--- a/tools/testing/selftests/cachestat/test_cachestat.c
+++ b/tools/testing/selftests/cachestat/test_cachestat.c
@@ -4,21 +4,25 @@
#include <stdio.h>
#include <stdbool.h>
+#include <stdlib.h>
#include <linux/kernel.h>
#include <linux/magic.h>
#include <linux/mman.h>
#include <sys/mman.h>
+#include <sys/mount.h>
#include <sys/shm.h>
+#include <sys/stat.h>
#include <sys/syscall.h>
#include <sys/vfs.h>
#include <unistd.h>
#include <string.h>
#include <fcntl.h>
#include <errno.h>
+#include <limits.h>
#include "kselftest.h"
-#define NR_TESTS 9
+#define NR_TESTS 11
static const char * const dev_files[] = {
"/dev/zero", "/dev/null", "/dev/urandom",
@@ -294,6 +298,62 @@ bool run_cachestat_test(enum file_type type)
return ret;
}
+/*
+ * Set up an overlayfs mount and run cachestat on a freshly created file in the
+ * merged directory. Overlayfs forwards data I/O to the underlying (upper)
+ * inode, so the page cache lives there and not in the overlay inode's mapping.
+ * This is a regression test for cachestat returning all zeroes on overlayfs.
+ */
+static int run_cachestat_overlayfs_test(void)
+{
+ char tmpl[] = "/tmp/cachestat_ovl.XXXXXX";
+ char lower[PATH_MAX], upper[PATH_MAX], work[PATH_MAX];
+ char merged[PATH_MAX], opts[4 * PATH_MAX], file[PATH_MAX];
+ char *base;
+ int ret;
+
+ base = mkdtemp(tmpl);
+ if (!base) {
+ ksft_print_msg("Unable to create overlayfs base dir: %s\n",
+ strerror(errno));
+ return KSFT_FAIL;
+ }
+
+ snprintf(lower, sizeof(lower), "%s/lower", base);
+ snprintf(upper, sizeof(upper), "%s/upper", base);
+ snprintf(work, sizeof(work), "%s/work", base);
+ snprintf(merged, sizeof(merged), "%s/merged", base);
+
+ if (mkdir(lower, 0755) || mkdir(upper, 0755) ||
+ mkdir(work, 0755) || mkdir(merged, 0755)) {
+ ksft_print_msg("Unable to create overlayfs dirs: %s\n",
+ strerror(errno));
+ ret = KSFT_FAIL;
+ goto cleanup;
+ }
+
+ snprintf(opts, sizeof(opts), "lowerdir=%s,upperdir=%s,workdir=%s",
+ lower, upper, work);
+
+ if (mount("overlay", merged, "overlay", 0, opts)) {
+ ksft_print_msg("Unable to mount overlayfs (need root?): %s\n",
+ strerror(errno));
+ ret = KSFT_SKIP;
+ goto cleanup;
+ }
+
+ snprintf(file, sizeof(file), "%s/merged/cachestat", base);
+ ret = test_cachestat(file, true, true, false, 4, O_CREAT | O_RDWR, 0600);
+
+ umount(merged);
+cleanup:
+ /* Best-effort recursive cleanup of the temporary tree. */
+ snprintf(opts, sizeof(opts), "rm -rf %s", base);
+ if (system(opts))
+ ksft_print_msg("Unable to clean up %s\n", base);
+ return ret;
+}
+
int main(void)
{
int ret;
@@ -361,5 +421,18 @@ int main(void)
ksft_test_result_fail("cachestat fails with a mmap file\n");
ret = 1;
}
+
+ switch (run_cachestat_overlayfs_test()) {
+ case KSFT_FAIL:
+ ksft_test_result_fail("cachestat fails with an overlayfs file\n");
+ ret = 1;
+ break;
+ case KSFT_PASS:
+ ksft_test_result_pass("cachestat works with an overlayfs file\n");
+ break;
+ case KSFT_SKIP:
+ ksft_test_result_skip("overlayfs not available\n");
+ break;
+ }
return ret;
}
--
2.54.0
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH 3/3] selftests/cachestat: add an overlayfs test case
2026-06-23 11:14 ` [PATCH 3/3] selftests/cachestat: add an overlayfs test case Pavel Tikhomirov
@ 2026-06-23 16:13 ` Nhat Pham
2026-06-24 12:22 ` Pavel Tikhomirov
0 siblings, 1 reply; 25+ messages in thread
From: Nhat Pham @ 2026-06-23 16:13 UTC (permalink / raw)
To: Pavel Tikhomirov
Cc: Miklos Szeredi, Amir Goldstein, Alexander Viro,
Christian Brauner, Jan Kara, Matthew Wilcox (Oracle),
Andrew Morton, Johannes Weiner, Shuah Khan, linux-unionfs,
linux-kernel, linux-fsdevel, linux-mm, linux-kselftest
On Tue, Jun 23, 2026 at 4:15 AM Pavel Tikhomirov
<ptikhomirov@virtuozzo.com> wrote:
>
> Mount an overlayfs, create and write a file in the merged directory, and
> run cachestat() on it, reusing the existing test_cachestat() helper.
>
> Also bump NR_TESTS to the actual number of tests run: it was 9 while
> ten tests were already executed, and this adds an eleventh.
>
> Signed-off-by: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>
> ---
> .../selftests/cachestat/test_cachestat.c | 75 ++++++++++++++++++-
Hmm this looks strange...?
> 1 file changed, 74 insertions(+), 1 deletion(-)
>
> diff --git a/tools/testing/selftests/cachestat/test_cachestat.c b/tools/testing/selftests/cachestat/test_cachestat.c
> index 542cd09cb4434..1662d9817c50b 100644
> --- a/tools/testing/selftests/cachestat/test_cachestat.c
> +++ b/tools/testing/selftests/cachestat/test_cachestat.c
> @@ -4,21 +4,25 @@
>
> #include <stdio.h>
> #include <stdbool.h>
> +#include <stdlib.h>
> #include <linux/kernel.h>
> #include <linux/magic.h>
> #include <linux/mman.h>
> #include <sys/mman.h>
> +#include <sys/mount.h>
> #include <sys/shm.h>
> +#include <sys/stat.h>
> #include <sys/syscall.h>
> #include <sys/vfs.h>
> #include <unistd.h>
> #include <string.h>
> #include <fcntl.h>
> #include <errno.h>
> +#include <limits.h>
>
> #include "kselftest.h"
>
> -#define NR_TESTS 9
> +#define NR_TESTS 11
>
> static const char * const dev_files[] = {
> "/dev/zero", "/dev/null", "/dev/urandom",
> @@ -294,6 +298,62 @@ bool run_cachestat_test(enum file_type type)
> return ret;
> }
>
> +/*
> + * Set up an overlayfs mount and run cachestat on a freshly created file in the
> + * merged directory. Overlayfs forwards data I/O to the underlying (upper)
> + * inode, so the page cache lives there and not in the overlay inode's mapping.
> + * This is a regression test for cachestat returning all zeroes on overlayfs.
> + */
> +static int run_cachestat_overlayfs_test(void)
> +{
> + char tmpl[] = "/tmp/cachestat_ovl.XXXXXX";
> + char lower[PATH_MAX], upper[PATH_MAX], work[PATH_MAX];
> + char merged[PATH_MAX], opts[4 * PATH_MAX], file[PATH_MAX];
> + char *base;
> + int ret;
> +
> + base = mkdtemp(tmpl);
> + if (!base) {
> + ksft_print_msg("Unable to create overlayfs base dir: %s\n",
> + strerror(errno));
> + return KSFT_FAIL;
> + }
> +
> + snprintf(lower, sizeof(lower), "%s/lower", base);
> + snprintf(upper, sizeof(upper), "%s/upper", base);
> + snprintf(work, sizeof(work), "%s/work", base);
> + snprintf(merged, sizeof(merged), "%s/merged", base);
> +
> + if (mkdir(lower, 0755) || mkdir(upper, 0755) ||
> + mkdir(work, 0755) || mkdir(merged, 0755)) {
> + ksft_print_msg("Unable to create overlayfs dirs: %s\n",
> + strerror(errno));
> + ret = KSFT_FAIL;
> + goto cleanup;
> + }
> +
> + snprintf(opts, sizeof(opts), "lowerdir=%s,upperdir=%s,workdir=%s",
> + lower, upper, work);
> +
> + if (mount("overlay", merged, "overlay", 0, opts)) {
> + ksft_print_msg("Unable to mount overlayfs (need root?): %s\n",
> + strerror(errno));
> + ret = KSFT_SKIP;
> + goto cleanup;
> + }
> +
> + snprintf(file, sizeof(file), "%s/merged/cachestat", base);
> + ret = test_cachestat(file, true, true, false, 4, O_CREAT | O_RDWR, 0600);
> +
> + umount(merged);
> +cleanup:
> + /* Best-effort recursive cleanup of the temporary tree. */
> + snprintf(opts, sizeof(opts), "rm -rf %s", base);
> + if (system(opts))
> + ksft_print_msg("Unable to clean up %s\n", base);
nit: no helper for these? :)
> + return ret;
> +}
> +
> int main(void)
> {
> int ret;
> @@ -361,5 +421,18 @@ int main(void)
> ksft_test_result_fail("cachestat fails with a mmap file\n");
> ret = 1;
> }
> +
> + switch (run_cachestat_overlayfs_test()) {
> + case KSFT_FAIL:
> + ksft_test_result_fail("cachestat fails with an overlayfs file\n");
> + ret = 1;
> + break;
> + case KSFT_PASS:
> + ksft_test_result_pass("cachestat works with an overlayfs file\n");
> + break;
> + case KSFT_SKIP:
> + ksft_test_result_skip("overlayfs not available\n");
> + break;
> + }
> return ret;
> }
> --
> 2.54.0
>
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH 3/3] selftests/cachestat: add an overlayfs test case
2026-06-23 16:13 ` Nhat Pham
@ 2026-06-24 12:22 ` Pavel Tikhomirov
0 siblings, 0 replies; 25+ messages in thread
From: Pavel Tikhomirov @ 2026-06-24 12:22 UTC (permalink / raw)
To: Nhat Pham
Cc: Miklos Szeredi, Amir Goldstein, Alexander Viro,
Christian Brauner, Jan Kara, Matthew Wilcox (Oracle),
Andrew Morton, Johannes Weiner, Shuah Khan, linux-unionfs,
linux-kernel, linux-fsdevel, linux-mm, linux-kselftest
On 6/23/26 18:13, Nhat Pham wrote:
> On Tue, Jun 23, 2026 at 4:15 AM Pavel Tikhomirov
> <ptikhomirov@virtuozzo.com> wrote:
>>
>> Mount an overlayfs, create and write a file in the merged directory, and
>> run cachestat() on it, reusing the existing test_cachestat() helper.
>>
>> Also bump NR_TESTS to the actual number of tests run: it was 9 while
>> ten tests were already executed, and this adds an eleventh.
>>
>> Signed-off-by: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>
>> ---
>> .../selftests/cachestat/test_cachestat.c | 75 ++++++++++++++++++-
>
> Hmm this looks strange...?
That's just how git-format-patch formats it for long path and big
change ("++...++") it hides part of the path.
>
>> 1 file changed, 74 insertions(+), 1 deletion(-)
>>
>> diff --git a/tools/testing/selftests/cachestat/test_cachestat.c b/tools/testing/selftests/cachestat/test_cachestat.c
>> index 542cd09cb4434..1662d9817c50b 100644
>> --- a/tools/testing/selftests/cachestat/test_cachestat.c
>> +++ b/tools/testing/selftests/cachestat/test_cachestat.c
>> @@ -4,21 +4,25 @@
>>
>> #include <stdio.h>
>> #include <stdbool.h>
>> +#include <stdlib.h>
>> #include <linux/kernel.h>
>> #include <linux/magic.h>
>> #include <linux/mman.h>
>> #include <sys/mman.h>
>> +#include <sys/mount.h>
>> #include <sys/shm.h>
>> +#include <sys/stat.h>
>> #include <sys/syscall.h>
>> #include <sys/vfs.h>
>> #include <unistd.h>
>> #include <string.h>
>> #include <fcntl.h>
>> #include <errno.h>
>> +#include <limits.h>
>>
>> #include "kselftest.h"
>>
>> -#define NR_TESTS 9
>> +#define NR_TESTS 11
>>
>> static const char * const dev_files[] = {
>> "/dev/zero", "/dev/null", "/dev/urandom",
>> @@ -294,6 +298,62 @@ bool run_cachestat_test(enum file_type type)
>> return ret;
>> }
>>
>> +/*
>> + * Set up an overlayfs mount and run cachestat on a freshly created file in the
>> + * merged directory. Overlayfs forwards data I/O to the underlying (upper)
>> + * inode, so the page cache lives there and not in the overlay inode's mapping.
>> + * This is a regression test for cachestat returning all zeroes on overlayfs.
>> + */
>> +static int run_cachestat_overlayfs_test(void)
>> +{
>> + char tmpl[] = "/tmp/cachestat_ovl.XXXXXX";
>> + char lower[PATH_MAX], upper[PATH_MAX], work[PATH_MAX];
>> + char merged[PATH_MAX], opts[4 * PATH_MAX], file[PATH_MAX];
>> + char *base;
>> + int ret;
>> +
>> + base = mkdtemp(tmpl);
>> + if (!base) {
>> + ksft_print_msg("Unable to create overlayfs base dir: %s\n",
>> + strerror(errno));
>> + return KSFT_FAIL;
>> + }
>> +
>> + snprintf(lower, sizeof(lower), "%s/lower", base);
>> + snprintf(upper, sizeof(upper), "%s/upper", base);
>> + snprintf(work, sizeof(work), "%s/work", base);
>> + snprintf(merged, sizeof(merged), "%s/merged", base);
>> +
>> + if (mkdir(lower, 0755) || mkdir(upper, 0755) ||
>> + mkdir(work, 0755) || mkdir(merged, 0755)) {
>> + ksft_print_msg("Unable to create overlayfs dirs: %s\n",
>> + strerror(errno));
>> + ret = KSFT_FAIL;
>> + goto cleanup;
>> + }
>> +
>> + snprintf(opts, sizeof(opts), "lowerdir=%s,upperdir=%s,workdir=%s",
>> + lower, upper, work);
>> +
>> + if (mount("overlay", merged, "overlay", 0, opts)) {
>> + ksft_print_msg("Unable to mount overlayfs (need root?): %s\n",
>> + strerror(errno));
>> + ret = KSFT_SKIP;
>> + goto cleanup;
>> + }
>> +
>> + snprintf(file, sizeof(file), "%s/merged/cachestat", base);
>> + ret = test_cachestat(file, true, true, false, 4, O_CREAT | O_RDWR, 0600);
>> +
>> + umount(merged);
>> +cleanup:
>> + /* Best-effort recursive cleanup of the temporary tree. */
>> + snprintf(opts, sizeof(opts), "rm -rf %s", base);
>> + if (system(opts))
>> + ksft_print_msg("Unable to clean up %s\n", base);
>
> nit: no helper for these? :)
This is similar to e.g. ./tools/testing/selftests/bpf/prog_tests/test_local_storage.c's
cleanup, I didn't find a common selftest helper for temporary directory removal.
>
>> + return ret;
>> +}
>> +
>> int main(void)
>> {
>> int ret;
>> @@ -361,5 +421,18 @@ int main(void)
>> ksft_test_result_fail("cachestat fails with a mmap file\n");
>> ret = 1;
>> }
>> +
>> + switch (run_cachestat_overlayfs_test()) {
>> + case KSFT_FAIL:
>> + ksft_test_result_fail("cachestat fails with an overlayfs file\n");
>> + ret = 1;
>> + break;
>> + case KSFT_PASS:
>> + ksft_test_result_pass("cachestat works with an overlayfs file\n");
>> + break;
>> + case KSFT_SKIP:
>> + ksft_test_result_skip("overlayfs not available\n");
>> + break;
>> + }
>> return ret;
>> }
>> --
>> 2.54.0
>>
--
Best regards, Pavel Tikhomirov
Senior Software Developer, Virtuozzo.
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 0/3] ovl: make cachestat() work
2026-06-23 11:14 [PATCH 0/3] ovl: make cachestat() work Pavel Tikhomirov
` (2 preceding siblings ...)
2026-06-23 11:14 ` [PATCH 3/3] selftests/cachestat: add an overlayfs test case Pavel Tikhomirov
@ 2026-06-24 20:50 ` Andrew Morton
3 siblings, 0 replies; 25+ messages in thread
From: Andrew Morton @ 2026-06-24 20:50 UTC (permalink / raw)
To: Pavel Tikhomirov
Cc: Miklos Szeredi, Amir Goldstein, Alexander Viro,
Christian Brauner, Jan Kara, Matthew Wilcox (Oracle),
Nhat Pham, Johannes Weiner, Shuah Khan, linux-unionfs,
linux-kernel, linux-fsdevel, linux-mm, linux-kselftest
On Tue, 23 Jun 2026 13:14:47 +0200 Pavel Tikhomirov <ptikhomirov@virtuozzo.com> wrote:
> We were using cachestat() for our internal Virtuozzo cache related tests
> and I saw that test behaves wrong on overlayfs, syscall returns all
> zeroes instead of any meaningful cache statistics there. Let's fix it
> similar to what was done for fadvise().
Thanks. Sashiko seems to have a found a couple of issues:
https://sashiko.dev/#/patchset/20260623111533.2285005-1-ptikhomirov@virtuozzo.com
^ permalink raw reply [flat|nested] 25+ messages in thread