kvm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] KVM: selftests: Speed up set_memory_region_test
@ 2021-04-26 13:01 Vitaly Kuznetsov
  2021-06-23  9:03 ` Zenghui Yu
  0 siblings, 1 reply; 4+ messages in thread
From: Vitaly Kuznetsov @ 2021-04-26 13:01 UTC (permalink / raw)
  To: kvm, Paolo Bonzini
  Cc: Sean Christopherson, Wanpeng Li, Jim Mattson, kernel test robot,
	linux-kernel

After commit 4fc096a99e01 ("KVM: Raise the maximum number of user memslots")
set_memory_region_test may take too long, reports are that the default
timeout value we have (120s) may not be enough even on a physical host.

Speed things up a bit by throwing away vm_userspace_mem_region_add() usage
from test_add_max_memory_regions(), we don't really need to do the majority
of the stuff it does for the sake of this test.

On my AMD EPYC 7401P, # time ./set_memory_region_test
pre-patch:
 Testing KVM_RUN with zero added memory regions
 Allowed number of memory slots: 32764
 Adding slots 0..32763, each memory region with 2048K size
 Testing MOVE of in-use region, 10 loops
 Testing DELETE of in-use region, 10 loops

 real	0m44.917s
 user	0m7.416s
 sys	0m34.601s

post-patch:
 Testing KVM_RUN with zero added memory regions
 Allowed number of memory slots: 32764
 Adding slots 0..32763, each memory region with 2048K size
 Testing MOVE of in-use region, 10 loops
 Testing DELETE of in-use region, 10 loops

 real	0m20.714s
 user	0m0.109s
 sys	0m18.359s

Reported-by: kernel test robot <oliver.sang@intel.com>
Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
 .../selftests/kvm/set_memory_region_test.c    | 61 ++++++++++++++-----
 1 file changed, 45 insertions(+), 16 deletions(-)

diff --git a/tools/testing/selftests/kvm/set_memory_region_test.c b/tools/testing/selftests/kvm/set_memory_region_test.c
index f127ed31dba7..978f5b5f4dc0 100644
--- a/tools/testing/selftests/kvm/set_memory_region_test.c
+++ b/tools/testing/selftests/kvm/set_memory_region_test.c
@@ -329,6 +329,22 @@ static void test_zero_memory_regions(void)
 }
 #endif /* __x86_64__ */
 
+static int test_memory_region_add(struct kvm_vm *vm, void *mem, uint32_t slot,
+				   uint32_t size, uint64_t guest_addr)
+{
+	struct kvm_userspace_memory_region region;
+	int ret;
+
+	region.slot = slot;
+	region.flags = 0;
+	region.guest_phys_addr = guest_addr;
+	region.memory_size = size;
+	region.userspace_addr = (uintptr_t) mem;
+	ret = ioctl(vm_get_fd(vm), KVM_SET_USER_MEMORY_REGION, &region);
+
+	return ret;
+}
+
 /*
  * Test it can be added memory slots up to KVM_CAP_NR_MEMSLOTS, then any
  * tentative to add further slots should fail.
@@ -339,9 +355,15 @@ static void test_add_max_memory_regions(void)
 	struct kvm_vm *vm;
 	uint32_t max_mem_slots;
 	uint32_t slot;
-	uint64_t guest_addr = 0x0;
-	uint64_t mem_reg_npages;
-	void *mem;
+	void *mem, *mem_aligned, *mem_extra;
+	size_t alignment;
+
+#ifdef __s390x__
+	/* On s390x, the host address must be aligned to 1M (due to PGSTEs) */
+	alignment = 0x100000;
+#else
+	alignment = 1;
+#endif
 
 	max_mem_slots = kvm_check_cap(KVM_CAP_NR_MEMSLOTS);
 	TEST_ASSERT(max_mem_slots > 0,
@@ -350,30 +372,37 @@ static void test_add_max_memory_regions(void)
 
 	vm = vm_create(VM_MODE_DEFAULT, 0, O_RDWR);
 
-	mem_reg_npages = vm_calc_num_guest_pages(VM_MODE_DEFAULT, MEM_REGION_SIZE);
-
 	/* Check it can be added memory slots up to the maximum allowed */
 	pr_info("Adding slots 0..%i, each memory region with %dK size\n",
 		(max_mem_slots - 1), MEM_REGION_SIZE >> 10);
+
+	mem = mmap(NULL, MEM_REGION_SIZE * max_mem_slots + alignment,
+		   PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+	TEST_ASSERT(mem != MAP_FAILED, "Failed to mmap() host");
+	mem_aligned = (void *)(((size_t) mem + alignment - 1) & ~(alignment - 1));
+
 	for (slot = 0; slot < max_mem_slots; slot++) {
-		vm_userspace_mem_region_add(vm, VM_MEM_SRC_ANONYMOUS,
-					    guest_addr, slot, mem_reg_npages,
-					    0);
-		guest_addr += MEM_REGION_SIZE;
+		ret = test_memory_region_add(vm, mem_aligned +
+					     ((uint64_t)slot * MEM_REGION_SIZE),
+					     slot, MEM_REGION_SIZE,
+					     (uint64_t)slot * MEM_REGION_SIZE);
+		TEST_ASSERT(ret == 0, "KVM_SET_USER_MEMORY_REGION IOCTL failed,\n"
+			    "  rc: %i errno: %i slot: %i\n",
+			    ret, errno, slot);
 	}
 
 	/* Check it cannot be added memory slots beyond the limit */
-	mem = mmap(NULL, MEM_REGION_SIZE, PROT_READ | PROT_WRITE,
-		   MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
-	TEST_ASSERT(mem != MAP_FAILED, "Failed to mmap() host");
+	mem_extra = mmap(NULL, MEM_REGION_SIZE, PROT_READ | PROT_WRITE,
+			 MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+	TEST_ASSERT(mem_extra != MAP_FAILED, "Failed to mmap() host");
 
-	ret = ioctl(vm_get_fd(vm), KVM_SET_USER_MEMORY_REGION,
-		    &(struct kvm_userspace_memory_region) {slot, 0, guest_addr,
-		    MEM_REGION_SIZE, (uint64_t) mem});
+	ret = test_memory_region_add(vm, mem_extra, max_mem_slots, MEM_REGION_SIZE,
+				     (uint64_t)max_mem_slots * MEM_REGION_SIZE);
 	TEST_ASSERT(ret == -1 && errno == EINVAL,
 		    "Adding one more memory slot should fail with EINVAL");
 
-	munmap(mem, MEM_REGION_SIZE);
+	munmap(mem, MEM_REGION_SIZE * max_mem_slots + alignment);
+	munmap(mem_extra, MEM_REGION_SIZE);
 	kvm_vm_free(vm);
 }
 
-- 
2.30.2


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

* Re: [PATCH] KVM: selftests: Speed up set_memory_region_test
  2021-04-26 13:01 [PATCH] KVM: selftests: Speed up set_memory_region_test Vitaly Kuznetsov
@ 2021-06-23  9:03 ` Zenghui Yu
  2021-06-23 21:45   ` Paolo Bonzini
  0 siblings, 1 reply; 4+ messages in thread
From: Zenghui Yu @ 2021-06-23  9:03 UTC (permalink / raw)
  To: Vitaly Kuznetsov
  Cc: kvm, Paolo Bonzini, Sean Christopherson, Wanpeng Li, Jim Mattson,
	kernel test robot, linux-kernel, wanghaibin.wang

On 2021/4/26 21:01, Vitaly Kuznetsov wrote:
> After commit 4fc096a99e01 ("KVM: Raise the maximum number of user memslots")
> set_memory_region_test may take too long, reports are that the default
> timeout value we have (120s) may not be enough even on a physical host.
> 
> Speed things up a bit by throwing away vm_userspace_mem_region_add() usage
> from test_add_max_memory_regions(), we don't really need to do the majority
> of the stuff it does for the sake of this test.
> 
> On my AMD EPYC 7401P, # time ./set_memory_region_test
> pre-patch:
>  Testing KVM_RUN with zero added memory regions
>  Allowed number of memory slots: 32764
>  Adding slots 0..32763, each memory region with 2048K size
>  Testing MOVE of in-use region, 10 loops
>  Testing DELETE of in-use region, 10 loops
> 
>  real	0m44.917s
>  user	0m7.416s
>  sys	0m34.601s
> 
> post-patch:
>  Testing KVM_RUN with zero added memory regions
>  Allowed number of memory slots: 32764
>  Adding slots 0..32763, each memory region with 2048K size
>  Testing MOVE of in-use region, 10 loops
>  Testing DELETE of in-use region, 10 loops
> 
>  real	0m20.714s
>  user	0m0.109s
>  sys	0m18.359s
> 
> Reported-by: kernel test robot <oliver.sang@intel.com>
> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>

I've seen the failure on my arm64 server, # ./set_memory_region_test

Allowed number of memory slots: 32767
Adding slots 0..32766, each memory region with 2048K size
==== Test Assertion Failure ====
   set_memory_region_test.c:391: ret == 0
   pid=42696 tid=42696 errno=22 - Invalid argument
      1	0x00000000004015a7: test_add_max_memory_regions at 
set_memory_region_test.c:389
      2	 (inlined by) main at set_memory_region_test.c:426
      3	0x0000ffffb7c63bdf: ?? ??:0
      4	0x00000000004016db: _start at :?
   KVM_SET_USER_MEMORY_REGION IOCTL failed,
   rc: -1 errno: 22 slot: 2624

> +	mem = mmap(NULL, MEM_REGION_SIZE * max_mem_slots + alignment,

The problem is that max_mem_slots is declared as uint32_t, the result
of (MEM_REGION_SIZE * max_mem_slots) is unexpectedly truncated to be
0xffe00000.

> +		   PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
> +	TEST_ASSERT(mem != MAP_FAILED, "Failed to mmap() host");
> +	mem_aligned = (void *)(((size_t) mem + alignment - 1) & ~(alignment - 1));
> +
>  	for (slot = 0; slot < max_mem_slots; slot++) {
> -		vm_userspace_mem_region_add(vm, VM_MEM_SRC_ANONYMOUS,
> -					    guest_addr, slot, mem_reg_npages,
> -					    0);
> -		guest_addr += MEM_REGION_SIZE;
> +		ret = test_memory_region_add(vm, mem_aligned +
> +					     ((uint64_t)slot * MEM_REGION_SIZE),

These unmapped VAs got caught by access_ok() checker in
__kvm_set_memory_region() as they happen to go beyond the task's
address space on arm64. Casting max_mem_slots to size_t in both
mmap() and munmap() fixes the issue for me.

Thanks,
Zenghui

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

* Re: [PATCH] KVM: selftests: Speed up set_memory_region_test
  2021-06-23  9:03 ` Zenghui Yu
