Changelog:
v10:
* Reorder the arguments for archs with alignment requirements.
(patch 2) (suggested by Arnd Bergmann)
v9:
* Remove syscall from all the architectures syscall table except x86
(patch 2)
* API change: handle different cases for offset and add compat syscall.
(patch 2) (suggested by Johannes Weiner and Arnd Bergmann)
v8:
* Add syscall to mips syscall tables (detected by kernel test robot)
(patch 2)
* Add a missing return (suggested by Yu Zhao) (patch 2)
v7:
* Fix and use lru_gen_test_recent (suggested by Brian Foster)
(patch 2)
* Small formatting and organizational fixes
v6:
* Add a missing fdput() (suggested by Brian Foster) (patch 2)
* Replace cstat_size with cstat_version (suggested by Brian Foster)
(patch 2)
* Add conditional resched to the xas walk. (suggested by Hillf Danton)
(patch 2)
v5:
* Separate first patch into its own series.
(suggested by Andrew Morton)
* Expose filemap_cachestat() to non-syscall usage
(patch 2) (suggested by Brian Foster).
* Fix some build errors from last version.
(patch 2)
* Explain eviction and recent eviction in the draft man page and
documentation (suggested by Andrew Morton).
(patch 2)
v4:
* Refactor cachestat and move it to mm/filemap.c (patch 3)
(suggested by Brian Foster)
* Remove redundant checks (!folio, access_ok)
(patch 3) (suggested by Matthew Wilcox and Al Viro)
* Fix a bug in handling multipages folio.
(patch 3) (suggested by Matthew Wilcox)
* Add a selftest for shmem files, which can be used to test huge
pages (patch 4) (suggested by Johannes Weiner)
v3:
* Fix some minor formatting issues and build errors.
* Add the new syscall entry to missing architecture syscall tables.
(patch 3).
* Add flags argument for the syscall. (patch 3).
* Clean up the recency refactoring (patch 2) (suggested by Yu Zhao)
* Add the new Kconfig (CONFIG_CACHESTAT) to disable the syscall.
(patch 3) (suggested by Josh Triplett)
v2:
* len == 0 means query to EOF. len < 0 is invalid.
(patch 3) (suggested by Brian Foster)
* Make cachestat extensible by adding the `cstat_size` argument in the
syscall (patch 3)
There is currently no good way to query the page cache state of large
file sets and directory trees. There is mincore(), but it scales poorly:
the kernel writes out a lot of bitmap data that userspace has to
aggregate, when the user really doesn not care about per-page information
in that case. The user also needs to mmap and unmap each file as it goes
along, which can be quite slow as well.
This series of patches introduces a new system call, cachestat, that
summarizes the page cache statistics (number of cached pages, dirty
pages, pages marked for writeback, evicted pages etc.) of a file, in a
specified range of bytes. It also include a selftest suite that tests some
typical usage. Currently, the syscall is only wired in for x86
architecture.
This interface is inspired by past discussion and concerns with fincore,
which has a similar design (and as a result, issues) as mincore.
Relevant links:
https://lkml.indiana.edu/hypermail/linux/kernel/1302.1/04207.htmlhttps://lkml.indiana.edu/hypermail/linux/kernel/1302.1/04209.html
For comparison with mincore, I ran both syscalls on a 2TB sparse file:
Using mincore:
real 0m37.510s
user 0m2.934s
sys 0m34.558s
Using cachestat:
real 0m0.009s
user 0m0.000s
sys 0m0.009s
This series should be applied on top of:
workingset: fix confusion around eviction vs refault container
https://lkml.org/lkml/2023/1/4/1066
This series consist of 3 patches:
Nhat Pham (3):
workingset: refactor LRU refault to expose refault recency check
cachestat: implement cachestat syscall
selftests: Add selftests for cachestat
MAINTAINERS | 7 +
arch/x86/entry/syscalls/syscall_32.tbl | 1 +
arch/x86/entry/syscalls/syscall_64.tbl | 1 +
include/linux/compat.h | 4 +-
include/linux/fs.h | 3 +
include/linux/swap.h | 1 +
include/linux/syscalls.h | 3 +
include/uapi/asm-generic/unistd.h | 5 +-
include/uapi/linux/mman.h | 9 +
init/Kconfig | 10 +
kernel/sys_ni.c | 1 +
mm/filemap.c | 158 +++++++++++
mm/workingset.c | 142 ++++++----
tools/testing/selftests/Makefile | 1 +
tools/testing/selftests/cachestat/.gitignore | 2 +
tools/testing/selftests/cachestat/Makefile | 8 +
.../selftests/cachestat/test_cachestat.c | 256 ++++++++++++++++++
17 files changed, 564 insertions(+), 48 deletions(-)
create mode 100644 tools/testing/selftests/cachestat/.gitignore
create mode 100644 tools/testing/selftests/cachestat/Makefile
create mode 100644 tools/testing/selftests/cachestat/test_cachestat.c
base-commit: 1440f576022887004f719883acb094e7e0dd4944
prerequisite-patch-id: 171a43d333e1b267ce14188a5beaea2f313787fb
--
2.39.1
In preparation for computing recently evicted pages in cachestat,
refactor workingset_refault and lru_gen_refault to expose a helper
function that would test if an evicted page is recently evicted.
Signed-off-by: Nhat Pham <nphamcs@gmail.com>
---
include/linux/swap.h | 1 +
mm/workingset.c | 142 +++++++++++++++++++++++++++++--------------
2 files changed, 97 insertions(+), 46 deletions(-)
There is currently no good way to query the page cache state of large
file sets and directory trees. There is mincore(), but it scales poorly:
the kernel writes out a lot of bitmap data that userspace has to
aggregate, when the user really doesn not care about per-page
information in that case. The user also needs to mmap and unmap each
file as it goes along, which can be quite slow as well.
This patch implements a new syscall that queries cache state of a file
and summarizes the number of cached pages, number of dirty pages, number
of pages marked for writeback, number of (recently) evicted pages, etc.
in a given range. Currently, the syscall is only wired in for x86
architecture.
NAME
cachestat - query the page cache statistics of a file.
SYNOPSIS
#include <sys/mman.h>
struct cachestat {
__u64 nr_cache;
__u64 nr_dirty;
__u64 nr_writeback;
__u64 nr_evicted;
__u64 nr_recently_evicted;
};
int cachestat(unsigned int fd, size_t len, loff_t off,
struct cachestat *cstat, unsigned int flags);
DESCRIPTION
cachestat() queries the number of cached pages, number of dirty
pages, number of pages marked for writeback, number of evicted
pages, number of recently evicted pages, in the bytes range given by
`off` and `len`.
An evicted page is a page that is previously in the page cache but
has been evicted since. A page is recently evicted if its last
eviction was recent enough that its reentry to the cache would
indicate that it is actively being used by the system, and that
there is memory pressure on the system.
These values are returned in a cachestat struct, whose address is
given by the `cstat` argument.
The `off` and `len` arguments must be non-negative integers. If
`len` > 0, the queried range is [`off`, `off` + `len`]. If `len` ==
0, we will query in the range from `off` to the end of the file.
The `flags` argument is unused for now, but is included for future
extensibility. User should pass 0 (i.e no flag specified).
RETURN VALUE
On success, cachestat returns 0. On error, -1 is returned, and errno
is set to indicate the error.
ERRORS
EFAULT cstat points to an invalid address.
EINVAL invalid offset or flags.
EBADF invalid file descriptor.
Signed-off-by: Nhat Pham <nphamcs@gmail.com>
---
arch/x86/entry/syscalls/syscall_32.tbl | 1 +
arch/x86/entry/syscalls/syscall_64.tbl | 1 +
include/linux/compat.h | 4 +-
include/linux/fs.h | 3 +
include/linux/syscalls.h | 3 +
include/uapi/asm-generic/unistd.h | 5 +-
include/uapi/linux/mman.h | 9 ++
init/Kconfig | 10 ++
kernel/sys_ni.c | 1 +
mm/filemap.c | 158 +++++++++++++++++++++++++
10 files changed, 193 insertions(+), 2 deletions(-)
@@ -372,6 +372,7 @@ 448 common process_mrelease sys_process_mrelease 449 common futex_waitv sys_futex_waitv 450 common set_mempolicy_home_node sys_set_mempolicy_home_node+451 common cachestat sys_cachestat # # Due to a historical design error, certain syscalls are numbered differently
@@ -1056,6 +1057,8 @@ asmlinkage long sys_memfd_secret(unsigned int flags);asmlinkagelongsys_set_mempolicy_home_node(unsignedlongstart,unsignedlonglen,unsignedlonghome_node,unsignedlongflags);+asmlinkagelongsys_cachestat(unsignedintfd,size_tlen,loff_toffset,+structcachestat__user*cstat,unsignedintflags);/**Architecture-specificsystemcalls
@@ -1798,6 +1798,16 @@ config RSEQIfunsure,sayY.+configCACHESTAT_SYSCALL+bool"Enable cachestat() system call"ifEXPERT+defaulty+help+Enablethecachestatsystemcall,whichqueriesthepagecache+statisticsofafile(numberofcachedpages,dirtypages,+pagesmarkedforwriteback,(recently)evictedpages).++IfunsuresayYhere.+configDEBUG_RSEQdefaultnbool"Enabled debugging of rseq() system call"ifEXPERT
@@ -0,0 +1,256 @@+// SPDX-License-Identifier: GPL-2.0+#define _GNU_SOURCE++#include<stdio.h>+#include<stdbool.h>+#include<linux/kernel.h>+#include<linux/mman.h>+#include<sys/mman.h>+#include<sys/shm.h>+#include<sys/syscall.h>+#include<unistd.h>+#include<string.h>+#include<fcntl.h>+#include<errno.h>++#include"../kselftest.h"++staticconstchar*constdev_files[]={+"/dev/zero","/dev/null","/dev/urandom",+"/proc/version","/proc"+};+staticconstintcachestat_nr=451;++voidprint_cachestat(structcachestat*cs)+{+ksft_print_msg(+"Using cachestat: Cached: %lu, Dirty: %lu, Writeback: %lu, Evicted: %lu, Recently Evicted: %lu\n",+cs->nr_cache,cs->nr_dirty,cs->nr_writeback,+cs->nr_evicted,cs->nr_recently_evicted);+}++boolwrite_exactly(intfd,size_tfilesize)+{+chardata[filesize];+boolret=true;+intrandom_fd=open("/dev/urandom",O_RDONLY);++if(random_fd<0){+ksft_print_msg("Unable to access urandom.\n");+ret=false;+gotoout;+}else{+intremained=filesize;+char*cursor=data;++while(remained){+ssize_tread_len=read(random_fd,cursor,remained);++if(read_len<=0){+ksft_print_msg("Unable to read from urandom.\n");+ret=false;+gotoclose_random_fd;+}++remained-=read_len;+cursor+=read_len;+}++/* write random data to fd */+remained=filesize;+cursor=data;+while(remained){+ssize_twrite_len=write(fd,cursor,remained);++if(write_len<=0){+ksft_print_msg("Unable write random data to file.\n");+ret=false;+gotoclose_random_fd;+}++remained-=write_len;+cursor+=write_len;+}+}++close_random_fd:+close(random_fd);+out:+returnret;+}++/*+*Open/createthefileatfilename,(optionally)writerandomdatatoit+*(exactlynum_pages),thentestthecachestatsyscallonthisfile.+*+*Iftest_fsync==true,fsyncthefile,thencheckthenumberofdirty+*pages.+*/+booltest_cachestat(constchar*filename,boolwrite_random,boolcreate,+booltest_fsync,unsignedlongnum_pages,intopen_flags,+mode_topen_mode)+{+size_tPS=sysconf(_SC_PAGESIZE);+intfilesize=num_pages*PS;+boolret=true;+longsyscall_ret;+structcachestatcs;++intfd=open(filename,open_flags,open_mode);++if(fd==-1){+ksft_print_msg("Unable to create/open file.\n");+gotoout;+}else{+ksft_print_msg("Create/open %s\n",filename);+}++if(write_random){+if(!write_exactly(fd,filesize)){+ksft_print_msg("Unable to access urandom.\n");+ret=false;+gotoout1;+}+}++syscall_ret=syscall(cachestat_nr,fd,filesize,0,&cs,0);++ksft_print_msg("Cachestat call returned %ld\n",syscall_ret);++if(syscall_ret){+ksft_print_msg("Cachestat returned non-zero.\n");+ret=false;+gotoout1;++}else{+print_cachestat(&cs);++if(write_random){+if(cs.nr_cache+cs.nr_evicted!=num_pages){+ksft_print_msg(+"Total number of cached and evicted pages is off.\n");+ret=false;+}+}+}++if(test_fsync){+if(fsync(fd)){+ksft_print_msg("fsync fails.\n");+ret=false;+}else{+syscall_ret=syscall(cachestat_nr,fd,filesize,0,&cs,0);++ksft_print_msg("Cachestat call (after fsync) returned %ld\n",+syscall_ret);++if(!syscall_ret){+print_cachestat(&cs);++if(cs.nr_dirty){+ret=false;+ksft_print_msg(+"Number of dirty should be zero after fsync.\n");+}+}else{+ksft_print_msg("Cachestat (after fsync) returned non-zero.\n");+ret=false;+gotoout1;+}+}+}++out1:+close(fd);++if(create)+remove(filename);+out:+returnret;+}++booltest_cachestat_shmem(void)+{+size_tPS=sysconf(_SC_PAGESIZE);+size_tfilesize=PS*512*2;/* 2 2MB huge pages */+intsyscall_ret;+off_toff=PS;+size_tcompute_len=PS*512;+char*filename="tmpshmcstat";+structcachestatcs;+boolret=true;+unsignedlongnum_pages=compute_len/PS;+intfd=shm_open(filename,O_CREAT|O_RDWR,0600);++if(fd<0){+ksft_print_msg("Unable to create shmem file.\n");+ret=false;+gotoout;+}++if(ftruncate(fd,filesize)){+ksft_print_msg("Unable to trucate shmem file.\n");+ret=false;+gotoclose_fd;+}++if(!write_exactly(fd,filesize)){+ksft_print_msg("Unable to write to shmem file.\n");+ret=false;+gotoclose_fd;+}++syscall_ret=syscall(cachestat_nr,fd,compute_len,off,&cs,0);++if(syscall_ret){+ksft_print_msg("Cachestat returned non-zero.\n");+ret=false;+gotoclose_fd;+}else{+print_cachestat(&cs);+if(cs.nr_cache+cs.nr_evicted!=num_pages){+ksft_print_msg(+"Total number of cached and evicted pages is off.\n");+ret=false;+}+}++close_fd:+shm_unlink(filename);+out:+returnret;+}++intmain(void)+{+intret=0;++for(inti=0;i<5;i++){+constchar*dev_filename=dev_files[i];++if(test_cachestat(dev_filename,false,false,false,+4,O_RDONLY,0400))+ksft_test_result_pass("cachestat works with %s\n",dev_filename);+else{+ksft_test_result_fail("cachestat fails with %s\n",dev_filename);+ret=1;+}+}++if(test_cachestat("tmpfilecachestat",true,true,+true,4,O_CREAT|O_RDWR,0400|0600))+ksft_test_result_pass("cachestat works with a normal file\n");+else{+ksft_test_result_fail("cachestat fails with normal file\n");+ret=1;+}++if(test_cachestat_shmem())+ksft_test_result_pass("cachestat works with a shmem file\n");+else{+ksft_test_result_fail("cachestat fails with a shmem file\n");+ret=1;+}++returnret;+}--
From: kernel test robot <hidden> Date: 2023-02-19 09:48:15
Hi Nhat,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on 1440f576022887004f719883acb094e7e0dd4944]
url: https://github.com/intel-lab-lkp/linux/commits/Nhat-Pham/workingset-refactor-LRU-refault-to-expose-refault-recency-check/20230219-153500
base: 1440f576022887004f719883acb094e7e0dd4944
patch link: https://lore.kernel.org/r/20230219073318.366189-3-nphamcs%40gmail.com
patch subject: [PATCH v10 2/3] cachestat: implement cachestat syscall
config: hexagon-randconfig-r014-20230219 (https://download.01.org/0day-ci/archive/20230219/202302191728.rXGLcydi-lkp@intel.com/config)
compiler: clang version 17.0.0 (https://github.com/llvm/llvm-project db89896bbbd2251fff457699635acbbedeead27f)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/d691a3b4a696ab20c9c5fe86c624e9e36814523c
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Nhat-Pham/workingset-refactor-LRU-refault-to-expose-refault-recency-check/20230219-153500
git checkout d691a3b4a696ab20c9c5fe86c624e9e36814523c
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=hexagon olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=hexagon SHELL=/bin/bash
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot [off-list ref]
| Link: https://lore.kernel.org/oe-kbuild-all/202302191728.rXGLcydi-lkp@intel.com/
All warnings (new ones prefixed by >>):
In file included from mm/filemap.c:20:
In file included from include/linux/kernel_stat.h:9:
In file included from include/linux/interrupt.h:11:
In file included from include/linux/hardirq.h:11:
In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1:
In file included from include/asm-generic/hardirq.h:17:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:13:
In file included from arch/hexagon/include/asm/io.h:334:
include/asm-generic/io.h:547:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
val = __raw_readb(PCI_IOBASE + addr);
~~~~~~~~~~ ^
include/asm-generic/io.h:560:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
~~~~~~~~~~ ^
include/uapi/linux/byteorder/little_endian.h:37:51: note: expanded from macro '__le16_to_cpu'
#define __le16_to_cpu(x) ((__force __u16)(__le16)(x))
^
In file included from mm/filemap.c:20:
In file included from include/linux/kernel_stat.h:9:
In file included from include/linux/interrupt.h:11:
In file included from include/linux/hardirq.h:11:
In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1:
In file included from include/asm-generic/hardirq.h:17:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:13:
In file included from arch/hexagon/include/asm/io.h:334:
include/asm-generic/io.h:573:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
~~~~~~~~~~ ^
include/uapi/linux/byteorder/little_endian.h:35:51: note: expanded from macro '__le32_to_cpu'
#define __le32_to_cpu(x) ((__force __u32)(__le32)(x))
^
In file included from mm/filemap.c:20:
In file included from include/linux/kernel_stat.h:9:
In file included from include/linux/interrupt.h:11:
In file included from include/linux/hardirq.h:11:
In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1:
In file included from include/asm-generic/hardirq.h:17:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:13:
In file included from arch/hexagon/include/asm/io.h:334:
include/asm-generic/io.h:584:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
__raw_writeb(value, PCI_IOBASE + addr);
~~~~~~~~~~ ^
include/asm-generic/io.h:594:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
__raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
~~~~~~~~~~ ^
include/asm-generic/io.h:604:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
__raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
~~~~~~~~~~ ^
quoted
mm/filemap.c:4035:6: warning: no previous prototype for function 'ksys_cachestat' [-Wmissing-prototypes]
long ksys_cachestat(unsigned int fd, size_t len, loff_t off,
^
mm/filemap.c:4035:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
long ksys_cachestat(unsigned int fd, size_t len, loff_t off,
^
static
7 warnings generated.
vim +/ksys_cachestat +4035 mm/filemap.c
4033
4034 #ifdef CONFIG_CACHESTAT_SYSCALL
4035 long ksys_cachestat(unsigned int fd, size_t len, loff_t off,
I find this comment hard to understand. First it talks about "the
folio", but it doesn't pass a folio. Then it talks about "the
references", but I don't have any idea what those are either.
I think what you mean is,
* Test if the shadow entry is for a folio which was recently evicted.
* Fills in @memcgid, @pgdat, @token and @workingset with values
* extracted from the shadow entry.
+/*+ * Test if the folio is recently evicted by checking if+ * refault distance of shadow exceeds workingset size. *- * Calculates and evaluates the refault distance of the previously- * evicted folio in the context of the node and the memcg whose memory- * pressure caused the eviction.+ * As a side effect, populate workingset with the value+ * unpacked from shadow. */
1. Shouldn't this be kernel-doc?
2. Again, don't use the term "side effect" here. It's just one of
the things that the function _does_.
I think this hunk should be:
-#include <asm/mman.h>
+#include <linux/mman.h>
(linux/mman.h includes uapi/linux/mman.h, which includes asm/mman.h)
quoted hunk
+/**+ * filemap_cachestat() - compute the page cache statistics of a mapping+ * @mapping: The mapping to compute the statistics for.+ * @first_index: The starting page cache index.+ * @last_index: The final page index (inclusive).+ * @cs: the cachestat struct to write the result to.+ *+ * This will query the page cache statistics of a mapping in the+ * page range of [first_index, last_index] (inclusive). The statistics+ * queried include: number of dirty pages, number of pages marked for+ * writeback, and the number of (recently) evicted pages.+ */
Do we care that this isn't going to work for hugetlbfs?
quoted hunk
+ rcu_read_lock();+ xas_for_each(&xas, folio, last_index) {+ unsigned long nr_pages;+ pgoff_t folio_first_index, folio_last_index;++ if (xas_retry(&xas, folio))+ continue;++ nr_pages = folio_nr_pages(folio);+ folio_first_index = folio_pgoff(folio);+ folio_last_index = folio_first_index + nr_pages - 1;++ /* Folios might straddle the range boundaries, only count covered subpages */
I find this comment hard to understand. First it talks about "the
folio", but it doesn't pass a folio. Then it talks about "the
references", but I don't have any idea what those are either.
I think what you mean is,
* Test if the shadow entry is for a folio which was recently evicted.
* Fills in @memcgid, @pgdat, @token and @workingset with values
* extracted from the shadow entry.
I'll fix this comment in the next version. Thanks for the suggestion!
+/*+ * Test if the folio is recently evicted by checking if+ * refault distance of shadow exceeds workingset size. *- * Calculates and evaluates the refault distance of the previously- * evicted folio in the context of the node and the memcg whose memory- * pressure caused the eviction.+ * As a side effect, populate workingset with the value+ * unpacked from shadow. */
1. Shouldn't this be kernel-doc?
Hmm good point...
2. Again, don't use the term "side effect" here. It's just one of
the things that the function _does_.
I think this hunk should be:
-#include <asm/mman.h>
+#include <linux/mman.h>
(linux/mman.h includes uapi/linux/mman.h, which includes asm/mman.h)
quoted
+/**+ * filemap_cachestat() - compute the page cache statistics of a mapping+ * @mapping: The mapping to compute the statistics for.+ * @first_index: The starting page cache index.+ * @last_index: The final page index (inclusive).+ * @cs: the cachestat struct to write the result to.+ *+ * This will query the page cache statistics of a mapping in the+ * page range of [first_index, last_index] (inclusive). The statistics+ * queried include: number of dirty pages, number of pages marked for+ * writeback, and the number of (recently) evicted pages.+ */
Do we care that this isn't going to work for hugetlbfs?
I ran a quick test using hugetlbfs. It looks like the current
implementation is treating it in accordance to the multi-page
folio case we discussed earlier, i.e:
- Returned number of "pages" is in terms of the number of
base/small pages (i.e 512 dirty pages instead of 1 dirty
huge page etc.)
- If we touch one byte in the huge page, it would report the
entire huge page as dirty, but again in terms of the underlying
pages.
Is this what you have in mind, or is there another edge
case that I'm missing...?
Thanks for the comments and suggestions, Matthew!
quoted
+ rcu_read_lock();+ xas_for_each(&xas, folio, last_index) {+ unsigned long nr_pages;+ pgoff_t folio_first_index, folio_last_index;++ if (xas_retry(&xas, folio))+ continue;++ nr_pages = folio_nr_pages(folio);+ folio_first_index = folio_pgoff(folio);+ folio_last_index = folio_first_index + nr_pages - 1;++ /* Folios might straddle the range boundaries, only count covered subpages */
From: Matthew Wilcox <willy@infradead.org> Date: 2023-03-03 07:03:48
On Thu, Mar 02, 2023 at 10:55:48PM -0800, Nhat Pham wrote:
On Sun, Feb 19, 2023 at 4:21 AM Matthew Wilcox [off-list ref] wrote:
quoted
quoted
+/**+ * filemap_cachestat() - compute the page cache statistics of a mapping+ * @mapping: The mapping to compute the statistics for.+ * @first_index: The starting page cache index.+ * @last_index: The final page index (inclusive).+ * @cs: the cachestat struct to write the result to.+ *+ * This will query the page cache statistics of a mapping in the+ * page range of [first_index, last_index] (inclusive). The statistics+ * queried include: number of dirty pages, number of pages marked for+ * writeback, and the number of (recently) evicted pages.+ */
Do we care that this isn't going to work for hugetlbfs?
I ran a quick test using hugetlbfs. It looks like the current
implementation is treating it in accordance to the multi-page
folio case we discussed earlier, i.e:
- Returned number of "pages" is in terms of the number of
base/small pages (i.e 512 dirty pages instead of 1 dirty
huge page etc.)
- If we touch one byte in the huge page, it would report the
entire huge page as dirty, but again in terms of the underlying
pages.
Is this what you have in mind, or is there another edge
case that I'm missing...?
Hugetlbfs indexes its pages by hugepage number rather than by smallpage
number. Imagine you have a 2MB folio at offset 4MB into the file.
Filesystems other than hugetlbfs store it at indices 1024-1535.
hugetlbfs stores it at index 2.
So your report probably seems to work, but if you ask it about a
range, you might be surprised by how wide that range will cover for
hugetlbfs.
I know Sidhartha is working on fixing that, but I'm not sure if what he
has is working yet.
On Thu, Mar 2, 2023 at 11:03 PM Matthew Wilcox [off-list ref] wrote:
On Thu, Mar 02, 2023 at 10:55:48PM -0800, Nhat Pham wrote:
quoted
On Sun, Feb 19, 2023 at 4:21 AM Matthew Wilcox [off-list ref] wrote:
quoted
quoted
+/**+ * filemap_cachestat() - compute the page cache statistics of a mapping+ * @mapping: The mapping to compute the statistics for.+ * @first_index: The starting page cache index.+ * @last_index: The final page index (inclusive).+ * @cs: the cachestat struct to write the result to.+ *+ * This will query the page cache statistics of a mapping in the+ * page range of [first_index, last_index] (inclusive). The statistics+ * queried include: number of dirty pages, number of pages marked for+ * writeback, and the number of (recently) evicted pages.+ */
Do we care that this isn't going to work for hugetlbfs?
I ran a quick test using hugetlbfs. It looks like the current
implementation is treating it in accordance to the multi-page
folio case we discussed earlier, i.e:
- Returned number of "pages" is in terms of the number of
base/small pages (i.e 512 dirty pages instead of 1 dirty
huge page etc.)
- If we touch one byte in the huge page, it would report the
entire huge page as dirty, but again in terms of the underlying
pages.
Is this what you have in mind, or is there another edge
case that I'm missing...?
Hugetlbfs indexes its pages by hugepage number rather than by smallpage
number. Imagine you have a 2MB folio at offset 4MB into the file.
Filesystems other than hugetlbfs store it at indices 1024-1535.
hugetlbfs stores it at index 2.
So your report probably seems to work, but if you ask it about a
range, you might be surprised by how wide that range will cover for
hugetlbfs.
I know Sidhartha is working on fixing that, but I'm not sure if what he
has is working yet.
Oh I see! Thanks for letting us know about this, Matthew.
In that case, I think we should just drop support for hugetlbfs for now.
It's also an odd ball - not swappable, no backing files, not fully
under user management, etc., so it's less interesting with respect to
cachestat as well.
In the future, we can always lift this restriction with a follow-up patch
or with Sidhartha's fix.
1. Why is this in fs.h instead of pagemap.h?
2. Why is it not static, since it's only used by the syscall,
which is also in filemap.c?
Actually, I think I can remove its occurrence in another header file
altogether. I think it was an artifact from a past iteration, which
has filemap_cachestat in filemap.c but the syscall in its own
file (cachestat.c).
And you're right - we only have one use case for it now
(for the syscall). I'll make it static.
I think this hunk should be:
-#include <asm/mman.h>
+#include <linux/mman.h>
Will be fixed in the next version - thanks for letting me know!
(linux/mman.h includes uapi/linux/mman.h, which includes asm/mman.h)
quoted
+/**+ * filemap_cachestat() - compute the page cache statistics of a mapping+ * @mapping: The mapping to compute the statistics for.+ * @first_index: The starting page cache index.+ * @last_index: The final page index (inclusive).+ * @cs: the cachestat struct to write the result to.+ *+ * This will query the page cache statistics of a mapping in the+ * page range of [first_index, last_index] (inclusive). The statistics+ * queried include: number of dirty pages, number of pages marked for+ * writeback, and the number of (recently) evicted pages.+ */
Do we care that this isn't going to work for hugetlbfs?
quoted
+ rcu_read_lock();+ xas_for_each(&xas, folio, last_index) {+ unsigned long nr_pages;+ pgoff_t folio_first_index, folio_last_index;++ if (xas_retry(&xas, folio))+ continue;++ nr_pages = folio_nr_pages(folio);+ folio_first_index = folio_pgoff(folio);+ folio_last_index = folio_first_index + nr_pages - 1;++ /* Folios might straddle the range boundaries, only count covered subpages */
s/subpages/pages/
Yeah that comment is incorrect/confusing with the pages terminology.
It'll be fixed!