[PATCH v3 0/5] KVM: selftests: access_tracking_perf_test fixes for NUMA balancing and MGLRU

[PATCH v3 0/5] KVM: selftests: access_tracking_perf_test fixes for NUMA balancing and MGLRU

[James Houghton]

This series fixes some issues with access_tracking_perf_test when MGLRU
or NUMA balancing are in use.

With MGLRU, touching a page doesn't necessarily clear the Idle flag.
This has come up in the past, and the recommendation was to use MGLRU
generation numbers[1], which this series does.

With NUMA balancing, pages are temporarily mapped as PROT_NONE, so the
SPTEs will be zapped, losing the Accessed bits. The fix here is, in the
event we have lost access information to print a warning and continue
with the test, just like what we do if the test is running a nested VM.

A flag is added for the user to specify if they wish for the test to
always enforce or always skip this check.

Based on kvm/next.

Changelog:

v2[3] -> v3:
- Applied David's directory fix on patch 3.
- Added SoB-by, R-by (patch 2, missed in v2), and A-by.

v1[2] -> v2:
- Re-add clone3_selftests.h for cgroup selftests (thanks Michal!)
- Some comment fixes, patches 2 and 5 (thanks Maxim!).

[1]: https://lore.kernel.org/all/CAOUHufZeADNp_y=Ng+acmMMgnTR=ZGFZ7z-m6O47O=CmJauWjw@mail.gmail.com/
[2]: https://lore.kernel.org/kvm/20250327012350.1135621-1-jthoughton@google.com/
[3]: https://lore.kernel.org/kvm/20250331213025.3602082-1-jthoughton@google.com/

James Houghton (3):
  cgroup: selftests: Move cgroup_util into its own library
  KVM: selftests: Build and link selftests/cgroup/lib into KVM selftests
  KVM: selftests: access_tracking_perf_test: Use MGLRU for access
    tracking

Maxim Levitsky (1):
  KVM: selftests: access_tracking_perf_test: Add option to skip the
    sanity check

Sean Christopherson (1):
  KVM: selftests: Extract guts of THP accessor to standalone sysfs
    helpers

 tools/testing/selftests/cgroup/Makefile       |  21 +-
 .../selftests/cgroup/{ => lib}/cgroup_util.c  |   2 +-
 .../cgroup/{ => lib/include}/cgroup_util.h    |   4 +-
 .../testing/selftests/cgroup/lib/libcgroup.mk |  19 +
 tools/testing/selftests/kvm/Makefile.kvm      |   4 +-
 .../selftests/kvm/access_tracking_perf_test.c | 263 ++++++++++--
 .../selftests/kvm/include/lru_gen_util.h      |  51 +++
 .../testing/selftests/kvm/include/test_util.h |   1 +
 .../testing/selftests/kvm/lib/lru_gen_util.c  | 383 ++++++++++++++++++
 tools/testing/selftests/kvm/lib/test_util.c   |  42 +-
 10 files changed, 733 insertions(+), 57 deletions(-)
 rename tools/testing/selftests/cgroup/{ => lib}/cgroup_util.c (99%)
 rename tools/testing/selftests/cgroup/{ => lib/include}/cgroup_util.h (99%)
 create mode 100644 tools/testing/selftests/cgroup/lib/libcgroup.mk
 create mode 100644 tools/testing/selftests/kvm/include/lru_gen_util.h
 create mode 100644 tools/testing/selftests/kvm/lib/lru_gen_util.c


base-commit: fd02aa45bda6d2f2fedcab70e828867332ef7e1c
-- 
2.49.0.604.gff1f9ca942-goog

[PATCH v3 1/5] KVM: selftests: Extract guts of THP accessor to standalone sysfs helpers

[James Houghton]

From: Sean Christopherson <seanjc@google.com>

Extract the guts of thp_configured() and get_trans_hugepagesz() to
standalone helpers so that the core logic can be reused for other sysfs
files, e.g. to query numa_balancing.

Opportunistically assert that the initial fscanf() read at least one byte,
and add a comment explaining the second call to fscanf().

Signed-off-by: Sean Christopherson <seanjc@google.com>
Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
Signed-off-by: James Houghton <jthoughton@google.com>
---
 tools/testing/selftests/kvm/lib/test_util.c | 35 ++++++++++++++-------
 1 file changed, 24 insertions(+), 11 deletions(-)

diff --git a/tools/testing/selftests/kvm/lib/test_util.c b/tools/testing/selftests/kvm/lib/test_util.c
index 8ed0b74ae8373..3dc8538f5d696 100644
--- a/tools/testing/selftests/kvm/lib/test_util.c
+++ b/tools/testing/selftests/kvm/lib/test_util.c
@@ -132,37 +132,50 @@ void print_skip(const char *fmt, ...)
 	puts(", skipping test");
 }
 
-bool thp_configured(void)
+static bool test_sysfs_path(const char *path)
 {
-	int ret;
 	struct stat statbuf;
+	int ret;
 
-	ret = stat("/sys/kernel/mm/transparent_hugepage", &statbuf);
+	ret = stat(path, &statbuf);
 	TEST_ASSERT(ret == 0 || (ret == -1 && errno == ENOENT),
-		    "Error in stating /sys/kernel/mm/transparent_hugepage");
+		    "Error in stat()ing '%s'", path);
 
 	return ret == 0;
 }
 
-size_t get_trans_hugepagesz(void)
+bool thp_configured(void)
+{
+	return test_sysfs_path("/sys/kernel/mm/transparent_hugepage");
+}
+
+static size_t get_sysfs_val(const char *path)
 {
 	size_t size;
 	FILE *f;
 	int ret;
 
-	TEST_ASSERT(thp_configured(), "THP is not configured in host kernel");
-
-	f = fopen("/sys/kernel/mm/transparent_hugepage/hpage_pmd_size", "r");
-	TEST_ASSERT(f != NULL, "Error in opening transparent_hugepage/hpage_pmd_size");
+	f = fopen(path, "r");
+	TEST_ASSERT(f, "Error opening '%s'", path);
 
 	ret = fscanf(f, "%ld", &size);
+	TEST_ASSERT(ret > 0, "Error reading '%s'", path);
+
+	/* Re-scan the input stream to verify the entire file was read. */
 	ret = fscanf(f, "%ld", &size);
-	TEST_ASSERT(ret < 1, "Error reading transparent_hugepage/hpage_pmd_size");
-	fclose(f);
+	TEST_ASSERT(ret < 1, "Error reading '%s'", path);
 
+	fclose(f);
 	return size;
 }
 