@ 2021-06-23 21:45   ` Paolo Bonzini
  2021-06-24  7:18     ` Zenghui Yu
  0 siblings, 1 reply; 4+ messages in thread
From: Paolo Bonzini @ 2021-06-23 21:45 UTC (permalink / raw)
  To: Zenghui Yu, Vitaly Kuznetsov
  Cc: kvm, Sean Christopherson, Wanpeng Li, Jim Mattson,
	kernel test robot, linux-kernel, wanghaibin.wang

On 23/06/21 11:03, Zenghui Yu wrote:
> On 2021/4/26 21:01, Vitaly Kuznetsov wrote:
>> After commit 4fc096a99e01 ("KVM: Raise the maximum number of user 
>> memslots")
>> set_memory_region_test may take too long, reports are that the default
>> timeout value we have (120s) may not be enough even on a physical host.
>>
>> Speed things up a bit by throwing away vm_userspace_mem_region_add() 
>> usage
>> from test_add_max_memory_regions(), we don't really need to do the 
>> majority
>> of the stuff it does for the sake of this test.
>>
>> On my AMD EPYC 7401P, # time ./set_memory_region_test
>> pre-patch:
>>  Testing KVM_RUN with zero added memory regions
>>  Allowed number of memory slots: 32764
>>  Adding slots 0..32763, each memory region with 2048K size
>>  Testing MOVE of in-use region, 10 loops
>>  Testing DELETE of in-use region, 10 loops
>>
>>  real    0m44.917s
>>  user    0m7.416s
>>  sys    0m34.601s
>>
>> post-patch:
>>  Testing KVM_RUN with zero added memory regions
>>  Allowed number of memory slots: 32764
>>  Adding slots 0..32763, each memory region with 2048K size
>>  Testing MOVE of in-use region, 10 loops
>>  Testing DELETE of in-use region, 10 loops
>>
>>  real    0m20.714s
>>  user    0m0.109s
>>  sys    0m18.359s
>>
>> Reported-by: kernel test robot <oliver.sang@intel.com>
>> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
> 
> I've seen the failure on my arm64 server, # ./set_memory_region_test
> 
> Allowed number of memory slots: 32767
> Adding slots 0..32766, each memory region with 2048K size
> ==== Test Assertion Failure ====
>    set_memory_region_test.c:391: ret == 0
>    pid=42696 tid=42696 errno=22 - Invalid argument
>       1    0x00000000004015a7: test_add_max_memory_regions at 
> set_memory_region_test.c:389
>       2     (inlined by) main at set_memory_region_test.c:426
>       3    0x0000ffffb7c63bdf: ?? ??:0
>       4    0x00000000004016db: _start at :?
>    KVM_SET_USER_MEMORY_REGION IOCTL failed,
>    rc: -1 errno: 22 slot: 2624
> 
>> +    mem = mmap(NULL, MEM_REGION_SIZE * max_mem_slots + alignment,
> 
> The problem is that max_mem_slots is declared as uint32_t, the result
> of (MEM_REGION_SIZE * max_mem_slots) is unexpectedly truncated to be
> 0xffe00000.
> 
>> +           PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
>> +    TEST_ASSERT(mem != MAP_FAILED, "Failed to mmap() host");
>> +    mem_aligned = (void *)(((size_t) mem + alignment - 1) & 
>> ~(alignment - 1));
>> +
>>      for (slot = 0; slot < max_mem_slots; slot++) {
>> -        vm_userspace_mem_region_add(vm, VM_MEM_SRC_ANONYMOUS,
>> -                        guest_addr, slot, mem_reg_npages,
>> -                        0);
>> -        guest_addr += MEM_REGION_SIZE;
>> +        ret = test_memory_region_add(vm, mem_aligned +
>> +                         ((uint64_t)slot * MEM_REGION_SIZE),
> 
> These unmapped VAs got caught by access_ok() checker in
> __kvm_set_memory_region() as they happen to go beyond the task's
> address space on arm64. Casting max_mem_slots to size_t in both
> mmap() and munmap() fixes the issue for me.

Can you provide a patch for both?

Paolo


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

* Re: [PATCH] KVM: selftests: Speed up set_memory_region_test
  2021-06-23 21:45   ` Paolo Bonzini
@ 2021-06-24  7:18     ` Zenghui Yu
  0 siblings, 0 replies; 4+ messages in thread
From: Zenghui Yu @ 2021-06-24  7:18 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Vitaly Kuznetsov, kvm, Sean Christopherson, Wanpeng Li,
	Jim Mattson, kernel test robot, linux-kernel, wanghaibin.wang

On 2021/6/24 5:45, Paolo Bonzini wrote:
> Can you provide a patch for both?

Sent now.

Zenghui

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

end of thread, other threads:[~2021-06-24  7:18 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-26 13:01 [PATCH] KVM: selftests: Speed up set_memory_region_test Vitaly Kuznetsov
2021-06-23  9:03 ` Zenghui Yu
2021-06-23 21:45   ` Paolo Bonzini
2021-06-24  7:18     ` Zenghui Yu

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