linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v10 0/3] cachestat: a new syscall for page cache state of files
@ 2023-02-19  7:33 Nhat Pham
  2023-02-19  7:33 ` [PATCH v10 1/3] workingset: refactor LRU refault to expose refault recency check Nhat Pham
                   ` (2 more replies)
  0 siblings, 3 replies; 15+ messages in thread
From: Nhat Pham @ 2023-02-19  7:33 UTC (permalink / raw)
  To: akpm
  Cc: hannes, linux-mm, linux-kernel, bfoster, willy, arnd, linux-api,
	kernel-team

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.html
https://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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v10 1/3] workingset: refactor LRU refault to expose refault recency check
  2023-02-19  7:33 [PATCH v10 0/3] cachestat: a new syscall for page cache state of files Nhat Pham
@ 2023-02-19  7:33 ` Nhat Pham
  2023-02-19 12:05   ` Matthew Wilcox
  2023-02-19  7:33 ` [PATCH v10 2/3] cachestat: implement cachestat syscall Nhat Pham
  2023-02-19  7:33 ` [PATCH v10 3/3] selftests: Add selftests for cachestat Nhat Pham
  2 siblings, 1 reply; 15+ messages in thread
From: Nhat Pham @ 2023-02-19  7:33 UTC (permalink / raw)
  To: akpm
  Cc: hannes, linux-mm, linux-kernel, bfoster, willy, arnd, linux-api,
	kernel-team

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(-)

diff --git a/include/linux/swap.h b/include/linux/swap.h
index a18cf4b7c724..dae6f6f955eb 100644
--- a/include/linux/swap.h
+++ b/include/linux/swap.h
@@ -361,6 +361,7 @@ static inline void folio_set_swap_entry(struct folio *folio, swp_entry_t entry)
 }

 /* linux/mm/workingset.c */
+bool workingset_test_recent(void *shadow, bool file, bool *workingset);
 void workingset_age_nonresident(struct lruvec *lruvec, unsigned long nr_pages);
 void *workingset_eviction(struct folio *folio, struct mem_cgroup *target_memcg);
 void workingset_refault(struct folio *folio, void *shadow);
diff --git a/mm/workingset.c b/mm/workingset.c
index 79585d55c45d..5f1d20a0a2fd 100644
--- a/mm/workingset.c
+++ b/mm/workingset.c
@@ -244,6 +244,30 @@ static void *lru_gen_eviction(struct folio *folio)
 	return pack_shadow(mem_cgroup_id(memcg), pgdat, token, refs);
 }

+/*
+ * Test if the folio is recently evicted.
+ *
+ * As a side effect, also populates the references with
+ * values unpacked from the shadow of the evicted folio.
+ */
+static bool lru_gen_test_recent(void *shadow, bool file, int *memcgid,
+		struct pglist_data **pgdat, unsigned long *token, bool *workingset)
+{
+	struct mem_cgroup *eviction_memcg;
+	struct lruvec *lruvec;
+	struct lru_gen_struct *lrugen;
+	unsigned long min_seq;
+
+	unpack_shadow(shadow, memcgid, pgdat, token, workingset);
+	eviction_memcg = mem_cgroup_from_id(*memcgid);
+
+	lruvec = mem_cgroup_lruvec(eviction_memcg, *pgdat);
+	lrugen = &lruvec->lrugen;
+
+	min_seq = READ_ONCE(lrugen->min_seq[file]);
+	return (*token >> LRU_REFS_WIDTH) == (min_seq & (EVICTION_MASK >> LRU_REFS_WIDTH));
+}
+
 static void lru_gen_refault(struct folio *folio, void *shadow)
 {
 	int hist, tier, refs;
@@ -258,23 +282,22 @@ static void lru_gen_refault(struct folio *folio, void *shadow)
 	int type = folio_is_file_lru(folio);
 	int delta = folio_nr_pages(folio);

-	unpack_shadow(shadow, &memcg_id, &pgdat, &token, &workingset);
-
-	if (pgdat != folio_pgdat(folio))
-		return;
-
 	rcu_read_lock();

+	if (!lru_gen_test_recent(shadow, type, &memcg_id, &pgdat, &token,
+			&workingset))
+		goto unlock;
+
 	memcg = folio_memcg_rcu(folio);
 	if (memcg_id != mem_cgroup_id(memcg))
 		goto unlock;

+	if (pgdat != folio_pgdat(folio))
+		return;
+
 	lruvec = mem_cgroup_lruvec(memcg, pgdat);
 	lrugen = &lruvec->lrugen;
-
 	min_seq = READ_ONCE(lrugen->min_seq[type]);
-	if ((token >> LRU_REFS_WIDTH) != (min_seq & (EVICTION_MASK >> LRU_REFS_WIDTH)))
-		goto unlock;

 	hist = lru_hist_from_seq(min_seq);
 	/* see the comment in folio_lru_refs() */
@@ -306,6 +329,12 @@ static void *lru_gen_eviction(struct folio *folio)
 	return NULL;
 }

+static bool lru_gen_test_recent(void *shadow, bool file, int *memcgid,
+		struct pglist_data **pgdat, unsigned long *token, bool *workingset)
+{
+	return false;
+}
+
 static void lru_gen_refault(struct folio *folio, void *shadow)
 {
 }
@@ -373,40 +402,31 @@ void *workingset_eviction(struct folio *folio, struct mem_cgroup *target_memcg)
 				folio_test_workingset(folio));
 }

-/**
- * workingset_refault - Evaluate the refault of a previously evicted folio.
- * @folio: The freshly allocated replacement folio.
- * @shadow: Shadow entry of the evicted folio.
+/*
+ * 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.
  */
