Re: [PATCH] KVM: selftests: Add a requirement for disabling numa balancing

[Tao Su <tao1.su@linux.intel.com>]

On Wed, Jan 17, 2024 at 07:12:08AM -0800, Sean Christopherson wrote:

[...]

> > diff --git a/tools/testing/selftests/kvm/x86_64/dirty_log_page_splitting_test.c b/tools/testing/selftests/kvm/x86_64/dirty_log_page_splitting_test.c
> > index 634c6bfcd572..f2c796111d83 100644
> > --- a/tools/testing/selftests/kvm/x86_64/dirty_log_page_splitting_test.c
> > +++ b/tools/testing/selftests/kvm/x86_64/dirty_log_page_splitting_test.c
> > @@ -212,10 +212,21 @@ static void help(char *name)
> >  
> >  int main(int argc, char *argv[])
> >  {
> > +	FILE *f;
> >  	int opt;
> > +	int ret, numa_balancing;
> >  
> >  	TEST_REQUIRE(get_kvm_param_bool("eager_page_split"));
> >  	TEST_REQUIRE(get_kvm_param_bool("tdp_mmu"));
> > +	f = fopen("/proc/sys/kernel/numa_balancing", "r");
> > +	if (f) {
> > +		ret = fscanf(f, "%d", &numa_balancing);
> > +		TEST_ASSERT(ret == 1, "Error reading numa_balancing");
> > +		TEST_ASSERT(!numa_balancing, "please run "
> > +			    "'echo 0 > /proc/sys/kernel/numa_balancing'");
> 
> If we go this route, this should be a TEST_REQUIRE(), not a TEST_ASSERT().  The
> test hasn't failed, rather it has detected an incompatible setup.

Yes, previously I wanted to print a more user-friendly prompt, but TEST_REQUIRE()
can’t customize the output…

> 
> Something isn't right though.  The test defaults to HugeTLB, and the invocation
> in the changelog doesn't override the backing source.  That suggests that NUMA
> auto-balancing is zapping HugeTLB VMAs, which AFAIK shouldn't happen, e.g. this
> code in task_numa_work() should cause such VMAs to be skipped:
> 
> 		if (!vma_migratable(vma) || !vma_policy_mof(vma) ||
> 			is_vm_hugetlb_page(vma) || (vma->vm_flags & VM_MIXEDMAP)) {
> 			trace_sched_skip_vma_numa(mm, vma, NUMAB_SKIP_UNSUITABLE);
> 			continue;
> 		}
> 
> And the test already warns the user if they opt to use something other than
> HugeTLB.
> 
> 	if (!is_backing_src_hugetlb(backing_src)) {
> 		pr_info("This test will only work reliably with HugeTLB memory. "
> 			"It can work with THP, but that is best effort.\n");
> 	}
> 
> If the test is defaulting to something other than HugeTLB, then we should fix
> that in the test.  If the kernel is doing NUMA balancing on HugeTLB VMAs, then
> we should fix that in the kernel.

HugeTLB VMAs are not affected by NUMA auto-balancing through my observation, but
the backing sources of the test code and per-vCPU stacks are not Huge TLB, e.g.
__vm_create() invokes

        vm_userspace_mem_region_add(vm, VM_MEM_SRC_ANONYMOUS, 0, 0, nr_pages, 0);

So, some pages are possible to be migrated.

In dirty_log_page_splitting_test, if sleep 3s before enabling dirty logging:

        --- a/tools/testing/selftests/kvm/x86_64/dirty_log_page_splitting_test.c
        +++ b/tools/testing/selftests/kvm/x86_64/dirty_log_page_splitting_test.c
        @@ -124,6 +124,7 @@ static void run_test(enum vm_guest_mode mode, void *unused)
                memstress_start_vcpu_threads(VCPUS, vcpu_worker);

                run_vcpu_iteration(vm);
        +       sleep(3);
                get_page_stats(vm, &stats_populated, "populating memory");

                /* Enable dirty logging */

I got these logs:

        # ./x86_64/dirty_log_page_splitting_test
        Testing guest mode: PA-bits:ANY, VA-bits:48,  4K pages
        __vm_create: mode='PA-bits:ANY, VA-bits:48,  4K pages' pages='2710'
        Guest physical address width detected: 52
        guest physical test memory: [0xfffff7fe00000, 0xfffffffe00000)
        Added VCPU 0 with test mem gpa [fffff7fe00000, fffffffe00000)
        Added VCPU 1 with test mem gpa [fffff7fe00000, fffffffe00000)

        Page stats after populating memory: 4K: 0 2M: 1024 1G: 0 huge: 1024

        Page stats after enabling dirty logging: 4K: 524288 2M: 0 1G: 0 huge: 0

        Page stats after dirtying memory: 4K: 525334 2M: 0 1G: 0 huge: 0

        Page stats after dirtying memory: 4K: 525334 2M: 0 1G: 0 huge: 0

        Page stats after disabling dirty logging: 4K: 1046 2M: 0 1G: 0 huge: 0

        Page stats after repopulating memory: 4K: 1046 2M: 1024 1G: 0 huge: 1024
        ==== Test Assertion Failure ====
          x86_64/dirty_log_page_splitting_test.c:196: stats_populated.pages_4k == stats_repopulated.pages_4k
          pid=2660413 tid=2660413 errno=0 - Success
             1  0x0000000000402d4b: run_test at dirty_log_page_splitting_test.c:196 (discriminator 1)
             2  0x0000000000403724: for_each_guest_mode at guest_modes.c:100
             3  0x00000000004024ef: main at dirty_log_page_splitting_test.c:246
             4  0x00007fd72c229d8f: ?? ??:0
             5  0x00007fd72c229e3f: ?? ??:0
             6  0x00000000004025b4: _start at ??:?
          0 != 0x416 (stats_populated.pages_4k != stats_repopulated.pages_4k)

Thanks,
Tao