+size_t get_trans_hugepagesz(void)
+{
+	TEST_ASSERT(thp_configured(), "THP is not configured in host kernel");
+
+	return get_sysfs_val("/sys/kernel/mm/transparent_hugepage/hpage_pmd_size");
+}
+
 size_t get_def_hugetlb_pagesz(void)
 {
 	char buf[64];
-- 
2.49.0.604.gff1f9ca942-goog

[PATCH v3 2/5] KVM: selftests: access_tracking_perf_test: Add option to skip the sanity check

[James Houghton]

From: Maxim Levitsky <mlevitsk@redhat.com>

Add an option to skip sanity check of number of still idle pages,
and set it by default to skip, in case hypervisor or NUMA balancing
is detected.

Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
Co-developed-by: James Houghton <jthoughton@google.com>
Signed-off-by: James Houghton <jthoughton@google.com>
Reviewed-by: Maxim Levitsky <mlevitsk@redhat.com>
---
 .../selftests/kvm/access_tracking_perf_test.c | 62 ++++++++++++++++---
 .../testing/selftests/kvm/include/test_util.h |  1 +
 tools/testing/selftests/kvm/lib/test_util.c   |  7 +++
 3 files changed, 61 insertions(+), 9 deletions(-)

diff --git a/tools/testing/selftests/kvm/access_tracking_perf_test.c b/tools/testing/selftests/kvm/access_tracking_perf_test.c
index 447e619cf856e..a2ac6fa2ba141 100644
--- a/tools/testing/selftests/kvm/access_tracking_perf_test.c
+++ b/tools/testing/selftests/kvm/access_tracking_perf_test.c
@@ -65,6 +65,16 @@ static int vcpu_last_completed_iteration[KVM_MAX_VCPUS];
 /* Whether to overlap the regions of memory vCPUs access. */
 static bool overlap_memory_access;
 
+/*
+ * If the test should only warn if there are too many idle pages (i.e., it is
+ * expected).
+ * -1: Not yet set.
+ *  0: We do not expect too many idle pages, so FAIL if too many idle pages.
+ *  1: Having too many idle pages is expected, so merely print a warning if
+ *     too many idle pages are found.
+ */
+static int idle_pages_warn_only = -1;
+
 struct test_params {
 	/* The backing source for the region of memory. */
 	enum vm_mem_backing_src_type backing_src;
@@ -177,18 +187,12 @@ static void mark_vcpu_memory_idle(struct kvm_vm *vm,
 	 * arbitrary; high enough that we ensure most memory access went through
 	 * access tracking but low enough as to not make the test too brittle
 	 * over time and across architectures.
-	 *
-	 * When running the guest as a nested VM, "warn" instead of asserting
-	 * as the TLB size is effectively unlimited and the KVM doesn't
-	 * explicitly flush the TLB when aging SPTEs.  As a result, more pages
-	 * are cached and the guest won't see the "idle" bit cleared.
 	 */
 	if (still_idle >= pages / 10) {
-#ifdef __x86_64__
-		TEST_ASSERT(this_cpu_has(X86_FEATURE_HYPERVISOR),
+		TEST_ASSERT(idle_pages_warn_only,
 			    "vCPU%d: Too many pages still idle (%lu out of %lu)",
 			    vcpu_idx, still_idle, pages);
-#endif
+
 		printf("WARNING: vCPU%d: Too many pages still idle (%lu out of %lu), "
 		       "this will affect performance results.\n",
 		       vcpu_idx, still_idle, pages);
@@ -328,6 +332,32 @@ static void run_test(enum vm_guest_mode mode, void *arg)
 	memstress_destroy_vm(vm);
 }
 
+static int access_tracking_unreliable(void)
+{
+#ifdef __x86_64__
+	/*
+	 * When running nested, the TLB size may be effectively unlimited (for
+	 * example, this is the case when running on KVM L0), and KVM doesn't
+	 * explicitly flush the TLB when aging SPTEs.  As a result, more pages
+	 * are cached and the guest won't see the "idle" bit cleared.
+	 */
+	if (this_cpu_has(X86_FEATURE_HYPERVISOR)) {
+		puts("Skipping idle page count sanity check, because the test is run nested");
+		return 1;
+	}
+#endif
+	/*
+	 * When NUMA balancing is enabled, guest memory will be unmapped to get
+	 * NUMA faults, dropping the Accessed bits.
+	 */
+	if (is_numa_balancing_enabled()) {
+		puts("Skipping idle page count sanity check, because NUMA balancing is enabled");
+		return 1;
+	}
+
+	return 0;
+}
+
 static void help(char *name)
 {
 	puts("");
@@ -342,6 +372,12 @@ static void help(char *name)
 	printf(" -v: specify the number of vCPUs to run.\n");
 	printf(" -o: Overlap guest memory accesses instead of partitioning\n"
 	       "     them into a separate region of memory for each vCPU.\n");
+	printf(" -w: Control whether the test warns or fails if more than 10%\n"
+	       "     of pages are still seen as idle/old after accessing guest\n"
+	       "     memory.  >0 == warn only, 0 == fail, <0 == auto.  For auto\n"
+	       "     mode, the test fails by default, but switches to warn only\n"
+	       "     if NUMA balancing is enabled or the test detects it's running\n"
+	       "     in a VM.\n");
 	backing_src_help("-s");
 	puts("");
 	exit(0);
@@ -359,7 +395,7 @@ int main(int argc, char *argv[])
 
 	guest_modes_append_default();
 
-	while ((opt = getopt(argc, argv, "hm:b:v:os:")) != -1) {
+	while ((opt = getopt(argc, argv, "hm:b:v:os:w:")) != -1) {
 		switch (opt) {
 		case 'm':
 			guest_modes_cmdline(optarg);
@@ -376,6 +412,11 @@ int main(int argc, char *argv[])
 		case 's':
 			params.backing_src = parse_backing_src_type(optarg);
 			break;
+		case 'w':
+			idle_pages_warn_only =
+				atoi_non_negative("Idle pages warning",
+						  optarg);
+			break;
 		case 'h':
 		default:
 			help(argv[0]);
@@ -388,6 +429,9 @@ int main(int argc, char *argv[])
 		       "CONFIG_IDLE_PAGE_TRACKING is not enabled");
 	close(page_idle_fd);
 
+	if (idle_pages_warn_only == -1)
+		idle_pages_warn_only = access_tracking_unreliable();
+
 	for_each_guest_mode(run_test, &params);
 
 	return 0;
diff --git a/tools/testing/selftests/kvm/include/test_util.h b/tools/testing/selftests/kvm/include/test_util.h
index 77d13d7920cb8..c6ef895fbd9ab 100644
--- a/tools/testing/selftests/kvm/include/test_util.h
+++ b/tools/testing/selftests/kvm/include/test_util.h
@@ -153,6 +153,7 @@ bool is_backing_src_hugetlb(uint32_t i);
 void backing_src_help(const char *flag);
 enum vm_mem_backing_src_type parse_backing_src_type(const char *type_name);
 long get_run_delay(void);
+bool is_numa_balancing_enabled(void);
 
 /*
  * Whether or not the given source type is shared memory (as opposed to
diff --git a/tools/testing/selftests/kvm/lib/test_util.c b/tools/testing/selftests/kvm/lib/test_util.c
index 3dc8538f5d696..03eb99af9b8de 100644
--- a/tools/testing/selftests/kvm/lib/test_util.c
+++ b/tools/testing/selftests/kvm/lib/test_util.c
@@ -176,6 +176,13 @@ size_t get_trans_hugepagesz(void)
 	return get_sysfs_val("/sys/kernel/mm/transparent_hugepage/hpage_pmd_size");
 }
 
+bool is_numa_balancing_enabled(void)
+{
+	if (!test_sysfs_path("/proc/sys/kernel/numa_balancing"))
+		return false;
+	return get_sysfs_val("/proc/sys/kernel/numa_balancing") == 1;
+}
+
 size_t get_def_hugetlb_pagesz(void)
 {
 	char buf[64];
-- 
2.49.0.604.gff1f9ca942-goog

[PATCH v3 3/5] cgroup: selftests: Move cgroup_util into its own library

[James Houghton]

KVM selftests will soon need to use some of the cgroup creation and
deletion functionality from cgroup_util.

Suggested-by: David Matlack <dmatlack@google.com>
Signed-off-by: James Houghton <jthoughton@google.com>
---
 tools/testing/selftests/cgroup/Makefile       | 21 ++++++++++---------
 .../selftests/cgroup/{ => lib}/cgroup_util.c  |  2 +-
 .../cgroup/{ => lib/include}/cgroup_util.h    |  4 ++--
 .../testing/selftests/cgroup/lib/libcgroup.mk | 19 +++++++++++++++++
 4 files changed, 33 insertions(+), 13 deletions(-)
 rename tools/testing/selftests/cgroup/{ => lib}/cgroup_util.c (99%)
 rename tools/testing/selftests/cgroup/{ => lib/include}/cgroup_util.h (99%)
 create mode 100644 tools/testing/selftests/cgroup/lib/libcgroup.mk

diff --git a/tools/testing/selftests/cgroup/Makefile b/tools/testing/selftests/cgroup/Makefile
index 1b897152bab6e..e01584c2189ac 100644
--- a/tools/testing/selftests/cgroup/Makefile
+++ b/tools/testing/selftests/cgroup/Makefile
@@ -21,14 +21,15 @@ TEST_GEN_PROGS += test_zswap
 LOCAL_HDRS += $(selfdir)/clone3/clone3_selftests.h $(selfdir)/pidfd/pidfd.h
 
 include ../lib.mk
+include lib/libcgroup.mk
 
-$(OUTPUT)/test_core: cgroup_util.c
-$(OUTPUT)/test_cpu: cgroup_util.c
-$(OUTPUT)/test_cpuset: cgroup_util.c
-$(OUTPUT)/test_freezer: cgroup_util.c
-$(OUTPUT)/test_hugetlb_memcg: cgroup_util.c
-$(OUTPUT)/test_kill: cgroup_util.c
-$(OUTPUT)/test_kmem: cgroup_util.c
-$(OUTPUT)/test_memcontrol: cgroup_util.c
-$(OUTPUT)/test_pids: cgroup_util.c
-$(OUTPUT)/test_zswap: cgroup_util.c
+$(OUTPUT)/test_core: $(LIBCGROUP_O)
+$(OUTPUT)/test_cpu: $(LIBCGROUP_O)
+$(OUTPUT)/test_cpuset: $(LIBCGROUP_O)
+$(OUTPUT)/test_freezer: $(LIBCGROUP_O)
+$(OUTPUT)/test_hugetlb_memcg: $(LIBCGROUP_O)
+$(OUTPUT)/test_kill: $(LIBCGROUP_O)
+$(OUTPUT)/test_kmem: $(LIBCGROUP_O)
+$(OUTPUT)/test_memcontrol: $(LIBCGROUP_O)
+$(OUTPUT)/test_pids: $(LIBCGROUP_O)
+$(OUTPUT)/test_zswap: $(LIBCGROUP_O)
diff --git a/tools/testing/selftests/cgroup/cgroup_util.c b/tools/testing/selftests/cgroup/lib/cgroup_util.c
similarity index 99%
rename from tools/testing/selftests/cgroup/cgroup_util.c
rename to tools/testing/selftests/cgroup/lib/cgroup_util.c
index 1e2d46636a0ca..f047d8adaec65 100644
--- a/tools/testing/selftests/cgroup/cgroup_util.c
+++ b/tools/testing/selftests/cgroup/lib/cgroup_util.c
@@ -17,7 +17,7 @@
 #include <unistd.h>
 
 #include "cgroup_util.h"
-#include "../clone3/clone3_selftests.h"
+#include "../../clone3/clone3_selftests.h"
 
 /* Returns read len on success, or -errno on failure. */
 static ssize_t read_text(const char *path, char *buf, size_t max_len)
diff --git a/tools/testing/selftests/cgroup/cgroup_util.h b/tools/testing/selftests/cgroup/lib/include/cgroup_util.h
similarity index 99%
rename from tools/testing/selftests/cgroup/cgroup_util.h
rename to tools/testing/selftests/cgroup/lib/include/cgroup_util.h
index 19b131ee77072..7a0441e5eb296 100644
--- a/tools/testing/selftests/cgroup/cgroup_util.h
+++ b/tools/testing/selftests/cgroup/lib/include/cgroup_util.h
@@ -2,9 +2,9 @@
 #include <stdbool.h>
 #include <stdlib.h>
 
-#include "../kselftest.h"
-
+#ifndef PAGE_SIZE
 #define PAGE_SIZE 4096
+#endif
 
 #define MB(x) (x << 20)
 
diff --git a/tools/testing/selftests/cgroup/lib/libcgroup.mk b/tools/testing/selftests/cgroup/lib/libcgroup.mk
new file mode 100644
index 0000000000000..7a73007204c39
--- /dev/null
+++ b/tools/testing/selftests/cgroup/lib/libcgroup.mk
@@ -0,0 +1,19 @@
+CGROUP_DIR := $(selfdir)/cgroup
+
+LIBCGROUP_C := lib/cgroup_util.c
+
+LIBCGROUP_O := $(patsubst %.c, $(OUTPUT)/%.o, $(LIBCGROUP_C))
+
+LIBCGROUP_O_DIRS := $(shell dirname $(LIBCGROUP_O) | uniq)
+
+CFLAGS += -I$(CGROUP_DIR)/lib/include
+
+EXTRA_HDRS := $(selfdir)/clone3/clone3_selftests.h
+
+$(LIBCGROUP_O_DIRS):
+	mkdir -p $@
+
+$(LIBCGROUP_O): $(OUTPUT)/%.o : $(CGROUP_DIR)/%.c $(EXTRA_HDRS) $(LIBCGROUP_O_DIRS)
+	$(CC) $(CFLAGS) $(CPPFLAGS) $(TARGET_ARCH) -c $< -o $@
+
+EXTRA_CLEAN += $(LIBCGROUP_O)
-- 
2.49.0.604.gff1f9ca942-goog

Re: [PATCH v3 3/5] cgroup: selftests: Move cgroup_util into its own library

[James Houghton]

On Mon, Apr 14, 2025 at 1:09 PM James Houghton <jthoughton@google.com> wrote:
>
> KVM selftests will soon need to use some of the cgroup creation and
> deletion functionality from cgroup_util.
>
> Suggested-by: David Matlack <dmatlack@google.com>
> Signed-off-by: James Houghton <jthoughton@google.com>

Tejun had provided his Acked-by[1] and I somehow forgot to include it.
(I know I should just use b4 and redownload the series with
everything...) Sorry about that.

[1]: https://lore.kernel.org/kvm/Z-sQ76PG14ai9jC0@slm.duckdns.org/

[PATCH v3 4/5] KVM: selftests: Build and link selftests/cgroup/lib into KVM selftests

[James Houghton]

libcgroup.o is built separately from KVM selftests and cgroup selftests,
so different compiler flags used by the different selftests will not
conflict with each other.

Signed-off-by: James Houghton <jthoughton@google.com>
---
 tools/testing/selftests/kvm/Makefile.kvm | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/kvm/Makefile.kvm b/tools/testing/selftests/kvm/Makefile.kvm
index f62b0a5aba35a..bea746878bcaa 100644
--- a/tools/testing/selftests/kvm/Makefile.kvm
+++ b/tools/testing/selftests/kvm/Makefile.kvm
@@ -204,6 +204,7 @@ OVERRIDE_TARGETS = 1
 # importantly defines, i.e. overwrites, $(CC) (unless `make -e` or `make CC=`,
 # which causes the environment variable to override the makefile).
 include ../lib.mk
+include ../cgroup/lib/libcgroup.mk
 
 INSTALL_HDR_PATH = $(top_srcdir)/usr
 LINUX_HDR_PATH = $(INSTALL_HDR_PATH)/include/
@@ -257,7 +258,7 @@ LIBKVM_S := $(filter %.S,$(LIBKVM))
 LIBKVM_C_OBJ := $(patsubst %.c, $(OUTPUT)/%.o, $(LIBKVM_C))
 LIBKVM_S_OBJ := $(patsubst %.S, $(OUTPUT)/%.o, $(LIBKVM_S))
 LIBKVM_STRING_OBJ := $(patsubst %.c, $(OUTPUT)/%.o, $(LIBKVM_STRING))
-LIBKVM_OBJS = $(LIBKVM_C_OBJ) $(LIBKVM_S_OBJ) $(LIBKVM_STRING_OBJ)
+LIBKVM_OBJS = $(LIBKVM_C_OBJ) $(LIBKVM_S_OBJ) $(LIBKVM_STRING_OBJ) $(LIBCGROUP_O)
 SPLIT_TEST_GEN_PROGS := $(patsubst %, $(OUTPUT)/%, $(SPLIT_TESTS))
 SPLIT_TEST_GEN_OBJ := $(patsubst %, $(OUTPUT)/$(ARCH)/%.o, $(SPLIT_TESTS))
 
-- 
2.49.0.604.gff1f9ca942-goog

Re: [PATCH v3 4/5] KVM: selftests: Build and link selftests/cgroup/lib into KVM selftests

[Sean Christopherson]

On Mon, Apr 14, 2025, James Houghton wrote:
> libcgroup.o is built separately from KVM selftests and cgroup selftests,
> so different compiler flags used by the different selftests will not
> conflict with each other.

This fails to build on some of my systems.  And it generates a warning, which
thanks to me building KVM selftests with -Werror, is the only such warning in
all of KVM selftests.

tools/testing/selftests/cgroup/lib/cgroup_util.c:511:17: warning: ignoring return value of ‘read’ declared with attribute ‘warn_unused_result’ [-Wunused-result]
  511 |                 read(fd, buf, sizeof(buf));
      |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~

In file included from /usr/include/fcntl.h:314,
                 from tools/testing/selftests/cgroup/lib/cgroup_util.c:6:
In function ‘open’,
    inlined from ‘get_temp_fd’ at tools/testing/selftests/cgroup/lib/cgroup_util.c:493:9:
/usr/include/x86_64-linux-gnu/bits/fcntl2.h:50:11: error: call to ‘__open_missing_mode’ declared with attribute error: open with O_CREAT or O_TMPFILE in second argument needs 3 arguments
   50 |           __open_missing_mode ();
      |           ^~~~~~~~~~~~~~~~~~~~~~


Given that the code in question has nothing to do with cgroups and is used only
by test_memcontrol.c, my vote is to move it into test_memcontrol.c and let the
cgroups folks sort things out at their leisure (if it even ever becomes an issue
for them).

E.g. slot this is before making cgroup_util.c a library?

--
From: Sean Christopherson <seanjc@google.com>
Date: Mon, 28 Apr 2025 17:38:14 -0700
Subject: [PATCH] selftests: cgroup: Move memcontrol specific helpers out of
 common cgroup_util.c

Move a handful of helpers out of cgroup_util.c and into test_memcontrol.c
that have nothing to with cgroups in general, in anticipation of making
cgroup_util.c a generic library that can be used by other selftests.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 tools/testing/selftests/cgroup/cgroup_util.c  | 78 -------------------
 tools/testing/selftests/cgroup/cgroup_util.h  |  5 --
 .../selftests/cgroup/test_memcontrol.c        | 78 +++++++++++++++++++
 3 files changed, 78 insertions(+), 83 deletions(-)

diff --git a/tools/testing/selftests/cgroup/cgroup_util.c b/tools/testing/selftests/cgroup/cgroup_util.c
index 1e2d46636a0c..023a87ff7ebc 100644
--- a/tools/testing/selftests/cgroup/cgroup_util.c
+++ b/tools/testing/selftests/cgroup/cgroup_util.c
@@ -488,84 +488,6 @@ int cg_run_nowait(const char *cgroup,
 	return pid;
 }
 
-int get_temp_fd(void)
-{
-	return open(".", O_TMPFILE | O_RDWR | O_EXCL);
-}
-
-int alloc_pagecache(int fd, size_t size)
-{
-	char buf[PAGE_SIZE];
-	struct stat st;
-	int i;
-
-	if (fstat(fd, &st))
-		goto cleanup;
-
-	size += st.st_size;
-
-	if (ftruncate(fd, size))
-		goto cleanup;
-
-	for (i = 0; i < size; i += sizeof(buf))
-		read(fd, buf, sizeof(buf));
-
-	return 0;
-
-cleanup:
-	return -1;
-}
-
-int alloc_anon(const char *cgroup, void *arg)
-{
-	size_t size = (unsigned long)arg;
-	char *buf, *ptr;
-
-	buf = malloc(size);
-	for (ptr = buf; ptr < buf + size; ptr += PAGE_SIZE)
-		*ptr = 0;
-
-	free(buf);
-	return 0;
-}
-
-int is_swap_enabled(void)
-{
-	char buf[PAGE_SIZE];
-	const char delim[] = "\n";
-	int cnt = 0;
-	char *line;
-
-	if (read_text("/proc/swaps", buf, sizeof(buf)) <= 0)
-		return -1;
-
-	for (line = strtok(buf, delim); line; line = strtok(NULL, delim))
-		cnt++;
-
-	return cnt > 1;
-}
-
-int set_oom_adj_score(int pid, int score)
-{
-	char path[PATH_MAX];
-	int fd, len;
-
-	sprintf(path, "/proc/%d/oom_score_adj", pid);
-
-	fd = open(path, O_WRONLY | O_APPEND);
-	if (fd < 0)
-		return fd;
-
-	len = dprintf(fd, "%d", score);
-	if (len < 0) {
-		close(fd);
-		return len;
-	}
-
-	close(fd);
-	return 0;
-}
-
 int proc_mount_contains(const char *option)
 {
 	char buf[4 * PAGE_SIZE];
diff --git a/tools/testing/selftests/cgroup/cgroup_util.h b/tools/testing/selftests/cgroup/cgroup_util.h
index 19b131ee7707..bdc50a8e6b85 100644
--- a/tools/testing/selftests/cgroup/cgroup_util.h
+++ b/tools/testing/selftests/cgroup/cgroup_util.h
@@ -49,11 +49,6 @@ extern int cg_enter_current_thread(const char *cgroup);
 extern int cg_run_nowait(const char *cgroup,
 			 int (*fn)(const char *cgroup, void *arg),
 			 void *arg);
-extern int get_temp_fd(void);
-extern int alloc_pagecache(int fd, size_t size);
-extern int alloc_anon(const char *cgroup, void *arg);
-extern int is_swap_enabled(void);
-extern int set_oom_adj_score(int pid, int score);
 extern int cg_wait_for_proc_count(const char *cgroup, int count);
 extern int cg_killall(const char *cgroup);
 int proc_mount_contains(const char *option);
diff --git a/tools/testing/selftests/cgroup/test_memcontrol.c b/tools/testing/selftests/cgroup/test_memcontrol.c
index 16f5d74ae762..5414ca4df24c 100644
--- a/tools/testing/selftests/cgroup/test_memcontrol.c
+++ b/tools/testing/selftests/cgroup/test_memcontrol.c
@@ -24,6 +24,84 @@
 static bool has_localevents;
 static bool has_recursiveprot;
 
+static int get_temp_fd(void)
+{
+	return open(".", O_TMPFILE | O_RDWR | O_EXCL);
+}
+
+static int alloc_pagecache(int fd, size_t size)
+{
+	char buf[PAGE_SIZE];
+	struct stat st;
+	int i;
+
+	if (fstat(fd, &st))
+		goto cleanup;
+
+	size += st.st_size;
+
+	if (ftruncate(fd, size))
+		goto cleanup;
+
+	for (i = 0; i < size; i += sizeof(buf))
+		read(fd, buf, sizeof(buf));
+
+	return 0;
+
+cleanup:
+	return -1;
+}
+
+static int alloc_anon(const char *cgroup, void *arg)
+{
+	size_t size = (unsigned long)arg;
+	char *buf, *ptr;
+
+	buf = malloc(size);
+	for (ptr = buf; ptr < buf + size; ptr += PAGE_SIZE)
+		*ptr = 0;
+
+	free(buf);
+	return 0;
+}
+
+static int is_swap_enabled(void)
+{
+	char buf[PAGE_SIZE];
+	const char delim[] = "\n";
+	int cnt = 0;
+	char *line;
+
+	if (read_text("/proc/swaps", buf, sizeof(buf)) <= 0)
+		return -1;
+
+	for (line = strtok(buf, delim); line; line = strtok(NULL, delim))
+		cnt++;
+
+	return cnt > 1;
+}
+
+static int set_oom_adj_score(int pid, int score)
+{
+	char path[PATH_MAX];
+	int fd, len;
+
+	sprintf(path, "/proc/%d/oom_score_adj", pid);
+
+	fd = open(path, O_WRONLY | O_APPEND);
+	if (fd < 0)
+		return fd;
+
+	len = dprintf(fd, "%d", score);
+	if (len < 0) {
+		close(fd);
+		return len;
+	}
+
+	close(fd);
+	return 0;
+}
+
 /*
  * This test creates two nested cgroups with and without enabling
  * the memory controller.

base-commit: 4a243ec9b255aeb0f033c646148aaf662fd92c64
--

Re: [PATCH v3 4/5] KVM: selftests: Build and link selftests/cgroup/lib into KVM selftests

[Michal Koutný]

[-- Attachment #1: Type: text/plain, Size: 998 bytes --]

On Mon, Apr 28, 2025 at 06:03:46PM -0700, Sean Christopherson <seanjc@google.com> wrote:
...
> E.g. slot this is before making cgroup_util.c a library?
> 
> --
> From: Sean Christopherson <seanjc@google.com>
> Date: Mon, 28 Apr 2025 17:38:14 -0700
> Subject: [PATCH] selftests: cgroup: Move memcontrol specific helpers out of
>  common cgroup_util.c
> 
> Move a handful of helpers out of cgroup_util.c and into test_memcontrol.c
> that have nothing to with cgroups in general, in anticipation of making
> cgroup_util.c a generic library that can be used by other selftests.
> 
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
>  tools/testing/selftests/cgroup/cgroup_util.c  | 78 -------------------
>  tools/testing/selftests/cgroup/cgroup_util.h  |  5 --
>  .../selftests/cgroup/test_memcontrol.c        | 78 +++++++++++++++++++
>  3 files changed, 78 insertions(+), 83 deletions(-)

This reorg makes sense to me,

Acked-by: Michal Koutný <mkoutny@suse.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[PATCH v3 5/5] KVM: selftests: access_tracking_perf_test: Use MGLRU for access tracking

[James Houghton]

By using MGLRU's debugfs for invoking test_young() and clear_young(), we
avoid page_idle's incompatibility with MGLRU, and we can mark pages as
idle (clear_young()) much faster.

The ability to use page_idle is left in, as it is useful for kernels
that do not have MGLRU built in. If MGLRU is enabled but is not usable
(e.g. we can't access the debugfs mount), the test will fail, as
page_idle is not compatible with MGLRU.

cgroup utility functions have been borrowed so that, when running with
MGLRU, we can create a memcg in which to run our test.

Other MGLRU-debugfs-specific parsing code has been added to
lru_gen_util.{c,h}.

Co-developed-by: Axel Rasmussen <axelrasmussen@google.com>
Signed-off-by: Axel Rasmussen <axelrasmussen@google.com>
Signed-off-by: James Houghton <jthoughton@google.com>
---
 tools/testing/selftests/kvm/Makefile.kvm      |   1 +
 .../selftests/kvm/access_tracking_perf_test.c | 207 ++++++++--
 .../selftests/kvm/include/lru_gen_util.h      |  51 +++
 .../testing/selftests/kvm/lib/lru_gen_util.c  | 383 ++++++++++++++++++
 4 files changed, 616 insertions(+), 26 deletions(-)
 create mode 100644 tools/testing/selftests/kvm/include/lru_gen_util.h
 create mode 100644 tools/testing/selftests/kvm/lib/lru_gen_util.c

diff --git a/tools/testing/selftests/kvm/Makefile.kvm b/tools/testing/selftests/kvm/Makefile.kvm
index bea746878bcaa..24d2a5d387d34 100644
--- a/tools/testing/selftests/kvm/Makefile.kvm
+++ b/tools/testing/selftests/kvm/Makefile.kvm
@@ -8,6 +8,7 @@ LIBKVM += lib/elf.c
 LIBKVM += lib/guest_modes.c
 LIBKVM += lib/io.c
 LIBKVM += lib/kvm_util.c
+LIBKVM += lib/lru_gen_util.c
 LIBKVM += lib/memstress.c
 LIBKVM += lib/guest_sprintf.c
 LIBKVM += lib/rbtree.c
diff --git a/tools/testing/selftests/kvm/access_tracking_perf_test.c b/tools/testing/selftests/kvm/access_tracking_perf_test.c
index a2ac6fa2ba141..d4ef201b67055 100644
--- a/tools/testing/selftests/kvm/access_tracking_perf_test.c
+++ b/tools/testing/selftests/kvm/access_tracking_perf_test.c
@@ -7,9 +7,11 @@
  * This test measures the performance effects of KVM's access tracking.
  * Access tracking is driven by the MMU notifiers test_young, clear_young, and
  * clear_flush_young. These notifiers do not have a direct userspace API,
- * however the clear_young notifier can be triggered by marking a pages as idle
- * in /sys/kernel/mm/page_idle/bitmap. This test leverages that mechanism to
- * enable access tracking on guest memory.
+ * however the clear_young notifier can be triggered either by
+ *   1. marking a pages as idle in /sys/kernel/mm/page_idle/bitmap OR
+ *   2. adding a new MGLRU generation using the lru_gen debugfs file.
+ * This test leverages page_idle to enable access tracking on guest memory
+ * unless MGLRU is enabled, in which case MGLRU is used.
  *
  * To measure performance this test runs a VM with a configurable number of
  * vCPUs that each touch every page in disjoint regions of memory. Performance
@@ -17,10 +19,11 @@
  * predefined region.
  *
  * Note that a deterministic correctness test of access tracking is not possible
- * by using page_idle as it exists today. This is for a few reasons:
+ * by using page_idle or MGLRU aging as it exists today. This is for a few
+ * reasons:
  *
- * 1. page_idle only issues clear_young notifiers, which lack a TLB flush. This
- *    means subsequent guest accesses are not guaranteed to see page table
+ * 1. page_idle and MGLRU only issue clear_young notifiers, which lack a TLB flush.
+ *    This means subsequent guest accesses are not guaranteed to see page table
  *    updates made by KVM until some time in the future.
  *
  * 2. page_idle only operates on LRU pages. Newly allocated pages are not
@@ -48,9 +51,17 @@
 #include "guest_modes.h"
 #include "processor.h"
 
+#include "cgroup_util.h"
+#include "lru_gen_util.h"
+
+static const char *TEST_MEMCG_NAME = "access_tracking_perf_test";
+
 /* Global variable used to synchronize all of the vCPU threads. */
 static int iteration;
 
+/* The cgroup v2 root. Needed for lru_gen-based aging. */
+char cgroup_root[PATH_MAX];
+
 /* Defines what vCPU threads should do during a given iteration. */
 static enum {
 	/* Run the vCPU to access all its memory. */
@@ -75,6 +86,12 @@ static bool overlap_memory_access;
  */
 static int idle_pages_warn_only = -1;
 
+/* Whether or not to use MGLRU instead of page_idle for access tracking */
+static bool use_lru_gen;
+
+/* Total number of pages to expect in the memcg after touching everything */
+static long total_pages;
+
 struct test_params {
 	/* The backing source for the region of memory. */
 	enum vm_mem_backing_src_type backing_src;
@@ -133,8 +150,24 @@ static void mark_page_idle(int page_idle_fd, uint64_t pfn)
 		    "Set page_idle bits for PFN 0x%" PRIx64, pfn);
 }
 
-static void mark_vcpu_memory_idle(struct kvm_vm *vm,
-				  struct memstress_vcpu_args *vcpu_args)
+static void too_many_idle_pages(long idle_pages, long total_pages, int vcpu_idx)
+{
+	char prefix[18] = {};
+
+	if (vcpu_idx >= 0)
+		snprintf(prefix, 18, "vCPU%d: ", vcpu_idx);
+
+	TEST_ASSERT(idle_pages_warn_only,
+		    "%sToo many pages still idle (%lu out of %lu)",
+		    prefix, idle_pages, total_pages);
+
+	printf("WARNING: %sToo many pages still idle (%lu out of %lu), "
+	       "this will affect performance results.\n",
+	       prefix, idle_pages, total_pages);
+}
+
+static void pageidle_mark_vcpu_memory_idle(struct kvm_vm *vm,
+					   struct memstress_vcpu_args *vcpu_args)
 {
 	int vcpu_idx = vcpu_args->vcpu_idx;
 	uint64_t base_gva = vcpu_args->gva;
@@ -188,20 +221,81 @@ static void mark_vcpu_memory_idle(struct kvm_vm *vm,
 	 * access tracking but low enough as to not make the test too brittle
 	 * over time and across architectures.
 	 */
-	if (still_idle >= pages / 10) {
-		TEST_ASSERT(idle_pages_warn_only,
-			    "vCPU%d: Too many pages still idle (%lu out of %lu)",
-			    vcpu_idx, still_idle, pages);
-
-		printf("WARNING: vCPU%d: Too many pages still idle (%lu out of %lu), "
-		       "this will affect performance results.\n",
-		       vcpu_idx, still_idle, pages);
-	}
+	if (still_idle >= pages / 10)
+		too_many_idle_pages(still_idle, pages,
+				    overlap_memory_access ? -1 : vcpu_idx);
 
 	close(page_idle_fd);
 	close(pagemap_fd);
 }
 
+int find_generation(struct memcg_stats *stats, long total_pages)
+{
+	/*
+	 * For finding the generation that contains our pages, use the same
+	 * 90% threshold that page_idle uses.
+	 */
+	int gen = lru_gen_find_generation(stats, total_pages * 9 / 10);
+
+	if (gen >= 0)
+		return gen;
+
+	if (!idle_pages_warn_only) {
+		TEST_FAIL("Could not find a generation with 90%% of guest memory (%ld pages).",
+			   total_pages * 9 / 10);
+		return gen;
+	}
+
+	/*
+	 * We couldn't find a generation with 90% of guest memory, which can
+	 * happen if access tracking is unreliable. Simply look for a majority
+	 * of pages.
+	 */
+	puts("WARNING: Couldn't find a generation with 90% of guest memory. "
+	     "Performance results may not be accurate.");
+	gen = lru_gen_find_generation(stats, total_pages / 2);
+	TEST_ASSERT(gen >= 0,
+		    "Could not find a generation with 50%% of guest memory (%ld pages).",
+		    total_pages / 2);
+	return gen;
+}
+
+static void lru_gen_mark_memory_idle(struct kvm_vm *vm)
+{
+	struct timespec ts_start;
+	struct timespec ts_elapsed;
+	struct memcg_stats stats;
+	int found_gens[2];
+
+	/* Find current generation the pages lie in. */
+	lru_gen_read_memcg_stats(&stats, TEST_MEMCG_NAME);
+	found_gens[0] = find_generation(&stats, total_pages);
+
+	/* Make a new generation */
+	clock_gettime(CLOCK_MONOTONIC, &ts_start);
+	lru_gen_do_aging(&stats, TEST_MEMCG_NAME);
+	ts_elapsed = timespec_elapsed(ts_start);
+
+	/* Check the generation again */
+	found_gens[1] = find_generation(&stats, total_pages);
+
+	/*
+	 * This function should only be invoked with newly-accessed pages,
+	 * so pages should always move to a newer generation.
+	 */
+	if (found_gens[0] >= found_gens[1]) {
+		/* We did not move to a newer generation. */
+		long idle_pages = lru_gen_sum_memcg_stats_for_gen(found_gens[1],
+								  &stats);
+
+		too_many_idle_pages(min_t(long, idle_pages, total_pages),
+				    total_pages, -1);
+	}
+	pr_info("%-30s: %ld.%09lds\n",
+		"Mark memory idle (lru_gen)", ts_elapsed.tv_sec,
+		ts_elapsed.tv_nsec);
+}
+
 static void assert_ucall(struct kvm_vcpu *vcpu, uint64_t expected_ucall)
 {
 	struct ucall uc;
@@ -241,7 +335,7 @@ static void vcpu_thread_main(struct memstress_vcpu_args *vcpu_args)
 			assert_ucall(vcpu, UCALL_SYNC);
 			break;
 		case ITERATION_MARK_IDLE:
-			mark_vcpu_memory_idle(vm, vcpu_args);
+			pageidle_mark_vcpu_memory_idle(vm, vcpu_args);
 			break;
 		}
 
@@ -293,15 +387,18 @@ static void access_memory(struct kvm_vm *vm, int nr_vcpus,
 
 static void mark_memory_idle(struct kvm_vm *vm, int nr_vcpus)
 {
+	if (use_lru_gen)
+		return lru_gen_mark_memory_idle(vm);
+
 	/*
 	 * Even though this parallelizes the work across vCPUs, this is still a
 	 * very slow operation because page_idle forces the test to mark one pfn
-	 * at a time and the clear_young notifier serializes on the KVM MMU
+	 * at a time and the clear_young notifier may serialize on the KVM MMU
 	 * lock.
 	 */
 	pr_debug("Marking VM memory idle (slow)...\n");
 	iteration_work = ITERATION_MARK_IDLE;
-	run_iteration(vm, nr_vcpus, "Mark memory idle");
+	run_iteration(vm, nr_vcpus, "Mark memory idle (page_idle)");
 }
 
 static void run_test(enum vm_guest_mode mode, void *arg)
@@ -318,6 +415,14 @@ static void run_test(enum vm_guest_mode mode, void *arg)
 	pr_info("\n");
 	access_memory(vm, nr_vcpus, ACCESS_WRITE, "Populating memory");
 
+	if (use_lru_gen) {
+		struct memcg_stats stats;
+
+		lru_gen_read_memcg_stats(&stats, TEST_MEMCG_NAME);
+		TEST_ASSERT(lru_gen_sum_memcg_stats(&stats) >= total_pages,
+			    "Not all pages accounted for. Was the memcg set up correctly?");
+	}
+
 	/* As a control, read and write to the populated memory first. */
 	access_memory(vm, nr_vcpus, ACCESS_WRITE, "Writing to populated memory");
 	access_memory(vm, nr_vcpus, ACCESS_READ, "Reading from populated memory");
@@ -354,7 +459,12 @@ static int access_tracking_unreliable(void)
 		puts("Skipping idle page count sanity check, because NUMA balancing is enabled");
 		return 1;
 	}
+	return 0;
+}
 
+int run_test_in_cg(const char *cgroup, void *arg)
+{
+	for_each_guest_mode(run_test, arg);
 	return 0;
 }
 
@@ -372,7 +482,7 @@ static void help(char *name)
 	printf(" -v: specify the number of vCPUs to run.\n");
 	printf(" -o: Overlap guest memory accesses instead of partitioning\n"
 	       "     them into a separate region of memory for each vCPU.\n");
-	printf(" -w: Control whether the test warns or fails if more than 10%\n"
+	printf(" -w: Control whether the test warns or fails if more than 10%%\n"
 	       "     of pages are still seen as idle/old after accessing guest\n"
 	       "     memory.  >0 == warn only, 0 == fail, <0 == auto.  For auto\n"
 	       "     mode, the test fails by default, but switches to warn only\n"
@@ -383,6 +493,12 @@ static void help(char *name)
 	exit(0);
 }
 
+void destroy_cgroup(char *cg)
+{
+	printf("Destroying cgroup: %s\n", cg);
+	cg_destroy(cg);
+}
+
 int main(int argc, char *argv[])
 {
 	struct test_params params = {
@@ -390,6 +506,7 @@ int main(int argc, char *argv[])
 		.vcpu_memory_bytes = DEFAULT_PER_VCPU_MEM_SIZE,
 		.nr_vcpus = 1,
 	};
+	char *new_cg = NULL;
 	int page_idle_fd;
 	int opt;
 
@@ -424,15 +541,53 @@ int main(int argc, char *argv[])
 		}
 	}
 
-	page_idle_fd = open("/sys/kernel/mm/page_idle/bitmap", O_RDWR);
-	__TEST_REQUIRE(page_idle_fd >= 0,
-		       "CONFIG_IDLE_PAGE_TRACKING is not enabled");
-	close(page_idle_fd);
+	if (lru_gen_usable()) {
+		if (cg_find_unified_root(cgroup_root, sizeof(cgroup_root), NULL))
+			ksft_exit_skip("cgroup v2 isn't mounted\n");
+
+		new_cg = cg_name(cgroup_root, TEST_MEMCG_NAME);
+		printf("Creating cgroup: %s\n", new_cg);
+		if (cg_create(new_cg) && errno != EEXIST)
+			ksft_exit_skip("could not create new cgroup: %s\n", new_cg);
+
+		use_lru_gen = true;
+	} else {
+		page_idle_fd = open("/sys/kernel/mm/page_idle/bitmap", O_RDWR);
+		__TEST_REQUIRE(page_idle_fd >= 0,
+			       "Couldn't open /sys/kernel/mm/page_idle/bitmap. "
+			       "Is CONFIG_IDLE_PAGE_TRACKING enabled?");
+
+		close(page_idle_fd);
+	}
 
 	if (idle_pages_warn_only == -1)
 		idle_pages_warn_only = access_tracking_unreliable();
 
-	for_each_guest_mode(run_test, &params);
+	/*
+	 * If guest_page_size is larger than the host's page size, the
+	 * guest (memstress) will only fault in a subset of the host's pages.
+	 */
+	total_pages = params.nr_vcpus * params.vcpu_memory_bytes /
+		      max(memstress_args.guest_page_size,
+			  (uint64_t)getpagesize());
+
+	if (use_lru_gen) {
+		int ret;
+
+		puts("Using lru_gen for aging");
+		/*
+		 * This will fork off a new process to run the test within
+		 * a new memcg, so we need to properly propagate the return
+		 * value up.
+		 */
+		ret = cg_run(new_cg, &run_test_in_cg, &params);
+		destroy_cgroup(new_cg);
+		if (ret)
+			return ret;
+	} else {
+		puts("Using page_idle for aging");
+		for_each_guest_mode(run_test, &params);
+	}
 
 	return 0;
 }
diff --git a/tools/testing/selftests/kvm/include/lru_gen_util.h b/tools/testing/selftests/kvm/include/lru_gen_util.h
new file mode 100644
index 0000000000000..d32ff5d8ffd05
--- /dev/null
+++ b/tools/testing/selftests/kvm/include/lru_gen_util.h
@@ -0,0 +1,51 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Tools for integrating with lru_gen, like parsing the lru_gen debugfs output.
+ *
+ * Copyright (C) 2025, Google LLC.
+ */
+#ifndef SELFTEST_KVM_LRU_GEN_UTIL_H
+#define SELFTEST_KVM_LRU_GEN_UTIL_H
+
+#include <inttypes.h>
+#include <limits.h>
+#include <stdlib.h>
+
+#include "test_util.h"
+
+#define MAX_NR_GENS 16 /* MAX_NR_GENS in include/linux/mmzone.h */
+#define MAX_NR_NODES 4 /* Maximum number of nodes supported by the test */
+
+#define LRU_GEN_DEBUGFS "/sys/kernel/debug/lru_gen"
+#define LRU_GEN_ENABLED_PATH "/sys/kernel/mm/lru_gen/enabled"
+#define LRU_GEN_ENABLED 1
+#define LRU_GEN_MM_WALK 2
+
+struct generation_stats {
+	int gen;
+	long age_ms;
+	long nr_anon;
+	long nr_file;
+};
+
+struct node_stats {
+	int node;
+	int nr_gens; /* Number of populated gens entries. */
+	struct generation_stats gens[MAX_NR_GENS];
+};
+
+struct memcg_stats {
+	unsigned long memcg_id;
+	int nr_nodes; /* Number of populated nodes entries. */
+	struct node_stats nodes[MAX_NR_NODES];
+};
+
+void lru_gen_read_memcg_stats(struct memcg_stats *stats, const char *memcg);
+long lru_gen_sum_memcg_stats(const struct memcg_stats *stats);
+long lru_gen_sum_memcg_stats_for_gen(int gen, const struct memcg_stats *stats);
+void lru_gen_do_aging(struct memcg_stats *stats, const char *memcg);
+int lru_gen_find_generation(const struct memcg_stats *stats,
+			    unsigned long total_pages);
+bool lru_gen_usable(void);
+
+#endif /* SELFTEST_KVM_LRU_GEN_UTIL_H */
diff --git a/tools/testing/selftests/kvm/lib/lru_gen_util.c b/tools/testing/selftests/kvm/lib/lru_gen_util.c
new file mode 100644
index 0000000000000..783a1f1028a26
--- /dev/null
+++ b/tools/testing/selftests/kvm/lib/lru_gen_util.c
@@ -0,0 +1,383 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2025, Google LLC.
+ */
+
+#include <time.h>
+
+#include "lru_gen_util.h"
+
+/*
+ * Tracks state while we parse memcg lru_gen stats. The file we're parsing is
+ * structured like this (some extra whitespace elided):
+ *
+ * memcg (id) (path)
+ * node (id)
+ * (gen_nr) (age_in_ms) (nr_anon_pages) (nr_file_pages)
+ */
+struct memcg_stats_parse_context {
+	bool consumed; /* Whether or not this line was consumed */
+	/* Next parse handler to invoke */
+	void (*next_handler)(struct memcg_stats *,
+			     struct memcg_stats_parse_context *, char *);
+	int current_node_idx; /* Current index in nodes array */
+	const char *name; /* The name of the memcg we're looking for */
+};
+
+static void memcg_stats_handle_searching(struct memcg_stats *stats,
+					 struct memcg_stats_parse_context *ctx,
+					 char *line);
+static void memcg_stats_handle_in_memcg(struct memcg_stats *stats,
+					struct memcg_stats_parse_context *ctx,
+					char *line);
+static void memcg_stats_handle_in_node(struct memcg_stats *stats,
+				       struct memcg_stats_parse_context *ctx,
+				       char *line);
+
+struct split_iterator {
+	char *str;
+	char *save;
+};
+
+static char *split_next(struct split_iterator *it)
+{
+	char *ret = strtok_r(it->str, " \t\n\r", &it->save);
+
+	it->str = NULL;
+	return ret;
+}
+
+static void memcg_stats_handle_searching(struct memcg_stats *stats,
+					 struct memcg_stats_parse_context *ctx,
+					 char *line)
+{
+	struct split_iterator it = { .str = line };
+	char *prefix = split_next(&it);
+	char *memcg_id = split_next(&it);
+	char *memcg_name = split_next(&it);
+	char *end;
+
+	ctx->consumed = true;
+
+	if (!prefix || strcmp("memcg", prefix))
+		return; /* Not a memcg line (maybe empty), skip */
+
+	TEST_ASSERT(memcg_id && memcg_name,
+		    "malformed memcg line; no memcg id or memcg_name");
+
+	if (strcmp(memcg_name + 1, ctx->name))
+		return; /* Wrong memcg, skip */
+
+	/* Found it! */
+
+	stats->memcg_id = strtoul(memcg_id, &end, 10);
+	TEST_ASSERT(*end == '\0', "malformed memcg id '%s'", memcg_id);
+	if (!stats->memcg_id)
+		return; /* Removed memcg? */
+
+	ctx->next_handler = memcg_stats_handle_in_memcg;
+}
+
+static void memcg_stats_handle_in_memcg(struct memcg_stats *stats,
+					struct memcg_stats_parse_context *ctx,
+					char *line)
+{
+	struct split_iterator it = { .str = line };
+	char *prefix = split_next(&it);
+	char *id = split_next(&it);
+	long found_node_id;
+	char *end;
+
+	ctx->consumed = true;
+	ctx->current_node_idx = -1;
+
+	if (!prefix)
+		return; /* Skip empty lines */
+
+	if (!strcmp("memcg", prefix)) {
+		/* Memcg done, found next one; stop. */
+		ctx->next_handler = NULL;
+		return;
+	} else if (strcmp("node", prefix))
+		TEST_ASSERT(false, "found malformed line after 'memcg ...',"
+				   "token: '%s'", prefix);
+
+	/* At this point we know we have a node line. Parse the ID. */
+
+	TEST_ASSERT(id, "malformed node line; no node id");
+
+	found_node_id = strtol(id, &end, 10);
+	TEST_ASSERT(*end == '\0', "malformed node id '%s'", id);
+
+	ctx->current_node_idx = stats->nr_nodes++;
+	TEST_ASSERT(ctx->current_node_idx < MAX_NR_NODES,
+		    "memcg has stats for too many nodes, max is %d",
+		    MAX_NR_NODES);
+	stats->nodes[ctx->current_node_idx].node = found_node_id;
+
+	ctx->next_handler = memcg_stats_handle_in_node;
+}
+
+static void memcg_stats_handle_in_node(struct memcg_stats *stats,
+				       struct memcg_stats_parse_context *ctx,
+				       char *line)
+{
+	char *my_line = strdup(line);
+	struct split_iterator it = { .str = my_line };
+	char *gen, *age, *nr_anon, *nr_file;
+	struct node_stats *node_stats;
+	struct generation_stats *gen_stats;
+	char *end;
+
+	TEST_ASSERT(it.str, "failed to copy input line");
+
+	gen = split_next(&it);
+
+	if (!gen)
+		goto out_consume; /* Skip empty lines */
+
+	if (!strcmp("memcg", gen) || !strcmp("node", gen)) {
+		/*
+		 * Reached next memcg or node section. Don't consume, let the
+		 * other handler deal with this.
+		 */
+		ctx->next_handler = memcg_stats_handle_in_memcg;
+		goto out;
+	}
+
+	node_stats = &stats->nodes[ctx->current_node_idx];
+	TEST_ASSERT(node_stats->nr_gens < MAX_NR_GENS,
+		    "found too many generation lines; max is %d",
+		    MAX_NR_GENS);
+	gen_stats = &node_stats->gens[node_stats->nr_gens++];
+
+	age = split_next(&it);
+	nr_anon = split_next(&it);
+	nr_file = split_next(&it);
+
+	TEST_ASSERT(age && nr_anon && nr_file,
+		    "malformed generation line; not enough tokens");
+
+	gen_stats->gen = (int)strtol(gen, &end, 10);
+	TEST_ASSERT(*end == '\0', "malformed generation number '%s'", gen);
+
+	gen_stats->age_ms = strtol(age, &end, 10);
+	TEST_ASSERT(*end == '\0', "malformed generation age '%s'", age);
+
+	gen_stats->nr_anon = strtol(nr_anon, &end, 10);
+	TEST_ASSERT(*end == '\0', "malformed anonymous page count '%s'",
+		    nr_anon);
+
+	gen_stats->nr_file = strtol(nr_file, &end, 10);
+	TEST_ASSERT(*end == '\0', "malformed file page count '%s'", nr_file);
+
+out_consume:
+	ctx->consumed = true;
+out:
+	free(my_line);
+}
+
+static void print_memcg_stats(const struct memcg_stats *stats, const char *name)
+{
+	int node, gen;
+
+	pr_debug("stats for memcg %s (id %lu):\n", name, stats->memcg_id);
+	for (node = 0; node < stats->nr_nodes; ++node) {
+		pr_debug("\tnode %d\n", stats->nodes[node].node);
+		for (gen = 0; gen < stats->nodes[node].nr_gens; ++gen) {
+			const struct generation_stats *gstats =
+				&stats->nodes[node].gens[gen];
+
+			pr_debug("\t\tgen %d\tage_ms %ld"
+				 "\tnr_anon %ld\tnr_file %ld\n",
+				 gstats->gen, gstats->age_ms, gstats->nr_anon,
+				 gstats->nr_file);
+		}
+	}
+}
+
+/* Re-read lru_gen debugfs information for @memcg into @stats. */
+void lru_gen_read_memcg_stats(struct memcg_stats *stats, const char *memcg)
+{
+	FILE *f;
+	ssize_t read = 0;
+	char *line = NULL;
+	size_t bufsz;
+	struct memcg_stats_parse_context ctx = {
+		.next_handler = memcg_stats_handle_searching,
+		.name = memcg,
+	};
+
+	memset(stats, 0, sizeof(struct memcg_stats));
+
+	f = fopen(LRU_GEN_DEBUGFS, "r");
+	TEST_ASSERT(f, "fopen(%s) failed", LRU_GEN_DEBUGFS);
+
+	while (ctx.next_handler && (read = getline(&line, &bufsz, f)) > 0) {
+		ctx.consumed = false;
+
+		do {
+			ctx.next_handler(stats, &ctx, line);
+			if (!ctx.next_handler)
+				break;
+		} while (!ctx.consumed);
+	}
+
+	if (read < 0 && !feof(f))
+		TEST_ASSERT(false, "getline(%s) failed", LRU_GEN_DEBUGFS);
+
+	TEST_ASSERT(stats->memcg_id > 0, "Couldn't find memcg: %s\n"
+		    "Did the memcg get created in the proper mount?",
+		    memcg);
+	if (line)
+		free(line);
+	TEST_ASSERT(!fclose(f), "fclose(%s) failed", LRU_GEN_DEBUGFS);
+
+	print_memcg_stats(stats, memcg);
+}
+
+/*
+ * Find all pages tracked by lru_gen for this memcg in generation @target_gen.
+ *
+ * If @target_gen is negative, look for all generations.
+ */
+long lru_gen_sum_memcg_stats_for_gen(int target_gen,
+				     const struct memcg_stats *stats)
+{
+	int node, gen;
+	long total_nr = 0;
+
+	for (node = 0; node < stats->nr_nodes; ++node) {
+		const struct node_stats *node_stats = &stats->nodes[node];
+
+		for (gen = 0; gen < node_stats->nr_gens; ++gen) {
+			const struct generation_stats *gen_stats =
+				&node_stats->gens[gen];
+
+			if (target_gen >= 0 && gen_stats->gen != target_gen)
+				continue;
+
+			total_nr += gen_stats->nr_anon + gen_stats->nr_file;
+		}
+	}
+
+	return total_nr;
+}
+
+/* Find all pages tracked by lru_gen for this memcg. */
+long lru_gen_sum_memcg_stats(const struct memcg_stats *stats)
+{
+	return lru_gen_sum_memcg_stats_for_gen(-1, stats);
+}
+
+/*
+ * If lru_gen aging should force page table scanning.
+ *
+ * If you want to set this to false, you will need to do eviction
+ * before doing extra aging passes.
+ */
+static const bool force_scan = true;
+
+static void run_aging_impl(unsigned long memcg_id, int node_id, int max_gen)
+{
+	FILE *f = fopen(LRU_GEN_DEBUGFS, "w");
+	char *command;
+	size_t sz;
+
+	TEST_ASSERT(f, "fopen(%s) failed", LRU_GEN_DEBUGFS);
+	sz = asprintf(&command, "+ %lu %d %d 1 %d\n",
+		      memcg_id, node_id, max_gen, force_scan);
+	TEST_ASSERT(sz > 0, "creating aging command failed");
+
+	pr_debug("Running aging command: %s", command);
+	if (fwrite(command, sizeof(char), sz, f) < sz) {
+		TEST_ASSERT(false, "writing aging command %s to %s failed",
+			    command, LRU_GEN_DEBUGFS);
+	}
+
+	TEST_ASSERT(!fclose(f), "fclose(%s) failed", LRU_GEN_DEBUGFS);
+}
+
+void lru_gen_do_aging(struct memcg_stats *stats, const char *memcg)
+{
+	int node, gen;
+
+	pr_debug("lru_gen: invoking aging...\n");
+
+	/* Must read memcg stats to construct the proper aging command. */
+	lru_gen_read_memcg_stats(stats, memcg);
+
+	for (node = 0; node < stats->nr_nodes; ++node) {
+		int max_gen = 0;
+
+		for (gen = 0; gen < stats->nodes[node].nr_gens; ++gen) {
+			int this_gen = stats->nodes[node].gens[gen].gen;
+
+			max_gen = max_gen > this_gen ? max_gen : this_gen;
+		}
+
+		run_aging_impl(stats->memcg_id, stats->nodes[node].node,
+			       max_gen);
+	}
+
+	/* Re-read so callers get updated information */
+	lru_gen_read_memcg_stats(stats, memcg);
+}
+
+/*
+ * Find which generation contains at least @pages pages, assuming that
+ * such a generation exists.
+ */
+int lru_gen_find_generation(const struct memcg_stats *stats,
+			    unsigned long pages)
+{
+	int node, gen, gen_idx, min_gen = INT_MAX, max_gen = -1;
+
+	for (node = 0; node < stats->nr_nodes; ++node)
+		for (gen_idx = 0; gen_idx < stats->nodes[node].nr_gens;
+		     ++gen_idx) {
+			gen = stats->nodes[node].gens[gen_idx].gen;
+			max_gen = gen > max_gen ? gen : max_gen;
+			min_gen = gen < min_gen ? gen : min_gen;
+		}
+
+	for (gen = min_gen; gen < max_gen; ++gen)
+		/* See if this generation has enough pages. */
+		if (lru_gen_sum_memcg_stats_for_gen(gen, stats) > pages)
+			return gen;
+
+	return -1;
+}
+
+bool lru_gen_usable(void)
+{
+	long required_features = LRU_GEN_ENABLED | LRU_GEN_MM_WALK;
+	int lru_gen_fd, lru_gen_debug_fd;
+	char mglru_feature_str[8] = {};
+	long mglru_features;
+
+	lru_gen_fd = open(LRU_GEN_ENABLED_PATH, O_RDONLY);
+	if (lru_gen_fd < 0) {
+		puts("lru_gen: Could not open " LRU_GEN_ENABLED_PATH);
+		return false;
+	}
+	if (read(lru_gen_fd, &mglru_feature_str, 7) < 7) {
+		puts("lru_gen: Could not read from " LRU_GEN_ENABLED_PATH);
+		close(lru_gen_fd);
+		return false;
+	}
+	close(lru_gen_fd);
+
+	mglru_features = strtol(mglru_feature_str, NULL, 16);
+	if ((mglru_features & required_features) != required_features) {
+		printf("lru_gen: missing features, got: %s", mglru_feature_str);
+		return false;
+	}
+
+	lru_gen_debug_fd = open(LRU_GEN_DEBUGFS, O_RDWR);
+	__TEST_REQUIRE(lru_gen_debug_fd >= 0,
+		       "lru_gen: Could not open " LRU_GEN_DEBUGFS ", "
+		       "but lru_gen is enabled, so cannot use page_idle.");
+	close(lru_gen_debug_fd);
+	return true;
+}
-- 
2.49.0.604.gff1f9ca942-goog

Re: [PATCH v3 5/5] KVM: selftests: access_tracking_perf_test: Use MGLRU for access tracking

[Sean Christopherson]

On Mon, Apr 14, 2025, James Houghton wrote:
> By using MGLRU's debugfs for invoking test_young() and clear_young(), we
> avoid page_idle's incompatibility with MGLRU, and we can mark pages as
> idle (clear_young()) much faster.
> 
> The ability to use page_idle is left in, as it is useful for kernels
> that do not have MGLRU built in. If MGLRU is enabled but is not usable
> (e.g. we can't access the debugfs mount), the test will fail, as
> page_idle is not compatible with MGLRU.
> 
> cgroup utility functions have been borrowed so that, when running with
> MGLRU, we can create a memcg in which to run our test.
> 
> Other MGLRU-debugfs-specific parsing code has been added to
> lru_gen_util.{c,h}.

This fails on my end due to not being able to find the cgroup.  I spent about 15
minutes poking at it and gave it.  FWIW, this is on our devrez hosts, so it's
presumably similar hardware to what you tested on.

Even if this turns out to be PEBKAC or some CONFIG_XXX incompatibility, there
needs to be better hints provided to the user of how they can some this.

And this would be a perfect opportunity to clean up this:

 	__TEST_REQUIRE(page_idle_fd >= 0,
		       "CONFIG_IDLE_PAGE_TRACKING is not enabled");

I can't count the number of times I've forgotten to run the test with root
privileges, and wasted a bunch of time remembering it's not that the kernel
doesn't have CONFIG_IDLE_PAGE_TRACKING, but that /sys/kernel/mm/page_idle/bitmap
isn't accessible.

I mention that, because on a kernel with MGRLU available but disabled, and
CONFIG_IDLE_PAGE_TRACKING=n, the user has no idea that they _can_ run the test
without mucking with their kernel.

==== Test Assertion Failure ====
  lib/lru_gen_util.c:229: stats->memcg_id > 0
  pid=423298 tid=423298 errno=2 - No such file or directory
     1	0x0000000000408b45: lru_gen_read_memcg_stats at lru_gen_util.c:229
     2	0x0000000000402e4c: run_test at access_tracking_perf_test.c:421
     3	0x0000000000403694: for_each_guest_mode at guest_modes.c:96
     4	0x00000000004023dd: run_test_in_cg at access_tracking_perf_test.c:467
     5	0x000000000041ba65: cg_run at cgroup_util.c:362
     6	0x0000000000402042: main at access_tracking_perf_test.c:583
     7	0x000000000041c753: __libc_start_call_main at libc-start.o:?
     8	0x000000000041e9ac: __libc_start_main_impl at ??:?
     9	0x0000000000402280: _start at ??:?
  Couldn't find memcg: access_tracking_perf_test
Did the memcg get created in the proper mount?
Destroying cgroup: /sys/fs/cgroup/access_tracking_perf_test

Re: [PATCH v3 5/5] KVM: selftests: access_tracking_perf_test: Use MGLRU for access tracking

[James Houghton]

On Fri, Apr 25, 2025 at 8:31 PM Sean Christopherson <seanjc@google.com> wrote:
>
> On Mon, Apr 14, 2025, James Houghton wrote:
> > By using MGLRU's debugfs for invoking test_young() and clear_young(), we
> > avoid page_idle's incompatibility with MGLRU, and we can mark pages as
> > idle (clear_young()) much faster.
> >
> > The ability to use page_idle is left in, as it is useful for kernels
> > that do not have MGLRU built in. If MGLRU is enabled but is not usable
> > (e.g. we can't access the debugfs mount), the test will fail, as
> > page_idle is not compatible with MGLRU.
> >
> > cgroup utility functions have been borrowed so that, when running with
> > MGLRU, we can create a memcg in which to run our test.
> >
> > Other MGLRU-debugfs-specific parsing code has been added to
> > lru_gen_util.{c,h}.
>
> This fails on my end due to not being able to find the cgroup.  I spent about 15
> minutes poking at it and gave it.  FWIW, this is on our devrez hosts, so it's
> presumably similar hardware to what you tested on.

Ah sorry, yes, this selftest needs to be patched when running the
devrez userspace, which uses a combination of cgroup-v1 and cgroup-v2.
Simply hard-coding the root to "/dev/cgroup/memory" (which is in fact
a cgroup-v1 mount) should be what you need if you want to give it
another go.

> Even if this turns out to be PEBKAC or some CONFIG_XXX incompatibility, there
> needs to be better hints provided to the user of how they can some this.

Yeah this can be better. I should at least check that the found
cgroup-v2 root's cgroup.controllers contains "memory". In your case,
it did not.

(cgroup.controllers is not available for cgroup-v1 -- because it
doesn't make sense -- so if I patch the selftest to check this file,
using cgroup-v1 mounts will stop working. So, again, you'd need to
patch the test to work on devrez.)

> And this would be a perfect opportunity to clean up this:
>
>         __TEST_REQUIRE(page_idle_fd >= 0,
>                        "CONFIG_IDLE_PAGE_TRACKING is not enabled");

I think the change I've already made to this string is sufficient
(happy to change it further if you like):
> > +               __TEST_REQUIRE(page_idle_fd >= 0,
> > +                              "Couldn't open /sys/kernel/mm/page_idle/bitmap. "
> > +                              "Is CONFIG_IDLE_PAGE_TRACKING enabled?");

> I can't count the number of times I've forgotten to run the test with root
> privileges, and wasted a bunch of time remembering it's not that the kernel
> doesn't have CONFIG_IDLE_PAGE_TRACKING, but that /sys/kernel/mm/page_idle/bitmap
> isn't accessible.
>
> I mention that, because on a kernel with MGRLU available but disabled, and
> CONFIG_IDLE_PAGE_TRACKING=n, the user has no idea that they _can_ run the test
> without mucking with their kernel.

Fair enough, I'll change the output from the test for that
configuration to say something like: "please either enable the missing
MGLRU features (e.g. `echo 3 > /sys/kernel/mm/lru_gen/enabled`) or
recompile your kernel with CONFIG_IDLE_PAGE_TRACKING=y."

> ==== Test Assertion Failure ====
>   lib/lru_gen_util.c:229: stats->memcg_id > 0
>   pid=423298 tid=423298 errno=2 - No such file or directory
>      1  0x0000000000408b45: lru_gen_read_memcg_stats at lru_gen_util.c:229
>      2  0x0000000000402e4c: run_test at access_tracking_perf_test.c:421
>      3  0x0000000000403694: for_each_guest_mode at guest_modes.c:96
>      4  0x00000000004023dd: run_test_in_cg at access_tracking_perf_test.c:467
>      5  0x000000000041ba65: cg_run at cgroup_util.c:362
>      6  0x0000000000402042: main at access_tracking_perf_test.c:583
>      7  0x000000000041c753: __libc_start_call_main at libc-start.o:?
>      8  0x000000000041e9ac: __libc_start_main_impl at ??:?
>      9  0x0000000000402280: _start at ??:?
>   Couldn't find memcg: access_tracking_perf_test
> Did the memcg get created in the proper mount?
> Destroying cgroup: /sys/fs/cgroup/access_tracking_perf_test

Thanks for taking a look!

Re: [PATCH v3 5/5] KVM: selftests: access_tracking_perf_test: Use MGLRU for access tracking

[Sean Christopherson]

On Mon, Apr 28, 2025, James Houghton wrote:
> On Fri, Apr 25, 2025 at 8:31 PM Sean Christopherson <seanjc@google.com> wrote:
> >
> > On Mon, Apr 14, 2025, James Houghton wrote:
> > > By using MGLRU's debugfs for invoking test_young() and clear_young(), we
> > > avoid page_idle's incompatibility with MGLRU, and we can mark pages as
> > > idle (clear_young()) much faster.
> > >
> > > The ability to use page_idle is left in, as it is useful for kernels
> > > that do not have MGLRU built in. If MGLRU is enabled but is not usable
> > > (e.g. we can't access the debugfs mount), the test will fail, as
> > > page_idle is not compatible with MGLRU.
> > >
> > > cgroup utility functions have been borrowed so that, when running with
> > > MGLRU, we can create a memcg in which to run our test.
> > >
> > > Other MGLRU-debugfs-specific parsing code has been added to
> > > lru_gen_util.{c,h}.
> >
> > This fails on my end due to not being able to find the cgroup.  I spent about 15
> > minutes poking at it and gave it.  FWIW, this is on our devrez hosts, so it's
> > presumably similar hardware to what you tested on.
> 
> Ah sorry, yes, this selftest needs to be patched when running the
> devrez userspace, which uses a combination of cgroup-v1 and cgroup-v2.
> Simply hard-coding the root to "/dev/cgroup/memory" (which is in fact
> a cgroup-v1 mount) should be what you need if you want to give it
> another go.
>
> > Even if this turns out to be PEBKAC or some CONFIG_XXX incompatibility, there
> > needs to be better hints provided to the user of how they can some this.
> 
> Yeah this can be better. I should at least check that the found
> cgroup-v2 root's cgroup.controllers contains "memory". In your case,
> it did not.
> 
> (cgroup.controllers is not available for cgroup-v1 -- because it
> doesn't make sense -- so if I patch the selftest to check this file,
> using cgroup-v1 mounts will stop working. So, again, you'd need to
> patch the test to work on devrez.)

Or, and I know this going to sound crazy, what if we simply make the test work
with v1 or v2?  That is not hard to do, at all.  Please slot the below into the
next version of the series.  Feel free to modify it as needed, e.g. to address
other maintainer feedback.  The only thing I care about is the selftest not failing.

> > And this would be a perfect opportunity to clean up this:
> >
> >         __TEST_REQUIRE(page_idle_fd >= 0,
> >                        "CONFIG_IDLE_PAGE_TRACKING is not enabled");
> 
> I think the change I've already made to this string is sufficient
> (happy to change it further if you like):

Doh, I missed that your patch did already improve the skip message to spit out
/sys/kernel/mm/page_idle/bitmap; that part I definitely like.

> > > +               __TEST_REQUIRE(page_idle_fd >= 0,
> > > +                              "Couldn't open /sys/kernel/mm/page_idle/bitmap. "
> > > +                              "Is CONFIG_IDLE_PAGE_TRACKING enabled?");
> 
> > I can't count the number of times I've forgotten to run the test with root
> > privileges, and wasted a bunch of time remembering it's not that the kernel
> > doesn't have CONFIG_IDLE_PAGE_TRACKING, but that /sys/kernel/mm/page_idle/bitmap
> > isn't accessible.
> >
> > I mention that, because on a kernel with MGRLU available but disabled, and
> > CONFIG_IDLE_PAGE_TRACKING=n, the user has no idea that they _can_ run the test
> > without mucking with their kernel.
> 
> Fair enough, I'll change the output from the test for that
> configuration to say something like: "please either enable the missing
> MGLRU features (e.g. `echo 3 > /sys/kernel/mm/lru_gen/enabled`) or
> recompile your kernel with CONFIG_IDLE_PAGE_TRACKING=y."

That's still misleading.  In my case, my kernels are already built with
CONFIG_IDLE_PAGE_TRACKING=y.

Looking at this again, we can do much better.  For my permissions issue, open()
should fail with -EACCES, whereas the CONFIG_IDLE_PAGE_TRACKING=n case should
fail with ENOENT.  And that is easy enough to handle in open_path_or_exit().

I'll send a small series to clean that up, and then will apply this series on top.
The resulting conflict will be trivial to resolve, so don't worry about rebasing
on top of my mini-series.

--
From: Sean Christopherson <seanjc@google.com>
Date: Mon, 28 Apr 2025 17:36:06 -0700
Subject: [PATCH] cgroup: selftests: Add API to find root of specific
 controller

Add an API in the cgroups library to find the root of a specific
controller.  KVM selftests will use the API to find the memory controller.

Search for the controller on both v1 and v2 mounts, as KVM selftests'
usage will be completely oblivious of v1 versus v2.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 .../selftests/cgroup/lib/cgroup_util.c        | 32 +++++++++++++++----
 .../cgroup/lib/include/cgroup_util.h          |  1 +
 2 files changed, 27 insertions(+), 6 deletions(-)

diff --git a/tools/testing/selftests/cgroup/lib/cgroup_util.c b/tools/testing/selftests/cgroup/lib/cgroup_util.c
index 69a68f43e3fa..4e7e7329b226 100644
--- a/tools/testing/selftests/cgroup/lib/cgroup_util.c
+++ b/tools/testing/selftests/cgroup/lib/cgroup_util.c
@@ -217,7 +217,8 @@ int cg_write_numeric(const char *cgroup, const char *control, long value)
 	return cg_write(cgroup, control, buf);
 }
 
-int cg_find_unified_root(char *root, size_t len, bool *nsdelegate)
+static int cg_find_root(char *root, size_t len, const char *controller,
+			bool *nsdelegate)
 {
 	char buf[10 * PAGE_SIZE];
 	char *fs, *mount, *type, *options;
@@ -237,17 +238,36 @@ int cg_find_unified_root(char *root, size_t len, bool *nsdelegate)
 		strtok(NULL, delim);
 		strtok(NULL, delim);
 
-		if (strcmp(type, "cgroup2") == 0) {
-			strncpy(root, mount, len);
-			if (nsdelegate)
-				*nsdelegate = !!strstr(options, "nsdelegate");
-			return 0;
+		if (strcmp(type, "cgroup") == 0) {
+			if (!controller || !strstr(options, controller))
+				continue;
+		} else if (strcmp(type, "cgroup2") == 0) {
+			if (controller &&
+			    cg_read_strstr(mount, "cgroup.controllers", controller))
+				continue;
+		} else {
+			continue;
 		}
+		strncpy(root, mount, len);
+
+		if (nsdelegate)
+			*nsdelegate = !!strstr(options, "nsdelegate");
+		return 0;
 	}
 
 	return -1;
 }
 
+int cg_find_controller_root(char *root, size_t len, const char *controller)
+{
+	return cg_find_root(root, len, controller, NULL);
+}
+
+int cg_find_unified_root(char *root, size_t len, bool *nsdelegate)
+{
+	return cg_find_root(root, len, NULL, nsdelegate);
+}
+
 int cg_create(const char *cgroup)
 {
 	return mkdir(cgroup, 0755);
diff --git a/tools/testing/selftests/cgroup/lib/include/cgroup_util.h b/tools/testing/selftests/cgroup/lib/include/cgroup_util.h
index cbe6f0b4247d..d9e6e3090b3f 100644
--- a/tools/testing/selftests/cgroup/lib/include/cgroup_util.h
+++ b/tools/testing/selftests/cgroup/lib/include/cgroup_util.h
@@ -21,6 +21,7 @@ static inline int values_close(long a, long b, int err)
 	return labs(a - b) <= (a + b) / 100 * err;
 }
 
+extern int cg_find_controller_root(char *root, size_t len, const char *controller);
 extern int cg_find_unified_root(char *root, size_t len, bool *nsdelegate);
 extern char *cg_name(const char *root, const char *name);
 extern char *cg_name_indexed(const char *root, const char *name, int index);

base-commit: 65a87fcc85da28361af2a5718c109dbc2f8d54a2
--

Re: [PATCH v3 5/5] KVM: selftests: access_tracking_perf_test: Use MGLRU for access tracking

[Sean Christopherson]

On Mon, Apr 14, 2025, James Houghton wrote:
> By using MGLRU's debugfs for invoking test_young() and clear_young(), we
> avoid page_idle's incompatibility with MGLRU, and we can mark pages as
> idle (clear_young()) much faster.
> 
> The ability to use page_idle is left in, as it is useful for kernels
> that do not have MGLRU built in. If MGLRU is enabled but is not usable
> (e.g. we can't access the debugfs mount), the test will fail, as
> page_idle is not compatible with MGLRU.
> 
> cgroup utility functions have been borrowed so that, when running with
> MGLRU, we can create a memcg in which to run our test.
> 
> Other MGLRU-debugfs-specific parsing code has been added to
> lru_gen_util.{c,h}.

This is not a proper changelog, at least not by upstream KVM standards.  Please
rewrite it describe the changes being made, using imperative mood/tone.  From
Documentation/process/maintainer-kvm-x86.rst:

  Changelog
  ~~~~~~~~~
  Most importantly, write changelogs using imperative mood and avoid pronouns.

> @@ -354,7 +459,12 @@ static int access_tracking_unreliable(void)
>  		puts("Skipping idle page count sanity check, because NUMA balancing is enabled");
>  		return 1;
>  	}
> +	return 0;
> +}
>  
> +int run_test_in_cg(const char *cgroup, void *arg)

static

> +{
> +	for_each_guest_mode(run_test, arg);

Having "separate" flows for MGLRU vs. page_idle is unnecessary.  Give the helper
a more common name and use it for both:

static int run_test_for_each_guest_mode(const char *cgroup, void *arg)
{
	for_each_guest_mode(run_test, arg);
	return 0;
}

>  	return 0;
>  }
>  
> @@ -372,7 +482,7 @@ static void help(char *name)
>  	printf(" -v: specify the number of vCPUs to run.\n");
>  	printf(" -o: Overlap guest memory accesses instead of partitioning\n"
>  	       "     them into a separate region of memory for each vCPU.\n");
> -	printf(" -w: Control whether the test warns or fails if more than 10%\n"
> +	printf(" -w: Control whether the test warns or fails if more than 10%%\n"
>  	       "     of pages are still seen as idle/old after accessing guest\n"
>  	       "     memory.  >0 == warn only, 0 == fail, <0 == auto.  For auto\n"
>  	       "     mode, the test fails by default, but switches to warn only\n"
> @@ -383,6 +493,12 @@ static void help(char *name)
>  	exit(0);
>  }
>  
> +void destroy_cgroup(char *cg)

static.  But this is a pointless wrapper, just delete it.

> +{
> +	printf("Destroying cgroup: %s\n", cg);
> +	cg_destroy(cg);
> +}
> +
>  int main(int argc, char *argv[])
>  {
>  	struct test_params params = {
> @@ -390,6 +506,7 @@ int main(int argc, char *argv[])
>  		.vcpu_memory_bytes = DEFAULT_PER_VCPU_MEM_SIZE,
>  		.nr_vcpus = 1,
>  	};
> +	char *new_cg = NULL;
>  	int page_idle_fd;
>  	int opt;
>  
> @@ -424,15 +541,53 @@ int main(int argc, char *argv[])
>  		}
>  	}
>  
> -	page_idle_fd = open("/sys/kernel/mm/page_idle/bitmap", O_RDWR);
> -	__TEST_REQUIRE(page_idle_fd >= 0,
> -		       "CONFIG_IDLE_PAGE_TRACKING is not enabled");
> -	close(page_idle_fd);
> +	if (lru_gen_usable()) {

Using MGLRU on my home box fails.  It's full cgroup v2, and has both
CONFIG_IDLE_PAGE_TRACKING=y and MGLRU enabled.

==== Test Assertion Failure ====
  access_tracking_perf_test.c:244: false
  pid=114670 tid=114670 errno=17 - File exists
     1	0x00000000004032a9: find_generation at access_tracking_perf_test.c:244
     2	0x00000000004032da: lru_gen_mark_memory_idle at access_tracking_perf_test.c:272
     3	0x00000000004034e4: mark_memory_idle at access_tracking_perf_test.c:391
     4	 (inlined by) run_test at access_tracking_perf_test.c:431
     5	0x0000000000403d84: for_each_guest_mode at guest_modes.c:96
     6	0x0000000000402c61: run_test_for_each_guest_mode at access_tracking_perf_test.c:492
     7	0x000000000041d8e2: cg_run at cgroup_util.c:382
     8	0x00000000004027fa: main at access_tracking_perf_test.c:572
     9	0x00007fa1cb629d8f: ?? ??:0
    10	0x00007fa1cb629e3f: ?? ??:0
    11	0x00000000004029d4: _start at ??:?
  Could not find a generation with 90% of guest memory (235929 pages).

Interestingly, if I force the test to use /sys/kernel/mm/page_idle/bitmap, it
passes.

Please try to reproduce the failure (assuming you haven't already tested that
exact combination of cgroups v2, MGLRU=y, and CONFIG_IDLE_PAGE_TRACKING=y). I
don't have bandwidth to dig any further at this time.

> +		if (cg_find_unified_root(cgroup_root, sizeof(cgroup_root), NULL))
> +			ksft_exit_skip("cgroup v2 isn't mounted\n");
> +
> +		new_cg = cg_name(cgroup_root, TEST_MEMCG_NAME);
> +		printf("Creating cgroup: %s\n", new_cg);
> +		if (cg_create(new_cg) && errno != EEXIST)
> +			ksft_exit_skip("could not create new cgroup: %s\n", new_cg);
> +
> +		use_lru_gen = true;
> +	} else {
> +		page_idle_fd = open("/sys/kernel/mm/page_idle/bitmap", O_RDWR);
> +		__TEST_REQUIRE(page_idle_fd >= 0,
> +			       "Couldn't open /sys/kernel/mm/page_idle/bitmap. "
> +			       "Is CONFIG_IDLE_PAGE_TRACKING enabled?");
> +
> +		close(page_idle_fd);
> +	}

Splitting the "check" and "execute" into separate if-else statements results in
some compilers complaining about new_cg possibly being unused.  The compiler is
probably being a bit stupid, but the code is just as must to blame.  There's zero
reason to split this in two, just do everything after the idle_pages_warn_only
and total_pages processing.  Code at the bottom (note, you'll have to rebase on
my not-yet-posted series, or undo the use of __open_path_or_exit()).

>  
>  	if (idle_pages_warn_only == -1)
>  		idle_pages_warn_only = access_tracking_unreliable();
>  
> -	for_each_guest_mode(run_test, &params);
> +	/*
> +	 * If guest_page_size is larger than the host's page size, the
> +	 * guest (memstress) will only fault in a subset of the host's pages.
> +	 */
> +	total_pages = params.nr_vcpus * params.vcpu_memory_bytes /
> +		      max(memstress_args.guest_page_size,
> +			  (uint64_t)getpagesize());
> +
> +	if (use_lru_gen) {
> +		int ret;
> +
> +		puts("Using lru_gen for aging");
> +		/*
> +		 * This will fork off a new process to run the test within
> +		 * a new memcg, so we need to properly propagate the return
> +		 * value up.
> +		 */
> +		ret = cg_run(new_cg, &run_test_in_cg, &params);
> +		destroy_cgroup(new_cg);
> +		if (ret)
> +			return ret;
> +	} else {
> +		puts("Using page_idle for aging");
> +		for_each_guest_mode(run_test, &params);
> +	}

static int run_test_for_each_guest_mode(const char *cgroup, void *arg)
{
	for_each_guest_mode(run_test, arg);
	return 0;
}

int main(int argc, char *argv[])
{
	struct test_params params = {
		.backing_src = DEFAULT_VM_MEM_SRC,
		.vcpu_memory_bytes = DEFAULT_PER_VCPU_MEM_SIZE,
		.nr_vcpus = 1,
	};
	int page_idle_fd;
	int opt;

	guest_modes_append_default();

	while ((opt = getopt(argc, argv, "hm:b:v:os:w:")) != -1) {
		switch (opt) {
		case 'm':
			guest_modes_cmdline(optarg);
			break;
		case 'b':
			params.vcpu_memory_bytes = parse_size(optarg);
			break;
		case 'v':
			params.nr_vcpus = atoi_positive("Number of vCPUs", optarg);
			break;
		case 'o':
			overlap_memory_access = true;
			break;
		case 's':
			params.backing_src = parse_backing_src_type(optarg);
			break;
		case 'w':
			idle_pages_warn_only =
				atoi_non_negative("Idle pages warning",
						  optarg);
			break;
		case 'h':
		default:
			help(argv[0]);
			break;
		}
	}

	if (idle_pages_warn_only == -1)
		idle_pages_warn_only = access_tracking_unreliable();

	/*
	 * If guest_page_size is larger than the host's page size, the
	 * guest (memstress) will only fault in a subset of the host's pages.
	 */
	total_pages = params.nr_vcpus * params.vcpu_memory_bytes /
		      max_t(uint64_t, memstress_args.guest_page_size, getpagesize());

	if (lru_gen_usable()) {
		bool cg_created = true;
		char *test_cg = NULL;
		int ret;

		puts("Using lru_gen for aging");
		use_lru_gen = true;

		if (cg_find_controller_root(cgroup_root, sizeof(cgroup_root), "memory"))
			ksft_exit_skip("Cannot find memory group controller\n");

		test_cg = cg_name(cgroup_root, TEST_MEMCG_NAME);
		printf("Creating cgroup: %s\n", test_cg);
		if (cg_create(test_cg)) {
			if (errno == EEXIST)
				cg_created = false;
			else
				ksft_exit_skip("could not create new cgroup: %s\n", test_cg);
		}

		/*
		 * This will fork off a new process to run the test within
		 * a new memcg, so we need to properly propagate the return
		 * value up.
		 */
		ret = cg_run(test_cg, &run_test_for_each_guest_mode, &params);
		if (cg_created)
			cg_destroy(test_cg);
		return ret;
	}

	puts("Using page_idle for aging");
	page_idle_fd = __open_path_or_exit("/sys/kernel/mm/page_idle/bitmap", O_RDWR,
					   "Is CONFIG_IDLE_PAGE_TRACKING enabled?");
	close(page_idle_fd);
	run_test_for_each_guest_mode(NULL, &params);
	return 0;
}

Re: [PATCH v3 5/5] KVM: selftests: access_tracking_perf_test: Use MGLRU for access tracking

[James Houghton]

On Mon, Apr 28, 2025 at 9:19 PM Sean Christopherson <seanjc@google.com> wrote:
>
> On Mon, Apr 14, 2025, James Houghton wrote:
> > By using MGLRU's debugfs for invoking test_young() and clear_young(), we
> > avoid page_idle's incompatibility with MGLRU, and we can mark pages as
> > idle (clear_young()) much faster.
> >
> > The ability to use page_idle is left in, as it is useful for kernels
> > that do not have MGLRU built in. If MGLRU is enabled but is not usable
> > (e.g. we can't access the debugfs mount), the test will fail, as
> > page_idle is not compatible with MGLRU.
> >
> > cgroup utility functions have been borrowed so that, when running with
> > MGLRU, we can create a memcg in which to run our test.
> >
> > Other MGLRU-debugfs-specific parsing code has been added to
> > lru_gen_util.{c,h}.
>
> This is not a proper changelog, at least not by upstream KVM standards.  Please
> rewrite it describe the changes being made, using imperative mood/tone.  From
> Documentation/process/maintainer-kvm-x86.rst:
>
>   Changelog
>   ~~~~~~~~~
>   Most importantly, write changelogs using imperative mood and avoid pronouns.

Right. I'll rewrite the changelog properly.

>
> > @@ -354,7 +459,12 @@ static int access_tracking_unreliable(void)
> >               puts("Skipping idle page count sanity check, because NUMA balancing is enabled");
> >               return 1;
> >       }
> > +     return 0;
> > +}
> > 
> > +int run_test_in_cg(const char *cgroup, void *arg)
>
> static

Will change.

>
> > +{
> > +     for_each_guest_mode(run_test, arg);
>
> Having "separate" flows for MGLRU vs. page_idle is unnecessary.  Give the helper
> a more common name and use it for both:
>
> static int run_test_for_each_guest_mode(const char *cgroup, void *arg)
> {
>         for_each_guest_mode(run_test, arg);
>         return 0;
> }

Applied for my next version, thanks.

>
> >       return 0;
> >  }
> > 
> > @@ -372,7 +482,7 @@ static void help(char *name)
> >       printf(" -v: specify the number of vCPUs to run.\n");
> >       printf(" -o: Overlap guest memory accesses instead of partitioning\n"
> >              "     them into a separate region of memory for each vCPU.\n");
> > -     printf(" -w: Control whether the test warns or fails if more than 10%\n"
> > +     printf(" -w: Control whether the test warns or fails if more than 10%%\n"
> >              "     of pages are still seen as idle/old after accessing guest\n"
> >              "     memory.  >0 == warn only, 0 == fail, <0 == auto.  For auto\n"
> >              "     mode, the test fails by default, but switches to warn only\n"
> > @@ -383,6 +493,12 @@ static void help(char *name)
> >       exit(0);
> >  }
> > 
> > +void destroy_cgroup(char *cg)
>
> static.  But this is a pointless wrapper, just delete it.

Will do.

> Using MGLRU on my home box fails.  It's full cgroup v2, and has both
> CONFIG_IDLE_PAGE_TRACKING=y and MGLRU enabled.
>
> ==== Test Assertion Failure ====
>   access_tracking_perf_test.c:244: false
>   pid=114670 tid=114670 errno=17 - File exists
>      1  0x00000000004032a9: find_generation at access_tracking_perf_test.c:244
>      2  0x00000000004032da: lru_gen_mark_memory_idle at access_tracking_perf_test.c:272
>      3  0x00000000004034e4: mark_memory_idle at access_tracking_perf_test.c:391
>      4   (inlined by) run_test at access_tracking_perf_test.c:431
>      5  0x0000000000403d84: for_each_guest_mode at guest_modes.c:96
>      6  0x0000000000402c61: run_test_for_each_guest_mode at access_tracking_perf_test.c:492
>      7  0x000000000041d8e2: cg_run at cgroup_util.c:382
>      8  0x00000000004027fa: main at access_tracking_perf_test.c:572
>      9  0x00007fa1cb629d8f: ?? ??:0
>     10  0x00007fa1cb629e3f: ?? ??:0
>     11  0x00000000004029d4: _start at ??:?
>   Could not find a generation with 90% of guest memory (235929 pages).
>
> Interestingly, if I force the test to use /sys/kernel/mm/page_idle/bitmap, it
> passes.
>
> Please try to reproduce the failure (assuming you haven't already tested that
> exact combination of cgroups v2, MGLRU=y, and CONFIG_IDLE_PAGE_TRACKING=y). I
> don't have bandwidth to dig any further at this time.

Sorry... please see the bottom of this message for a diff that should fix this.
It fixes these bugs:

1.  Tracking generation numbers without hardware Accessed bit management.
    (This is addition of lru_gen_last_gen.)
1.5 It does an initial aging pass so that pages always move to newer
    generations in (or before) the subsequent aging passes. This probably
    isn't needed given the change I made for (1).
2.  Fixes the expected number of pages for guest page sizes > PAGE_SIZE.
    (This is the move of test_pages. test_pages has also been renamed to avoid
    shadowing.)
3.  Fixes an off-by-one error when looking for the generation with the most
    pages. Previously it failed to check the youngest generation, which I think
    is the bug you ran into. (This is the change to lru_gen_util.c.)

It might take a couple tweaks to compile in your tree. (It is just a WIP diff
from when I was applying changes from your feedback, so it contains partial
changes you asked for.

> > +             if (cg_find_unified_root(cgroup_root, sizeof(cgroup_root), NULL))
> > +                     ksft_exit_skip("cgroup v2 isn't mounted\n");
> > +
> > +             new_cg = cg_name(cgroup_root, TEST_MEMCG_NAME);
> > +             printf("Creating cgroup: %s\n", new_cg);
> > +             if (cg_create(new_cg) && errno != EEXIST)
> > +                     ksft_exit_skip("could not create new cgroup: %s\n", new_cg);
> > +
> > +             use_lru_gen = true;
> > +     } else {
> > +             page_idle_fd = open("/sys/kernel/mm/page_idle/bitmap", O_RDWR);
> > +             __TEST_REQUIRE(page_idle_fd >= 0,
> > +                            "Couldn't open /sys/kernel/mm/page_idle/bitmap. "
> > +                            "Is CONFIG_IDLE_PAGE_TRACKING enabled?");
> > +
> > +             close(page_idle_fd);
> > +     }
>
> Splitting the "check" and "execute" into separate if-else statements results in
> some compilers complaining about new_cg possibly being unused.  The compiler is
> probably being a bit stupid, but the code is just as must to blame.  There's zero
> reason to split this in two, just do everything after the idle_pages_warn_only
> and total_pages processing.  Code at the bottom (note, you'll have to rebase on
> my not-yet-posted series, or undo the use of __open_path_or_exit()).

I have applied the below suggestion for the next version of the series. Thanks.

> static int run_test_for_each_guest_mode(const char *cgroup, void *arg)
> {
>         for_each_guest_mode(run_test, arg);
>         return 0;
> }
>
> int main(int argc, char *argv[])
> {
>         struct test_params params = {
>                 .backing_src = DEFAULT_VM_MEM_SRC,
>                 .vcpu_memory_bytes = DEFAULT_PER_VCPU_MEM_SIZE,
>                 .nr_vcpus = 1,
>         };
>         int page_idle_fd;
>         int opt;
>
>         guest_modes_append_default();
>
>         while ((opt = getopt(argc, argv, "hm:b:v:os:w:")) != -1) {
>                 switch (opt) {
>                 case 'm':
>                         guest_modes_cmdline(optarg);
>                         break;
>                 case 'b':
>                         params.vcpu_memory_bytes = parse_size(optarg);
>                         break;
>                 case 'v':
>                         params.nr_vcpus = atoi_positive("Number of vCPUs", optarg);
>                         break;
>                 case 'o':
>                         overlap_memory_access = true;
>                         break;
>                 case 's':
>                         params.backing_src = parse_backing_src_type(optarg);
>                         break;
>                 case 'w':
>                         idle_pages_warn_only =
>                                 atoi_non_negative("Idle pages warning",
>                                                   optarg);
>                         break;
>                 case 'h':
>                 default:
>                         help(argv[0]);
>                         break;
>                 }
>         }
>
>         if (idle_pages_warn_only == -1)
>                 idle_pages_warn_only = access_tracking_unreliable();
>
>         /*
>          * If guest_page_size is larger than the host's page size, the
>          * guest (memstress) will only fault in a subset of the host's pages.
>          */
>         total_pages = params.nr_vcpus * params.vcpu_memory_bytes /
>                       max_t(uint64_t, memstress_args.guest_page_size, getpagesize());
>
>         if (lru_gen_usable()) {
>                 bool cg_created = true;
>                 char *test_cg = NULL;
>                 int ret;
>
>                 puts("Using lru_gen for aging");
>                 use_lru_gen = true;
>
>                 if (cg_find_controller_root(cgroup_root, sizeof(cgroup_root), "memory"))
>                         ksft_exit_skip("Cannot find memory group controller\n");
>
>                 test_cg = cg_name(cgroup_root, TEST_MEMCG_NAME);
>                 printf("Creating cgroup: %s\n", test_cg);
>                 if (cg_create(test_cg)) {
>                         if (errno == EEXIST)
>                                 cg_created = false;
>                         else
>                                 ksft_exit_skip("could not create new cgroup: %s\n", test_cg);
>                 }
>
>                 /*
>                  * This will fork off a new process to run the test within
>                  * a new memcg, so we need to properly propagate the return
>                  * value up.
>                  */
>                 ret = cg_run(test_cg, &run_test_for_each_guest_mode, &params);
>                 if (cg_created)
>                         cg_destroy(test_cg);
>                 return ret;
>         }
>
>         puts("Using page_idle for aging");
>         page_idle_fd = __open_path_or_exit("/sys/kernel/mm/page_idle/bitmap", O_RDWR,
>                                            "Is CONFIG_IDLE_PAGE_TRACKING enabled?");
>         close(page_idle_fd);
>         run_test_for_each_guest_mode(NULL, &params);
>         return 0;
> }

And here is the diff that make the test start working for you:

diff --git a/tools/testing/selftests/kvm/access_tracking_perf_test.c b/tools/testing/selftests/kvm/access_tracking_perf_test.c
index d4ef201b67055..d4ae29c7dfe35 100644
--- a/tools/testing/selftests/kvm/access_tracking_perf_test.c
+++ b/tools/testing/selftests/kvm/access_tracking_perf_test.c
@@ -90,7 +90,10 @@ static int idle_pages_warn_only = -1;
 static bool use_lru_gen;
 
 /* Total number of pages to expect in the memcg after touching everything */
-static long total_pages;
+static long test_pages;
+
+/* Last generation we found the pages in */
+static int lru_gen_last_gen = -1;
 
 struct test_params {
 	/* The backing source for the region of memory. */
@@ -265,11 +268,7 @@ static void lru_gen_mark_memory_idle(struct kvm_vm *vm)
 	struct timespec ts_start;
 	struct timespec ts_elapsed;
 	struct memcg_stats stats;
-	int found_gens[2];
-
-	/* Find current generation the pages lie in. */
-	lru_gen_read_memcg_stats(&stats, TEST_MEMCG_NAME);
-	found_gens[0] = find_generation(&stats, total_pages);
+	int new_gen;
 
 	/* Make a new generation */
 	clock_gettime(CLOCK_MONOTONIC, &ts_start);
@@ -277,23 +276,24 @@ static void lru_gen_mark_memory_idle(struct kvm_vm *vm)
 	ts_elapsed = timespec_elapsed(ts_start);
 
 	/* Check the generation again */
-	found_gens[1] = find_generation(&stats, total_pages);
+	new_gen = find_generation(&stats, test_pages);
 
 	/*
 	 * This function should only be invoked with newly-accessed pages,
 	 * so pages should always move to a newer generation.
 	 */
-	if (found_gens[0] >= found_gens[1]) {
+	if (new_gen <= lru_gen_last_gen) {
 		/* We did not move to a newer generation. */
-		long idle_pages = lru_gen_sum_memcg_stats_for_gen(found_gens[1],
+		long idle_pages = lru_gen_sum_memcg_stats_for_gen(lru_gen_last_gen,
 								  &stats);
 
-		too_many_idle_pages(min_t(long, idle_pages, total_pages),
-				    total_pages, -1);
+		too_many_idle_pages(min_t(long, idle_pages, test_pages),
+				    test_pages, -1);
 	}
 	pr_info("%-30s: %ld.%09lds\n",
 		"Mark memory idle (lru_gen)", ts_elapsed.tv_sec,
 		ts_elapsed.tv_nsec);
+	lru_gen_last_gen = new_gen;
 }
 
 static void assert_ucall(struct kvm_vcpu *vcpu, uint64_t expected_ucall)
@@ -410,6 +410,14 @@ static void run_test(enum vm_guest_mode mode, void *arg)
 	vm = memstress_create_vm(mode, nr_vcpus, params->vcpu_memory_bytes, 1,
 				 params->backing_src, !overlap_memory_access);
 
+	/*
+	 * If guest_page_size is larger than the host's page size, the
+	 * guest (memstress) will only fault in a subset of the host's pages.
+	 */
+	test_pages = params->nr_vcpus * params->vcpu_memory_bytes /
+		      max(memstress_args.guest_page_size,
+			  (uint64_t)getpagesize());
+
 	memstress_start_vcpu_threads(nr_vcpus, vcpu_thread_main);
 
 	pr_info("\n");
@@ -418,9 +426,18 @@ static void run_test(enum vm_guest_mode mode, void *arg)
 	if (use_lru_gen) {
 		struct memcg_stats stats;
 
-		lru_gen_read_memcg_stats(&stats, TEST_MEMCG_NAME);
-		TEST_ASSERT(lru_gen_sum_memcg_stats(&stats) >= total_pages,
-			    "Not all pages accounted for. Was the memcg set up correctly?");
+		/*
+		 * Do a page table scan now. After initial population, aging
+		 * may not cause the pages to move to a newer generation. Do
+		 * an aging pass now so that future aging passes always move
+		 * pages to a newer generation.
+		 */
+		printf("Initial aging pass (lru_gen)\n");
+		lru_gen_do_aging(&stats, TEST_MEMCG_NAME);
+		TEST_ASSERT(lru_gen_sum_memcg_stats(&stats) >= test_pages,
+			    "Not all pages accounted for (looking for %ld). "
+			    "Was the memcg set up correctly?", test_pages);
+		access_memory(vm, nr_vcpus, ACCESS_WRITE, "Re-populating memory");
 	}
 
 	/* As a control, read and write to the populated memory first. */
@@ -496,7 +513,6 @@ static void help(char *name)
 void destroy_cgroup(char *cg)
 {
 	printf("Destroying cgroup: %s\n", cg);
-	cg_destroy(cg);
 }
 
 int main(int argc, char *argv[])
@@ -541,50 +557,48 @@ int main(int argc, char *argv[])
 		}
 	}
 
-	if (lru_gen_usable()) {
-		if (cg_find_unified_root(cgroup_root, sizeof(cgroup_root), NULL))
-			ksft_exit_skip("cgroup v2 isn't mounted\n");
-
-		new_cg = cg_name(cgroup_root, TEST_MEMCG_NAME);
-		printf("Creating cgroup: %s\n", new_cg);
-		if (cg_create(new_cg) && errno != EEXIST)
-			ksft_exit_skip("could not create new cgroup: %s\n", new_cg);
-
-		use_lru_gen = true;
-	} else {
-		page_idle_fd = open("/sys/kernel/mm/page_idle/bitmap", O_RDWR);
-		__TEST_REQUIRE(page_idle_fd >= 0,
-			       "Couldn't open /sys/kernel/mm/page_idle/bitmap. "
-			       "Is CONFIG_IDLE_PAGE_TRACKING enabled?");
-
-		close(page_idle_fd);
-	}
-
 	if (idle_pages_warn_only == -1)
 		idle_pages_warn_only = access_tracking_unreliable();
 
-	/*
-	 * If guest_page_size is larger than the host's page size, the
-	 * guest (memstress) will only fault in a subset of the host's pages.
-	 */
-	total_pages = params.nr_vcpus * params.vcpu_memory_bytes /
-		      max(memstress_args.guest_page_size,
-			  (uint64_t)getpagesize());
-
-	if (use_lru_gen) {
+	if (lru_gen_usable()) {
+		bool cg_created = true;
 		int ret;
 
 		puts("Using lru_gen for aging");
+		use_lru_gen = true;
+
+		if (cg_find_controller_root(cgroup_root, sizeof(cgroup_root), "memory"))
+			ksft_exit_skip("Cannot find memory cgroup controller\n");
+
+		new_cg = cg_name(cgroup_root, TEST_MEMCG_NAME);
+		printf("Creating cgroup: %s\n", new_cg);
+		if (cg_create(new_cg)) {
+			if (errno == EEXIST) {
+				printf("Found existing cgroup");
+				cg_created = false;
+			}
+			else
+				ksft_exit_skip("could not create new cgroup: %s\n", new_cg);
+		}
+
 		/*
 		 * This will fork off a new process to run the test within
 		 * a new memcg, so we need to properly propagate the return
 		 * value up.
 		 */
 		ret = cg_run(new_cg, &run_test_in_cg, &params);
-		destroy_cgroup(new_cg);
+		if (cg_created)
+			cg_destroy(new_cg);
 		if (ret)
 			return ret;
 	} else {
+		page_idle_fd = open("/sys/kernel/mm/page_idle/bitmap", O_RDWR);
+		__TEST_REQUIRE(page_idle_fd >= 0,
+			       "Couldn't open /sys/kernel/mm/page_idle/bitmap. "
+			       "Is CONFIG_IDLE_PAGE_TRACKING enabled?");
+
+		close(page_idle_fd);
+
 		puts("Using page_idle for aging");
 		for_each_guest_mode(run_test, &params);
 	}
diff --git a/tools/testing/selftests/kvm/lib/lru_gen_util.c b/tools/testing/selftests/kvm/lib/lru_gen_util.c
index 783a1f1028a26..cab54935b160a 100644
--- a/tools/testing/selftests/kvm/lib/lru_gen_util.c
+++ b/tools/testing/selftests/kvm/lib/lru_gen_util.c
@@ -341,7 +341,7 @@ int lru_gen_find_generation(const struct memcg_stats *stats,
 			min_gen = gen < min_gen ? gen : min_gen;
 		}
 
-	for (gen = min_gen; gen < max_gen; ++gen)
+	for (gen = min_gen; gen <= max_gen; ++gen)
 		/* See if this generation has enough pages. */
 		if (lru_gen_sum_memcg_stats_for_gen(gen, stats) > pages)
 			return gen;

Re: [PATCH v3 5/5] KVM: selftests: access_tracking_perf_test: Use MGLRU for access tracking

[Sean Christopherson]

On Tue, Apr 29, 2025, James Houghton wrote:
> On Mon, Apr 28, 2025 at 9:19 PM Sean Christopherson <seanjc@google.com> wrote:
> > Using MGLRU on my home box fails.  It's full cgroup v2, and has both
> > CONFIG_IDLE_PAGE_TRACKING=y and MGLRU enabled.
> >
> > ==== Test Assertion Failure ====
> >   access_tracking_perf_test.c:244: false
> >   pid=114670 tid=114670 errno=17 - File exists
> >      1  0x00000000004032a9: find_generation at access_tracking_perf_test.c:244
> >      2  0x00000000004032da: lru_gen_mark_memory_idle at access_tracking_perf_test.c:272
> >      3  0x00000000004034e4: mark_memory_idle at access_tracking_perf_test.c:391
> >      4   (inlined by) run_test at access_tracking_perf_test.c:431
> >      5  0x0000000000403d84: for_each_guest_mode at guest_modes.c:96
> >      6  0x0000000000402c61: run_test_for_each_guest_mode at access_tracking_perf_test.c:492
> >      7  0x000000000041d8e2: cg_run at cgroup_util.c:382
> >      8  0x00000000004027fa: main at access_tracking_perf_test.c:572
> >      9  0x00007fa1cb629d8f: ?? ??:0
> >     10  0x00007fa1cb629e3f: ?? ??:0
> >     11  0x00000000004029d4: _start at ??:?
> >   Could not find a generation with 90% of guest memory (235929 pages).
> >
> > Interestingly, if I force the test to use /sys/kernel/mm/page_idle/bitmap, it
> > passes.
> >
> > Please try to reproduce the failure (assuming you haven't already tested that
> > exact combination of cgroups v2, MGLRU=y, and CONFIG_IDLE_PAGE_TRACKING=y). I
> > don't have bandwidth to dig any further at this time.
> 
> Sorry... please see the bottom of this message for a diff that should fix this.
> It fixes these bugs:
> 
> 1.  Tracking generation numbers without hardware Accessed bit management.
>     (This is addition of lru_gen_last_gen.)
> 1.5 It does an initial aging pass so that pages always move to newer
>     generations in (or before) the subsequent aging passes. This probably
>     isn't needed given the change I made for (1).
> 2.  Fixes the expected number of pages for guest page sizes > PAGE_SIZE.
>     (This is the move of test_pages. test_pages has also been renamed to avoid
>     shadowing.)
> 3.  Fixes an off-by-one error when looking for the generation with the most
>     pages. Previously it failed to check the youngest generation, which I think
>     is the bug you ran into. (This is the change to lru_gen_util.c.)

Ya, this was the bug I initially ran into, I also encountered more failues after
applying just that fix.  But, with the full diff applied, it's passing, so good
to go for the next version from my end.