-void workingset_refault(struct folio *folio, void *shadow)
+bool workingset_test_recent(void *shadow, bool file, bool *workingset)
 {
-	bool file = folio_is_file_lru(folio);
 	struct mem_cgroup *eviction_memcg;
 	struct lruvec *eviction_lruvec;
 	unsigned long refault_distance;
 	unsigned long workingset_size;
-	struct pglist_data *pgdat;
-	struct mem_cgroup *memcg;
-	unsigned long eviction;
-	struct lruvec *lruvec;
 	unsigned long refault;
-	bool workingset;
 	int memcgid;
-	long nr;
+	struct pglist_data *pgdat;
+	unsigned long eviction;

-	if (lru_gen_enabled()) {
-		lru_gen_refault(folio, shadow);
-		return;
-	}
+	if (lru_gen_enabled())
+		return lru_gen_test_recent(shadow, file, &memcgid, &pgdat, &eviction,
+			workingset);

-	unpack_shadow(shadow, &memcgid, &pgdat, &eviction, &workingset);
+	unpack_shadow(shadow, &memcgid, &pgdat, &eviction, workingset);
 	eviction <<= bucket_order;

-	rcu_read_lock();
 	/*
 	 * Look up the memcg associated with the stored ID. It might
 	 * have been deleted since the folio's eviction.
@@ -425,7 +445,8 @@ void workingset_refault(struct folio *folio, void *shadow)
 	 */
 	eviction_memcg = mem_cgroup_from_id(memcgid);
 	if (!mem_cgroup_disabled() && !eviction_memcg)
-		goto out;
+		return false;
+
 	eviction_lruvec = mem_cgroup_lruvec(eviction_memcg, pgdat);
 	refault = atomic_long_read(&eviction_lruvec->nonresident_age);

@@ -447,21 +468,6 @@ void workingset_refault(struct folio *folio, void *shadow)
 	 */
 	refault_distance = (refault - eviction) & EVICTION_MASK;

-	/*
-	 * The activation decision for this folio is made at the level
-	 * where the eviction occurred, as that is where the LRU order
-	 * during folio reclaim is being determined.
-	 *
-	 * However, the cgroup that will own the folio is the one that
-	 * is actually experiencing the refault event.
-	 */
-	nr = folio_nr_pages(folio);
-	memcg = folio_memcg(folio);
-	pgdat = folio_pgdat(folio);
-	lruvec = mem_cgroup_lruvec(memcg, pgdat);
-
-	mod_lruvec_state(lruvec, WORKINGSET_REFAULT_BASE + file, nr);
-
 	mem_cgroup_flush_stats_delayed();
 	/*
 	 * Compare the distance to the existing workingset size. We
@@ -483,7 +489,51 @@ void workingset_refault(struct folio *folio, void *shadow)
 						     NR_INACTIVE_ANON);
 		}
 	}
-	if (refault_distance > workingset_size)
+
+	return refault_distance <= workingset_size;
+}
+
+/**
+ * workingset_refault - Evaluate the refault of a previously evicted folio.
+ * @folio: The freshly allocated replacement folio.
+ * @shadow: Shadow entry of the evicted folio.
+ *
+ * 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.
+ */
+void workingset_refault(struct folio *folio, void *shadow)
+{
+	bool file = folio_is_file_lru(folio);
+	struct pglist_data *pgdat;
+	struct mem_cgroup *memcg;
+	struct lruvec *lruvec;
+	bool workingset;
+	long nr;
+
+	if (lru_gen_enabled()) {
+		lru_gen_refault(folio, shadow);
+		return;
+	}
+
+	rcu_read_lock();
+
+	/*
+	 * The activation decision for this folio is made at the level
+	 * where the eviction occurred, as that is where the LRU order
+	 * during folio reclaim is being determined.
+	 *
+	 * However, the cgroup that will own the folio is the one that
+	 * is actually experiencing the refault event.
+	 */
+	nr = folio_nr_pages(folio);
+	memcg = folio_memcg(folio);
+	pgdat = folio_pgdat(folio);
+	lruvec = mem_cgroup_lruvec(memcg, pgdat);
+
+	mod_lruvec_state(lruvec, WORKINGSET_REFAULT_BASE + file, nr);
+
+	if (!workingset_test_recent(shadow, file, &workingset))
 		goto out;

 	folio_set_active(folio);
--
2.39.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v10 2/3] cachestat: implement cachestat syscall
  2023-02-19  7:33 [PATCH v10 0/3] cachestat: a new syscall for page cache state of files Nhat Pham
  2023-02-19  7:33 ` [PATCH v10 1/3] workingset: refactor LRU refault to expose refault recency check Nhat Pham
@ 2023-02-19  7:33 ` Nhat Pham
  2023-02-19  9:44   ` kernel test robot
                     ` (4 more replies)
  2023-02-19  7:33 ` [PATCH v10 3/3] selftests: Add selftests for cachestat Nhat Pham
  2 siblings, 5 replies; 15+ messages in thread
From: Nhat Pham @ 2023-02-19  7:33 UTC (permalink / raw)
  To: akpm
  Cc: hannes, linux-mm, linux-kernel, bfoster, willy, arnd, linux-api,
	kernel-team

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(-)

diff --git a/arch/x86/entry/syscalls/syscall_32.tbl b/arch/x86/entry/syscalls/syscall_32.tbl
index 320480a8db4f..01d1cc009250 100644
--- a/arch/x86/entry/syscalls/syscall_32.tbl
+++ b/arch/x86/entry/syscalls/syscall_32.tbl
@@ -455,3 +455,4 @@
 448	i386	process_mrelease	sys_process_mrelease
 449	i386	futex_waitv		sys_futex_waitv
 450	i386	set_mempolicy_home_node		sys_set_mempolicy_home_node
+451	i386	cachestat		sys_cachestat		compat_sys_cachestat
diff --git a/arch/x86/entry/syscalls/syscall_64.tbl b/arch/x86/entry/syscalls/syscall_64.tbl
index c84d12608cd2..227538b0ce80 100644
--- a/arch/x86/entry/syscalls/syscall_64.tbl
+++ b/arch/x86/entry/syscalls/syscall_64.tbl
@@ -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
diff --git a/include/linux/compat.h b/include/linux/compat.h
index 594357881b0b..1c5d385be859 100644
--- a/include/linux/compat.h
+++ b/include/linux/compat.h
@@ -816,7 +816,9 @@ asmlinkage long compat_sys_sendmsg(int fd, struct compat_msghdr __user *msg,
 asmlinkage long compat_sys_recvmsg(int fd, struct compat_msghdr __user *msg,
 				   unsigned int flags);

-/* mm/filemap.c: No generic prototype for readahead */
+/* mm/filemap.c */
+asmlinkage long compat_sys_cachestat(int fd, size_t len, compat_arg_u64(off),
+		struct cachestat __user *cstat, unsigned int flags);

 /* security/keys/keyctl.c */
 asmlinkage long compat_sys_keyctl(u32 option,
diff --git a/include/linux/fs.h b/include/linux/fs.h
index e654435f1651..83300f1491e7 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -75,6 +75,7 @@ struct fs_context;
 struct fs_parameter_spec;
 struct fileattr;
 struct iomap_ops;
+struct cachestat;

 extern void __init inode_init(void);
 extern void __init inode_init_early(void);
@@ -830,6 +831,8 @@ void filemap_invalidate_lock_two(struct address_space *mapping1,
 				 struct address_space *mapping2);
 void filemap_invalidate_unlock_two(struct address_space *mapping1,
 				   struct address_space *mapping2);
+void filemap_cachestat(struct address_space *mapping, pgoff_t first_index,
+		pgoff_t last_index, struct cachestat *cs);


 /*
diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
index a34b0f9a9972..9f384a6d7e77 100644
--- a/include/linux/syscalls.h
+++ b/include/linux/syscalls.h
@@ -72,6 +72,7 @@ struct open_how;
 struct mount_attr;
 struct landlock_ruleset_attr;
 enum landlock_rule_type;
+struct cachestat;

 #include <linux/types.h>
 #include <linux/aio_abi.h>
@@ -1056,6 +1057,8 @@ asmlinkage long sys_memfd_secret(unsigned int flags);
 asmlinkage long sys_set_mempolicy_home_node(unsigned long start, unsigned long len,
 					    unsigned long home_node,
 					    unsigned long flags);
+asmlinkage long sys_cachestat(unsigned int fd, size_t len, loff_t offset,
+		struct cachestat __user *cstat, unsigned int flags);

 /*
  * Architecture-specific system calls
diff --git a/include/uapi/asm-generic/unistd.h b/include/uapi/asm-generic/unistd.h
index 45fa180cc56a..dc5fcfee7f09 100644
--- a/include/uapi/asm-generic/unistd.h
+++ b/include/uapi/asm-generic/unistd.h
@@ -886,8 +886,11 @@ __SYSCALL(__NR_futex_waitv, sys_futex_waitv)
 #define __NR_set_mempolicy_home_node 450
 __SYSCALL(__NR_set_mempolicy_home_node, sys_set_mempolicy_home_node)

+#define __NR_cachestat 451
+__SC_COMP(__NR_cachestat, sys_cachestat, compat_sys_cachestat)
+
 #undef __NR_syscalls
-#define __NR_syscalls 451
+#define __NR_syscalls 452

 /*
  * 32 bit systems traditionally used different
diff --git a/include/uapi/linux/mman.h b/include/uapi/linux/mman.h
index f55bc680b5b0..fe03ed0b7587 100644
--- a/include/uapi/linux/mman.h
+++ b/include/uapi/linux/mman.h
@@ -4,6 +4,7 @@

 #include <asm/mman.h>
 #include <asm-generic/hugetlb_encode.h>
+#include <linux/types.h>

 #define MREMAP_MAYMOVE		1
 #define MREMAP_FIXED		2
@@ -41,4 +42,12 @@
 #define MAP_HUGE_2GB	HUGETLB_FLAG_ENCODE_2GB
 #define MAP_HUGE_16GB	HUGETLB_FLAG_ENCODE_16GB

+struct cachestat {
+	__u64 nr_cache;
+	__u64 nr_dirty;
+	__u64 nr_writeback;
+	__u64 nr_evicted;
+	__u64 nr_recently_evicted;
+};
+
 #endif /* _UAPI_LINUX_MMAN_H */
diff --git a/init/Kconfig b/init/Kconfig
index 694f7c160c9c..da96ac29af1d 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -1798,6 +1798,16 @@ config RSEQ

 	  If unsure, say Y.

+config CACHESTAT_SYSCALL
+	bool "Enable cachestat() system call" if EXPERT
+	default y
+	help
+	  Enable the cachestat system call, which queries the page cache
+	  statistics of a file (number of cached pages, dirty pages,
+	  pages marked for writeback, (recently) evicted pages).
+
+	  If unsure say Y here.
+
 config DEBUG_RSEQ
 	default n
 	bool "Enabled debugging of rseq() system call" if EXPERT
diff --git a/kernel/sys_ni.c b/kernel/sys_ni.c
index 860b2dcf3ac4..04bfb1e4d377 100644
--- a/kernel/sys_ni.c
+++ b/kernel/sys_ni.c
@@ -299,6 +299,7 @@ COND_SYSCALL(set_mempolicy);
 COND_SYSCALL(migrate_pages);
 COND_SYSCALL(move_pages);
 COND_SYSCALL(set_mempolicy_home_node);
+COND_SYSCALL(cachestat);

 COND_SYSCALL(perf_event_open);
 COND_SYSCALL(accept4);
diff --git a/mm/filemap.c b/mm/filemap.c
index 08341616ae7a..a00b5988d21c 100644
--- a/mm/filemap.c
+++ b/mm/filemap.c
@@ -22,6 +22,7 @@
 #include <linux/mm.h>
 #include <linux/swap.h>
 #include <linux/swapops.h>
+#include <linux/syscalls.h>
 #include <linux/mman.h>
 #include <linux/pagemap.h>
 #include <linux/file.h>
@@ -55,6 +56,9 @@
 #include <linux/buffer_head.h> /* for try_to_free_buffers */

 #include <asm/mman.h>
+#include <uapi/linux/mman.h>
+
+#include "swap.h"

 /*
  * Shared mappings implemented 30.11.1994. It's not fully working yet,
@@ -3949,3 +3953,157 @@ bool filemap_release_folio(struct folio *folio, gfp_t gfp)
 	return try_to_free_buffers(folio);
 }
 EXPORT_SYMBOL(filemap_release_folio);
+
+/**
+ * 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.
+ */
+void filemap_cachestat(struct address_space *mapping, pgoff_t first_index,
+		pgoff_t last_index, struct cachestat *cs)
+{
+	XA_STATE(xas, &mapping->i_pages, first_index);
+	struct folio *folio;
+
+	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 */
+		if (folio_first_index < first_index)
+			nr_pages -= first_index - folio_first_index;
+
+		if (folio_last_index > last_index)
+			nr_pages -= folio_last_index - last_index;
+
+		if (xa_is_value(folio)) {
+			/* page is evicted */
+			void *shadow = (void *)folio;
+			bool workingset; /* not used */
+
+			cs->nr_evicted += nr_pages;
+
+#ifdef CONFIG_SWAP /* implies CONFIG_MMU */
+			if (shmem_mapping(mapping)) {
+				/* shmem file - in swap cache */
+				swp_entry_t swp = radix_to_swp_entry(folio);
+
+				shadow = get_shadow_from_swap_cache(swp);
+			}
+#endif
+			if (workingset_test_recent(shadow, true, &workingset))
+				cs->nr_recently_evicted += nr_pages;
+
+			goto resched;
+		}
+
+		/* page is in cache */
+		cs->nr_cache += nr_pages;
+
+		if (folio_test_dirty(folio))
+			cs->nr_dirty += nr_pages;
+
+		if (folio_test_writeback(folio))
+			cs->nr_writeback += nr_pages;
+
+resched:
+		if (need_resched()) {
+			xas_pause(&xas);
+			cond_resched_rcu();
+		}
+	}
+	rcu_read_unlock();
+}
+EXPORT_SYMBOL(filemap_cachestat);
+
+#ifdef CONFIG_CACHESTAT_SYSCALL
+long ksys_cachestat(unsigned int fd, size_t len, loff_t off,
+		struct cachestat __user *cstat, unsigned int flags)
+{
+	struct fd f = fdget(fd);
+	struct address_space *mapping;
+	struct cachestat cs;
+	pgoff_t first_index = off >> PAGE_SHIFT;
+	pgoff_t last_index =
+		len == 0 ? ULONG_MAX : (off + len - 1) >> PAGE_SHIFT;
+
+	if (!f.file)
+		return -EBADF;
+
+	if (off < 0 || flags != 0) {
+		fdput(f);
+		return -EINVAL;
+	}
+
+	memset(&cs, 0, sizeof(struct cachestat));
+	mapping = f.file->f_mapping;
+	filemap_cachestat(mapping, first_index, last_index, &cs);
+	fdput(f);
+
+	if (copy_to_user(cstat, &cs, sizeof(struct cachestat)))
+		return -EFAULT;
+
+	return 0;
+}
+
+/*
+ * The cachestat(5) system call.
+ *
+ * cachestat() returns the page cache statistics of a file in the
+ * bytes range specified by `off` and `len`: number of cached pages,
+ * number of dirty pages, number of pages marked for writeback,
+ * number of evicted pages, and number of recently evicted pages.
+ *
+ * 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.
+ *
+ * `off` and `len` 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).
+ *
+ * Because the status of a page can change after cachestat() checks it
+ * but before it returns to the application, the returned values may
+ * contain stale information.
+ *
+ * return values:
+ *  zero    - success
+ *  -EFAULT - cstat points to an illegal address
+ *  -EINVAL - invalid arguments
+ *  -EBADF	- invalid file descriptor
+ */
+SYSCALL_DEFINE5(cachestat, unsigned int, fd, size_t, len, loff_t, off,
+		struct cachestat __user *, cstat, unsigned int, flags)
+{
+	return ksys_cachestat(fd, len, off, cstat, flags);
+}
+
+#ifdef CONFIG_COMPAT
+COMPAT_SYSCALL_DEFINE6(cachestat, unsigned int, fd, size_t, len,
+		compat_arg_u64_dual(off), struct cachestat __user *, cstat,
+		unsigned int, flags)
+{
+	return ksys_cachestat(fd, len, compat_arg_u64_glue(off), cstat, flags);
+}
+#endif
+#endif /* CONFIG_CACHESTAT_SYSCALL */
--
2.39.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v10 3/3] selftests: Add selftests for cachestat
  2023-02-19  7:33 [PATCH v10 0/3] cachestat: a new syscall for page cache state of files Nhat Pham
  2023-02-19  7:33 ` [PATCH v10 1/3] workingset: refactor LRU refault to expose refault recency check Nhat Pham
  2023-02-19  7:33 ` [PATCH v10 2/3] cachestat: implement cachestat syscall Nhat Pham
@ 2023-02-19  7:33 ` Nhat Pham
  2 siblings, 0 replies; 15+ messages in thread
From: Nhat Pham @ 2023-02-19  7:33 UTC (permalink / raw)
  To: akpm
  Cc: hannes, linux-mm, linux-kernel, bfoster, willy, arnd, linux-api,
	kernel-team

Test cachestat on a newly created file, /dev/ files, and /proc/ files.
Also test on a shmem file (which can also be tested with huge pages
since tmpfs supports huge pages).

Signed-off-by: Nhat Pham <nphamcs@gmail.com>
---
 MAINTAINERS                                   |   7 +
 tools/testing/selftests/Makefile              |   1 +
 tools/testing/selftests/cachestat/.gitignore  |   2 +
 tools/testing/selftests/cachestat/Makefile    |   8 +
 .../selftests/cachestat/test_cachestat.c      | 256 ++++++++++++++++++
 5 files changed, 274 insertions(+)
 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

diff --git a/MAINTAINERS b/MAINTAINERS
index a198da986146..792a866353ec 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4552,6 +4552,13 @@ S:	Supported
 F:	Documentation/filesystems/caching/cachefiles.rst
 F:	fs/cachefiles/

+CACHESTAT: PAGE CACHE STATS FOR A FILE
+M:	Nhat Pham <nphamcs@gmail.com>
+M:	Johannes Weiner <hannes@cmpxchg.org>
+L:	linux-mm@kvack.org
+S:	Maintained
+F:	tools/testing/selftests/cachestat/test_cachestat.c
+
 CADENCE MIPI-CSI2 BRIDGES
 M:	Maxime Ripard <mripard@kernel.org>
 L:	linux-media@vger.kernel.org
diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
index 0464b2c6c1e4..3cad0b38c5c2 100644
--- a/tools/testing/selftests/Makefile
+++ b/tools/testing/selftests/Makefile
@@ -4,6 +4,7 @@ TARGETS += amd-pstate
 TARGETS += arm64
 TARGETS += bpf
 TARGETS += breakpoints
+TARGETS += cachestat
 TARGETS += capabilities
 TARGETS += cgroup
 TARGETS += clone3
diff --git a/tools/testing/selftests/cachestat/.gitignore b/tools/testing/selftests/cachestat/.gitignore
new file mode 100644
index 000000000000..d6c30b43a4bb
--- /dev/null
+++ b/tools/testing/selftests/cachestat/.gitignore
@@ -0,0 +1,2 @@
+# SPDX-License-Identifier: GPL-2.0-only
+test_cachestat
diff --git a/tools/testing/selftests/cachestat/Makefile b/tools/testing/selftests/cachestat/Makefile
new file mode 100644
index 000000000000..fca73aaa7d14
--- /dev/null
+++ b/tools/testing/selftests/cachestat/Makefile
@@ -0,0 +1,8 @@
+# SPDX-License-Identifier: GPL-2.0
+TEST_GEN_PROGS := test_cachestat
+
+CFLAGS += $(KHDR_INCLUDES)
+CFLAGS += -Wall
+CFLAGS += -lrt
+
+include ../lib.mk
diff --git a/tools/testing/selftests/cachestat/test_cachestat.c b/tools/testing/selftests/cachestat/test_cachestat.c
new file mode 100644
index 000000000000..5d0fe5ae62f1
--- /dev/null
+++ b/tools/testing/selftests/cachestat/test_cachestat.c
@@ -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"
+
+static const char * const dev_files[] = {
+	"/dev/zero", "/dev/null", "/dev/urandom",
+	"/proc/version", "/proc"
+};
+static const int cachestat_nr = 451;
+
+void print_cachestat(struct cachestat *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);
+}
+
+bool write_exactly(int fd, size_t filesize)
+{
+	char data[filesize];
+	bool ret = true;
+	int random_fd = open("/dev/urandom", O_RDONLY);
+
+	if (random_fd < 0) {
+		ksft_print_msg("Unable to access urandom.\n");
+		ret = false;
+		goto out;
+	} else {
+		int remained = filesize;
+		char *cursor = data;
+
+		while (remained) {
+			ssize_t read_len = read(random_fd, cursor, remained);
+
+			if (read_len <= 0) {
+				ksft_print_msg("Unable to read from urandom.\n");
+				ret = false;
+				goto close_random_fd;
+			}
+
+			remained -= read_len;
+			cursor += read_len;
+		}
+
+		/* write random data to fd */
+		remained = filesize;
+		cursor = data;
+		while (remained) {
+			ssize_t write_len = write(fd, cursor, remained);
+
+			if (write_len <= 0) {
+				ksft_print_msg("Unable write random data to file.\n");
+				ret = false;
+				goto close_random_fd;
+			}
+
+			remained -= write_len;
+			cursor += write_len;
+		}
+	}
+
+close_random_fd:
+	close(random_fd);
+out:
+	return ret;
+}
+
+/*
+ * Open/create the file at filename, (optionally) write random data to it
+ * (exactly num_pages), then test the cachestat syscall on this file.
+ *
+ * If test_fsync == true, fsync the file, then check the number of dirty
+ * pages.
+ */
+bool test_cachestat(const char *filename, bool write_random, bool create,
+		bool test_fsync, unsigned long num_pages, int open_flags,
+		mode_t open_mode)
+{
+	size_t PS = sysconf(_SC_PAGESIZE);
+	int filesize = num_pages * PS;
+	bool ret = true;
+	long syscall_ret;
+	struct cachestat cs;
+
+	int fd = open(filename, open_flags, open_mode);
+
+	if (fd == -1) {
+		ksft_print_msg("Unable to create/open file.\n");
+		goto out;
+	} 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;
+			goto out1;
+		}
+	}
+
+	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;
+		goto out1;
+
+	} 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;
+				goto out1;
+			}
+		}
+	}
+
+out1:
+	close(fd);
+
+	if (create)
+		remove(filename);
+out:
+	return ret;
+}
+
+bool test_cachestat_shmem(void)
+{
+	size_t PS = sysconf(_SC_PAGESIZE);
+	size_t filesize = PS * 512 * 2; /* 2 2MB huge pages */
+	int syscall_ret;
+	off_t off = PS;
+	size_t compute_len = PS * 512;
+	char *filename = "tmpshmcstat";
+	struct cachestat cs;
+	bool ret = true;
+	unsigned long num_pages = compute_len / PS;
+	int fd = shm_open(filename, O_CREAT | O_RDWR, 0600);
+
+	if (fd < 0) {
+		ksft_print_msg("Unable to create shmem file.\n");
+		ret = false;
+		goto out;
+	}
+
+	if (ftruncate(fd, filesize)) {
+		ksft_print_msg("Unable to trucate shmem file.\n");
+		ret = false;
+		goto close_fd;
+	}
+
+	if (!write_exactly(fd, filesize)) {
+		ksft_print_msg("Unable to write to shmem file.\n");
+		ret = false;
+		goto close_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;
+		goto close_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:
+	return ret;
+}
+
+int main(void)
+{
+	int ret = 0;
+
+	for (int i = 0; i < 5; i++) {
+		const char *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;
+	}
+
+	return ret;
+}
--
2.39.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH v10 2/3] cachestat: implement cachestat syscall
  2023-02-19  7:33 ` [PATCH v10 2/3] cachestat: implement cachestat syscall Nhat Pham
@ 2023-02-19  9:44   ` kernel test robot
  2023-02-19  9:45   ` kernel test robot
                     ` (3 subsequent siblings)
  4 siblings, 0 replies; 15+ messages in thread
