On Tue, 2025-03-25 at 11:01 -0700, James Houghton wrote:
On Mon, Mar 24, 2025 at 6:57 PM Maxim Levitsky mlevitsk@redhat.com wrote:
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
Thanks Maxim! I'm still working on a respin of this test with MGLRU integration, like [1]. Sorry it's taking me so long. I'll apply my changes on top of yours.
.../selftests/kvm/access_tracking_perf_test.c | 33 ++++++++++++++++--- .../testing/selftests/kvm/include/test_util.h | 1 + tools/testing/selftests/kvm/lib/test_util.c | 7 ++++ 3 files changed, 37 insertions(+), 4 deletions(-)
diff --git a/tools/testing/selftests/kvm/access_tracking_perf_test.c b/tools/testing/selftests/kvm/access_tracking_perf_test.c index 3c7defd34f56..6d50c829f00c 100644 --- a/tools/testing/selftests/kvm/access_tracking_perf_test.c +++ b/tools/testing/selftests/kvm/access_tracking_perf_test.c @@ -65,6 +65,8 @@ static int vcpu_last_completed_iteration[KVM_MAX_VCPUS]; /* Whether to overlap the regions of memory vCPUs access. */ static bool overlap_memory_access;
+static int warn_on_too_many_idle_pages = -1;
struct test_params { /* The backing source for the region of memory. */ enum vm_mem_backing_src_type backing_src; @@ -184,11 +186,10 @@ static void mark_vcpu_memory_idle(struct kvm_vm *vm, * 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(warn_on_too_many_idle_pages,I think this assertion is flipped (or how warn_on_too_many_idle_pages is being set is flipped, see below).
Yes it is no doubt about.
I didn't notice this when I flipped the meaning of the variable as Sean suggested. Thanks!
Best regards, Maxim Levitsky
"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);@@ -342,6 +343,8 @@ 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: Skip or force enable the check that after dirtying the guest memory, most (90%%) of \n""it is reported as dirty again (0/1)"); backing_src_help("-s"); puts(""); exit(0);@@ -359,7 +362,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 +379,11 @@ int main(int argc, char *argv[]) case 's': params.backing_src = parse_backing_src_type(optarg); break;
case 'w':warn_on_too_many_idle_pages =atoi_non_negative("1 - enable warning, 0 - disable",optarg);We still get a "warning" either way, right? Maybe this should be called "fail_on_too_many_idle_pages" (in which case the above assertion is indeed flipped). Or "warn_on_too_many_idle_pages" should mean *only* warn, i.e., *don't* fail, in which case, below we need to flip how we set it below.
break; case 'h': default: help(argv[0]);@@ -386,6 +394,23 @@ 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");
if (warn_on_too_many_idle_pages == -1) {+#ifdef __x86_64__
if (this_cpu_has(X86_FEATURE_HYPERVISOR)) {printf("Skipping idle page count sanity check, because the test is run nested\n");warn_on_too_many_idle_pages = 0;} else+#endif
if (is_numa_balancing_enabled()) {printf("Skipping idle page count sanity check, because NUMA balance is enabled\n");warn_on_too_many_idle_pages = 0;} else {warn_on_too_many_idle_pages = 1;}} else if (!warn_on_too_many_idle_pages) {printf("Skipping idle page count sanity check, because this was requested by the user\n");}close(page_idle_fd); for_each_guest_mode(run_test, ¶ms);diff --git a/tools/testing/selftests/kvm/include/test_util.h b/tools/testing/selftests/kvm/include/test_util.h index 3e473058849f..1bc9b0a92427 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 3dc8538f5d69..03eb99af9b8d 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.26.3