From: kernel test robot @ 2023-02-19  9:44 UTC (permalink / raw)
  To: Nhat Pham, akpm
  Cc: llvm, oe-kbuild-all, hannes, linux-mm, linux-kernel, bfoster,
	willy, arnd, linux-api, kernel-team

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 <lkp@intel.com>
| 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);
                                                         ~~~~~~~~~~ ^
>> 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,
  4036			struct cachestat __user *cstat, unsigned int flags)
  4037	{
  4038		struct fd f = fdget(fd);
  4039		struct address_space *mapping;
  4040		struct cachestat cs;
  4041		pgoff_t first_index = off >> PAGE_SHIFT;
  4042		pgoff_t last_index =
  4043			len == 0 ? ULONG_MAX : (off + len - 1) >> PAGE_SHIFT;
  4044	
  4045		if (!f.file)
  4046			return -EBADF;
  4047	
  4048		if (off < 0 || flags != 0) {
  4049			fdput(f);
  4050			return -EINVAL;
  4051		}
  4052	
  4053		memset(&cs, 0, sizeof(struct cachestat));
  4054		mapping = f.file->f_mapping;
  4055		filemap_cachestat(mapping, first_index, last_index, &cs);
  4056		fdput(f);
  4057	
  4058		if (copy_to_user(cstat, &cs, sizeof(struct cachestat)))
  4059			return -EFAULT;
  4060	
  4061		return 0;
  4062	}
  4063	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v10 2/3] cachestat: implement cachestat syscall
  2023-02-19  7:33 ` [PATCH v10 2/3] cachestat: implement cachestat syscall Nhat Pham
  2023-02-19  9:44   ` kernel test robot
  2023-02-19  9:45   ` kernel test robot
@ 2023-02-19  9:45   ` kernel test robot
  2023-02-19 10:36   ` kernel test robot
  2023-02-19 12:21   ` Matthew Wilcox
  4 siblings, 0 replies; 15+ messages in thread
From: kernel test robot @ 2023-02-19  9:45 UTC (permalink / raw)
  To: Nhat Pham, akpm
  Cc: oe-kbuild-all, hannes, linux-mm, linux-kernel, bfoster, willy,
	arnd, linux-api, kernel-team

Hi Nhat,

Thank you for the patch! Yet something to improve:

[auto build test ERROR 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: riscv-buildonly-randconfig-r005-20230219 (https://download.01.org/0day-ci/archive/20230219/202302191700.Pg6Yy5Xp-lkp@intel.com/config)
compiler: riscv64-linux-gcc (GCC) 12.1.0
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=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=riscv olddefconfig
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=riscv SHELL=/bin/bash

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202302191700.Pg6Yy5Xp-lkp@intel.com/

All errors (new ones prefixed by >>):

   mm/filemap.c:4035:6: warning: no previous prototype for 'ksys_cachestat' [-Wmissing-prototypes]
    4035 | long ksys_cachestat(unsigned int fd, size_t len, loff_t off,
         |      ^~~~~~~~~~~~~~
   In file included from arch/riscv/include/asm/elf.h:12,
                    from include/linux/elf.h:6,
                    from include/linux/module.h:19,
                    from include/linux/device/driver.h:21,
                    from include/linux/device.h:32,
                    from include/linux/blk_types.h:11,
                    from include/linux/writeback.h:13,
                    from include/linux/memcontrol.h:22,
                    from include/linux/swap.h:9,
                    from mm/filemap.c:23:
>> include/linux/compat.h:75:25: error: conflicting types for 'compat_sys_cachestat'; have 'long int(unsigned int,  size_t,  u32,  u32,  struct cachestat *, unsigned int)' {aka 'long int(unsigned int,  long unsigned int,  unsigned int,  unsigned int,  struct cachestat *, unsigned int)'}
      75 |         asmlinkage long compat_sys##name(__MAP(x,__SC_DECL,__VA_ARGS__))        \
         |                         ^~~~~~~~~~
   include/linux/compat.h:63:9: note: in expansion of macro 'COMPAT_SYSCALL_DEFINEx'
      63 |         COMPAT_SYSCALL_DEFINEx(6, _##name, __VA_ARGS__)
         |         ^~~~~~~~~~~~~~~~~~~~~~
   mm/filemap.c:4102:1: note: in expansion of macro 'COMPAT_SYSCALL_DEFINE6'
    4102 | COMPAT_SYSCALL_DEFINE6(cachestat, unsigned int, fd, size_t, len,
         | ^~~~~~~~~~~~~~~~~~~~~~
   include/linux/compat.h:820:17: note: previous declaration of 'compat_sys_cachestat' with type 'long int(int,  size_t,  u32,  u32,  struct cachestat *, unsigned int)' {aka 'long int(int,  long unsigned int,  unsigned int,  unsigned int,  struct cachestat *, unsigned int)'}
     820 | asmlinkage long compat_sys_cachestat(int fd, size_t len, compat_arg_u64(off),
         |                 ^~~~~~~~~~~~~~~~~~~~


vim +75 include/linux/compat.h

217f4433fc2fe7 Heiko Carstens    2014-02-26  51  
468366138850f2 Al Viro           2012-11-23  52  #define COMPAT_SYSCALL_DEFINE1(name, ...) \
468366138850f2 Al Viro           2012-11-23  53          COMPAT_SYSCALL_DEFINEx(1, _##name, __VA_ARGS__)
468366138850f2 Al Viro           2012-11-23  54  #define COMPAT_SYSCALL_DEFINE2(name, ...) \
468366138850f2 Al Viro           2012-11-23  55  	COMPAT_SYSCALL_DEFINEx(2, _##name, __VA_ARGS__)
468366138850f2 Al Viro           2012-11-23  56  #define COMPAT_SYSCALL_DEFINE3(name, ...) \
468366138850f2 Al Viro           2012-11-23  57  	COMPAT_SYSCALL_DEFINEx(3, _##name, __VA_ARGS__)
468366138850f2 Al Viro           2012-11-23  58  #define COMPAT_SYSCALL_DEFINE4(name, ...) \
468366138850f2 Al Viro           2012-11-23  59  	COMPAT_SYSCALL_DEFINEx(4, _##name, __VA_ARGS__)
468366138850f2 Al Viro           2012-11-23  60  #define COMPAT_SYSCALL_DEFINE5(name, ...) \
468366138850f2 Al Viro           2012-11-23  61  	COMPAT_SYSCALL_DEFINEx(5, _##name, __VA_ARGS__)
468366138850f2 Al Viro           2012-11-23  62  #define COMPAT_SYSCALL_DEFINE6(name, ...) \
468366138850f2 Al Viro           2012-11-23  63  	COMPAT_SYSCALL_DEFINEx(6, _##name, __VA_ARGS__)
468366138850f2 Al Viro           2012-11-23  64  
5ac9efa3c50d7c Dominik Brodowski 2018-04-09  65  /*
5ac9efa3c50d7c Dominik Brodowski 2018-04-09  66   * The asmlinkage stub is aliased to a function named __se_compat_sys_*() which
5ac9efa3c50d7c Dominik Brodowski 2018-04-09  67   * sign-extends 32-bit ints to longs whenever needed. The actual work is
5ac9efa3c50d7c Dominik Brodowski 2018-04-09  68   * done within __do_compat_sys_*().
5ac9efa3c50d7c Dominik Brodowski 2018-04-09  69   */
7303e30ec1d8fb Dominik Brodowski 2018-04-05  70  #ifndef COMPAT_SYSCALL_DEFINEx
468366138850f2 Al Viro           2012-11-23  71  #define COMPAT_SYSCALL_DEFINEx(x, name, ...)					\
bee20031772af3 Arnd Bergmann     2018-06-19  72  	__diag_push();								\
bee20031772af3 Arnd Bergmann     2018-06-19  73  	__diag_ignore(GCC, 8, "-Wattribute-alias",				\
bee20031772af3 Arnd Bergmann     2018-06-19  74  		      "Type aliasing is used to sanitize syscall arguments");\
83460ec8dcac14 Andi Kleen        2013-11-12 @75  	asmlinkage long compat_sys##name(__MAP(x,__SC_DECL,__VA_ARGS__))	\
5ac9efa3c50d7c Dominik Brodowski 2018-04-09  76  		__attribute__((alias(__stringify(__se_compat_sys##name))));	\
c9a211951c7c79 Howard McLauchlan 2018-03-21  77  	ALLOW_ERROR_INJECTION(compat_sys##name, ERRNO);				\
5ac9efa3c50d7c Dominik Brodowski 2018-04-09  78  	static inline long __do_compat_sys##name(__MAP(x,__SC_DECL,__VA_ARGS__));\
5ac9efa3c50d7c Dominik Brodowski 2018-04-09  79  	asmlinkage long __se_compat_sys##name(__MAP(x,__SC_LONG,__VA_ARGS__));	\
5ac9efa3c50d7c Dominik Brodowski 2018-04-09  80  	asmlinkage long __se_compat_sys##name(__MAP(x,__SC_LONG,__VA_ARGS__))	\
468366138850f2 Al Viro           2012-11-23  81  	{									\
bee20031772af3 Arnd Bergmann     2018-06-19  82  		long ret = __do_compat_sys##name(__MAP(x,__SC_DELOUSE,__VA_ARGS__));\
bee20031772af3 Arnd Bergmann     2018-06-19  83  		__MAP(x,__SC_TEST,__VA_ARGS__);					\
bee20031772af3 Arnd Bergmann     2018-06-19  84  		return ret;							\
468366138850f2 Al Viro           2012-11-23  85  	}									\
bee20031772af3 Arnd Bergmann     2018-06-19  86  	__diag_pop();								\
5ac9efa3c50d7c Dominik Brodowski 2018-04-09  87  	static inline long __do_compat_sys##name(__MAP(x,__SC_DECL,__VA_ARGS__))
7303e30ec1d8fb Dominik Brodowski 2018-04-05  88  #endif /* COMPAT_SYSCALL_DEFINEx */
468366138850f2 Al Viro           2012-11-23  89  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v10 2/3] cachestat: implement cachestat syscall
  2023-02-19  7:33 ` [PATCH v10 2/3] cachestat: implement cachestat syscall Nhat Pham
  2023-02-19  9:44   ` kernel test robot
@ 2023-02-19  9:45   ` kernel test robot
  2023-02-19  9:45   ` kernel test robot
                     ` (2 subsequent siblings)
  4 siblings, 0 replies; 15+ messages in thread
From: kernel test robot @ 2023-02-19  9:45 UTC (permalink / raw)
  To: Nhat Pham, akpm
  Cc: oe-kbuild-all, hannes, linux-mm, linux-kernel, bfoster, willy,
	arnd, linux-api, kernel-team

Hi Nhat,

Thank you for the patch! Yet something to improve:

[auto build test ERROR 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: sparc-randconfig-r033-20230219 (https://download.01.org/0day-ci/archive/20230219/202302191708.tLz0L0ZE-lkp@intel.com/config)
compiler: sparc64-linux-gcc (GCC) 12.1.0
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=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=sparc olddefconfig
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=sparc SHELL=/bin/bash

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202302191708.tLz0L0ZE-lkp@intel.com/

All errors (new ones prefixed by >>):

   mm/filemap.c:4035:6: warning: no previous prototype for 'ksys_cachestat' [-Wmissing-prototypes]
    4035 | long ksys_cachestat(unsigned int fd, size_t len, loff_t off,
         |      ^~~~~~~~~~~~~~
>> mm/filemap.c:4102:34: error: expected ')' before 'unsigned'
    4102 | COMPAT_SYSCALL_DEFINE6(cachestat, unsigned int, fd, size_t, len,
         |                                  ^~~~~~~~~
         |                                  )


vim +4102 mm/filemap.c

  4100	
  4101	#ifdef CONFIG_COMPAT
> 4102	COMPAT_SYSCALL_DEFINE6(cachestat, unsigned int, fd, size_t, len,

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v10 2/3] cachestat: implement cachestat syscall
  2023-02-19  7:33 ` [PATCH v10 2/3] cachestat: implement cachestat syscall Nhat Pham
                     ` (2 preceding siblings ...)
  2023-02-19  9:45   ` kernel test robot
@ 2023-02-19 10:36   ` kernel test robot
  2023-02-19 12:21   ` Matthew Wilcox
  4 siblings, 0 replies; 15+ messages in thread
From: kernel test robot @ 2023-02-19 10:36 UTC (permalink / raw)
  To: Nhat Pham, akpm
  Cc: oe-kbuild-all, hannes, linux-mm, linux-kernel, bfoster, willy,
	arnd, linux-api, kernel-team

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: i386-randconfig-a001 (https://download.01.org/0day-ci/archive/20230219/202302191804.5m2GB1PU-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.3.0-8) 11.3.0
reproduce (this is a W=1 build):
        # 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
        make W=1 O=build_dir ARCH=i386 olddefconfig
        make W=1 O=build_dir ARCH=i386 SHELL=/bin/bash

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202302191804.5m2GB1PU-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> mm/filemap.c:4035:6: warning: no previous prototype for 'ksys_cachestat' [-Wmissing-prototypes]
    4035 | long ksys_cachestat(unsigned int fd, size_t len, loff_t off,
         |      ^~~~~~~~~~~~~~


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,
  4036			struct cachestat __user *cstat, unsigned int flags)
  4037	{
  4038		struct fd f = fdget(fd);
  4039		struct address_space *mapping;
  4040		struct cachestat cs;
  4041		pgoff_t first_index = off >> PAGE_SHIFT;
  4042		pgoff_t last_index =
  4043			len == 0 ? ULONG_MAX : (off + len - 1) >> PAGE_SHIFT;
  4044	
  4045		if (!f.file)
  4046			return -EBADF;
  4047	
  4048		if (off < 0 || flags != 0) {
  4049			fdput(f);
  4050			return -EINVAL;
  4051		}
  4052	
  4053		memset(&cs, 0, sizeof(struct cachestat));
  4054		mapping = f.file->f_mapping;
  4055		filemap_cachestat(mapping, first_index, last_index, &cs);
  4056		fdput(f);
  4057	
  4058		if (copy_to_user(cstat, &cs, sizeof(struct cachestat)))
  4059			return -EFAULT;
  4060	
  4061		return 0;
  4062	}
  4063	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v10 1/3] workingset: refactor LRU refault to expose refault recency check
  2023-02-19  7:33 ` [PATCH v10 1/3] workingset: refactor LRU refault to expose refault recency check Nhat Pham
@ 2023-02-19 12:05   ` Matthew Wilcox
  2023-02-21  8:49     ` Nhat Pham
  0 siblings, 1 reply; 15+ messages in thread
From: Matthew Wilcox @ 2023-02-19 12:05 UTC (permalink / raw)
  To: Nhat Pham
  Cc: akpm, hannes, linux-mm, linux-kernel, bfoster, arnd, linux-api,
	kernel-team

On Sat, Feb 18, 2023 at 11:33:16PM -0800, Nhat Pham wrote:
> +++ b/mm/workingset.c
> @@ -244,6 +244,30 @@ static void *lru_gen_eviction(struct folio *folio)
>  	return pack_shadow(mem_cgroup_id(memcg), pgdat, token, refs);
>  }
> 
> +/*
> + * Test if the folio is recently evicted.
> + *
> + * As a side effect, also populates the references with
> + * values unpacked from the shadow of the evicted folio.
> + */

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.

> +static bool lru_gen_test_recent(void *shadow, bool file, int *memcgid,
> +		struct pglist_data **pgdat, unsigned long *token, bool *workingset)
> +{
> +	struct mem_cgroup *eviction_memcg;
> +	struct lruvec *lruvec;
> +	struct lru_gen_struct *lrugen;
> +	unsigned long min_seq;
> +
> +	unpack_shadow(shadow, memcgid, pgdat, token, workingset);
> +	eviction_memcg = mem_cgroup_from_id(*memcgid);
> +
> +	lruvec = mem_cgroup_lruvec(eviction_memcg, *pgdat);
> +	lrugen = &lruvec->lrugen;
> +
> +	min_seq = READ_ONCE(lrugen->min_seq[file]);
> +	return (*token >> LRU_REFS_WIDTH) == (min_seq & (EVICTION_MASK >> LRU_REFS_WIDTH));
> +}

[...]

> +/*
> + * 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_.


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v10 2/3] cachestat: implement cachestat syscall
  2023-02-19  7:33 ` [PATCH v10 2/3] cachestat: implement cachestat syscall Nhat Pham
                     ` (3 preceding siblings ...)
  2023-02-19 10:36   ` kernel test robot
@ 2023-02-19 12:21   ` Matthew Wilcox
  2023-03-03  6:55     ` Nhat Pham
  2023-03-05 10:32     ` Nhat Pham
  4 siblings, 2 replies; 15+ messages in thread
From: Matthew Wilcox @ 2023-02-19 12:21 UTC (permalink / raw)
  To: Nhat Pham
  Cc: akpm, hannes, linux-mm, linux-kernel, bfoster, arnd, linux-api,
	kernel-team

On Sat, Feb 18, 2023 at 11:33:17PM -0800, Nhat Pham wrote:
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index e654435f1651..83300f1491e7 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -75,6 +75,7 @@ struct fs_context;
>  struct fs_parameter_spec;
>  struct fileattr;
>  struct iomap_ops;
> +struct cachestat;
> 
>  extern void __init inode_init(void);
>  extern void __init inode_init_early(void);
> @@ -830,6 +831,8 @@ void filemap_invalidate_lock_two(struct address_space *mapping1,
>  				 struct address_space *mapping2);
>  void filemap_invalidate_unlock_two(struct address_space *mapping1,
>  				   struct address_space *mapping2);
> +void filemap_cachestat(struct address_space *mapping, pgoff_t first_index,
> +		pgoff_t last_index, struct cachestat *cs);

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?

> @@ -55,6 +56,9 @@
>  #include <linux/buffer_head.h> /* for try_to_free_buffers */
> 
>  #include <asm/mman.h>
> +#include <uapi/linux/mman.h>

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)

> +/**
> + * 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?

> +	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/


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v10 1/3] workingset: refactor LRU refault to expose refault recency check
  2023-02-19 12:05   ` Matthew Wilcox
@ 2023-02-21  8:49     ` Nhat Pham
  0 siblings, 0 replies; 15+ messages in thread
From: Nhat Pham @ 2023-02-21  8:49 UTC (permalink / raw)
  To: Matthew Wilcox
  Cc: akpm, hannes, linux-mm, linux-kernel, bfoster, arnd, linux-api,
	kernel-team

On Sun, Feb 19, 2023 at 4:05 AM Matthew Wilcox <willy@infradead.org> wrote:
>
> On Sat, Feb 18, 2023 at 11:33:16PM -0800, Nhat Pham wrote:
> > +++ b/mm/workingset.c
> > @@ -244,6 +244,30 @@ static void *lru_gen_eviction(struct folio *folio)
> >       return pack_shadow(mem_cgroup_id(memcg), pgdat, token, refs);
> >  }
> >
> > +/*
> > + * Test if the folio is recently evicted.
> > + *
> > + * As a side effect, also populates the references with
> > + * values unpacked from the shadow of the evicted folio.
> > + */
>
> 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!

>
> > +static bool lru_gen_test_recent(void *shadow, bool file, int *memcgid,
> > +             struct pglist_data **pgdat, unsigned long *token, bool *workingset)
> > +{
> > +     struct mem_cgroup *eviction_memcg;
> > +     struct lruvec *lruvec;
> > +     struct lru_gen_struct *lrugen;
> > +     unsigned long min_seq;
> > +
> > +     unpack_shadow(shadow, memcgid, pgdat, token, workingset);
> > +     eviction_memcg = mem_cgroup_from_id(*memcgid);
> > +
> > +     lruvec = mem_cgroup_lruvec(eviction_memcg, *pgdat);
> > +     lrugen = &lruvec->lrugen;
> > +
> > +     min_seq = READ_ONCE(lrugen->min_seq[file]);
> > +     return (*token >> LRU_REFS_WIDTH) == (min_seq & (EVICTION_MASK >> LRU_REFS_WIDTH));
> > +}
>
> [...]
>
> > +/*
> > + * 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'll fix this! Thanks for pointing it out.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v10 2/3] cachestat: implement cachestat syscall
  2023-02-19 12:21   ` Matthew Wilcox
@ 2023-03-03  6:55     ` Nhat Pham
  2023-03-03  7:03       ` Matthew Wilcox
  2023-03-05 10:32     ` Nhat Pham
  1 sibling, 1 reply; 15+ messages in thread
From: Nhat Pham @ 2023-03-03  6:55 UTC (permalink / raw)
  To: Matthew Wilcox
  Cc: akpm, hannes, linux-mm, linux-kernel, bfoster, arnd, linux-api,
	kernel-team

On Sun, Feb 19, 2023 at 4:21 AM Matthew Wilcox <willy@infradead.org> wrote:
>
> On Sat, Feb 18, 2023 at 11:33:17PM -0800, Nhat Pham wrote:
> > diff --git a/include/linux/fs.h b/include/linux/fs.h
> > index e654435f1651..83300f1491e7 100644
> > --- a/include/linux/fs.h
> > +++ b/include/linux/fs.h
> > @@ -75,6 +75,7 @@ struct fs_context;
> >  struct fs_parameter_spec;
> >  struct fileattr;
> >  struct iomap_ops;
> > +struct cachestat;
> >
> >  extern void __init inode_init(void);
> >  extern void __init inode_init_early(void);
> > @@ -830,6 +831,8 @@ void filemap_invalidate_lock_two(struct address_space *mapping1,
> >                                struct address_space *mapping2);
> >  void filemap_invalidate_unlock_two(struct address_space *mapping1,
> >                                  struct address_space *mapping2);
> > +void filemap_cachestat(struct address_space *mapping, pgoff_t first_index,
> > +             pgoff_t last_index, struct cachestat *cs);
>
> 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?
>
> > @@ -55,6 +56,9 @@
> >  #include <linux/buffer_head.h> /* for try_to_free_buffers */
> >
> >  #include <asm/mman.h>
> > +#include <uapi/linux/mman.h>
>
> 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)
>
> > +/**
> > + * 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!


>
> > +     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/
>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v10 2/3] cachestat: implement cachestat syscall
  2023-03-03  6:55     ` Nhat Pham
@ 2023-03-03  7:03       ` Matthew Wilcox
  2023-03-05 10:24         ` Nhat Pham
  0 siblings, 1 reply; 15+ messages in thread
From: Matthew Wilcox @ 2023-03-03  7:03 UTC (permalink / raw)
  To: Nhat Pham
  Cc: akpm, hannes, linux-mm, linux-kernel, bfoster, arnd, linux-api,
	kernel-team

On Thu, Mar 02, 2023 at 10:55:48PM -0800, Nhat Pham wrote:
> On Sun, Feb 19, 2023 at 4:21 AM Matthew Wilcox <willy@infradead.org> wrote:
> > > +/**
> > > + * 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.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v10 2/3] cachestat: implement cachestat syscall
  2023-03-03  7:03       ` Matthew Wilcox
@ 2023-03-05 10:24         ` Nhat Pham
  0 siblings, 0 replies; 15+ messages in thread
From: Nhat Pham @ 2023-03-05 10:24 UTC (permalink / raw)
  To: Matthew Wilcox
  Cc: akpm, hannes, linux-mm, linux-kernel, bfoster, arnd, linux-api,
	kernel-team

On Thu, Mar 2, 2023 at 11:03 PM Matthew Wilcox <willy@infradead.org> wrote:
>
> On Thu, Mar 02, 2023 at 10:55:48PM -0800, Nhat Pham wrote:
> > On Sun, Feb 19, 2023 at 4:21 AM Matthew Wilcox <willy@infradead.org> wrote:
> > > > +/**
> > > > + * 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.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v10 2/3] cachestat: implement cachestat syscall
  2023-02-19 12:21   ` Matthew Wilcox
  2023-03-03  6:55     ` Nhat Pham
@ 2023-03-05 10:32     ` Nhat Pham
  1 sibling, 0 replies; 15+ messages in thread
From: Nhat Pham @ 2023-03-05 10:32 UTC (permalink / raw)
  To: Matthew Wilcox
  Cc: akpm, hannes, linux-mm, linux-kernel, bfoster, arnd, linux-api,
	kernel-team

On Sun, Feb 19, 2023 at 4:21 AM Matthew Wilcox <willy@infradead.org> wrote:
>
> On Sat, Feb 18, 2023 at 11:33:17PM -0800, Nhat Pham wrote:
> > diff --git a/include/linux/fs.h b/include/linux/fs.h
> > index e654435f1651..83300f1491e7 100644
> > --- a/include/linux/fs.h
> > +++ b/include/linux/fs.h
> > @@ -75,6 +75,7 @@ struct fs_context;
> >  struct fs_parameter_spec;
> >  struct fileattr;
> >  struct iomap_ops;
> > +struct cachestat;
> >
> >  extern void __init inode_init(void);
> >  extern void __init inode_init_early(void);
> > @@ -830,6 +831,8 @@ void filemap_invalidate_lock_two(struct address_space *mapping1,
> >                                struct address_space *mapping2);
> >  void filemap_invalidate_unlock_two(struct address_space *mapping1,
> >                                  struct address_space *mapping2);
> > +void filemap_cachestat(struct address_space *mapping, pgoff_t first_index,
> > +             pgoff_t last_index, struct cachestat *cs);
>
> 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.

>
> > @@ -55,6 +56,9 @@
> >  #include <linux/buffer_head.h> /* for try_to_free_buffers */
> >
> >  #include <asm/mman.h>
> > +#include <uapi/linux/mman.h>
>
> 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)
>
> > +/**
> > + * 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?
>
> > +     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!

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2023-03-05 10:32 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-02-19  7:33 [PATCH v10 0/3] cachestat: a new syscall for page cache state of files Nhat Pham
2023-02-19  7:33 ` [PATCH v10 1/3] workingset: refactor LRU refault to expose refault recency check Nhat Pham
2023-02-19 12:05   ` Matthew Wilcox
2023-02-21  8:49     ` Nhat Pham
2023-02-19  7:33 ` [PATCH v10 2/3] cachestat: implement cachestat syscall Nhat Pham
2023-02-19  9:44   ` kernel test robot
2023-02-19  9:45   ` kernel test robot
2023-02-19  9:45   ` kernel test robot
2023-02-19 10:36   ` kernel test robot
2023-02-19 12:21   ` Matthew Wilcox
2023-03-03  6:55     ` Nhat Pham
2023-03-03  7:03       ` Matthew Wilcox
2023-03-05 10:24         ` Nhat Pham
2023-03-05 10:32     ` Nhat Pham
2023-02-19  7:33 ` [PATCH v10 3/3] selftests: Add selftests for cachestat Nhat Pham

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).