diff mbox series

[RFC,v2,6/7] KVM: selftests: Adapt vm_userspace_mem_region_add to new helpers

Message ID 20210225055940.18748-7-wangyanan55@huawei.com (mailing list archive)
State New, archived
Headers show
Series Some improvement and a new test for kvm page table | expand

Commit Message

Yanan Wang Feb. 25, 2021, 5:59 a.m. UTC 
With VM_MEM_SRC_ANONYMOUS_THP specified in vm_userspace_mem_region_add(),
we have to get the transparent hugepage size for HVA alignment. With the
new helpers, we can use get_backing_src_pagesz() to check whether THP is
configured and then get the exact configured hugepage size.

As different architectures may have different THP page sizes configured,
this can get the accurate THP page sizes on any platform.

Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
---
 tools/testing/selftests/kvm/lib/kvm_util.c | 27 +++++++---------------
 1 file changed, 8 insertions(+), 19 deletions(-)

Comments

Ben Gardon Feb. 25, 2021, 11:44 p.m. UTC | #1 
On Wed, Feb 24, 2021 at 10:03 PM Yanan Wang <wangyanan55@huawei.com> wrote:
>
> With VM_MEM_SRC_ANONYMOUS_THP specified in vm_userspace_mem_region_add(),
> we have to get the transparent hugepage size for HVA alignment. With the
> new helpers, we can use get_backing_src_pagesz() to check whether THP is
> configured and then get the exact configured hugepage size.
>
> As different architectures may have different THP page sizes configured,
> this can get the accurate THP page sizes on any platform.
>
> Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
> ---
>  tools/testing/selftests/kvm/lib/kvm_util.c | 27 +++++++---------------
>  1 file changed, 8 insertions(+), 19 deletions(-)
>
> diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c
> index b91c8e3a7ee1..0105fbfed036 100644
> --- a/tools/testing/selftests/kvm/lib/kvm_util.c
> +++ b/tools/testing/selftests/kvm/lib/kvm_util.c
> @@ -18,7 +18,6 @@
>  #include <unistd.h>
>  #include <linux/kernel.h>
>
> -#define KVM_UTIL_PGS_PER_HUGEPG 512
>  #define KVM_UTIL_MIN_PFN       2
>
>  /* Aligns x up to the next multiple of size. Size must be a power of 2. */
> @@ -686,7 +685,7 @@ void vm_userspace_mem_region_add(struct kvm_vm *vm,
>  {
>         int ret;
>         struct userspace_mem_region *region;
> -       size_t huge_page_size = KVM_UTIL_PGS_PER_HUGEPG * vm->page_size;
> +       size_t backing_src_pagesz = get_backing_src_pagesz(src_type);
>         size_t alignment;
>
>         TEST_ASSERT(vm_adjust_num_guest_pages(vm->mode, npages) == npages,
> @@ -748,7 +747,7 @@ void vm_userspace_mem_region_add(struct kvm_vm *vm,
>  #endif
>
>         if (src_type == VM_MEM_SRC_ANONYMOUS_THP)
> -               alignment = max(huge_page_size, alignment);
> +               alignment = max(backing_src_pagesz, alignment);
>
>         /* Add enough memory to align up if necessary */
>         if (alignment > 1)
> @@ -767,22 +766,12 @@ void vm_userspace_mem_region_add(struct kvm_vm *vm,
>         region->host_mem = align(region->mmap_start, alignment);
>
>         /* As needed perform madvise */
> -       if (src_type == VM_MEM_SRC_ANONYMOUS || src_type == VM_MEM_SRC_ANONYMOUS_THP) {
> -               struct stat statbuf;
> -
> -               ret = stat("/sys/kernel/mm/transparent_hugepage", &statbuf);
> -               TEST_ASSERT(ret == 0 || (ret == -1 && errno == ENOENT),
> -                           "stat /sys/kernel/mm/transparent_hugepage");
> -
> -               TEST_ASSERT(ret == 0 || src_type != VM_MEM_SRC_ANONYMOUS_THP,
> -                           "VM_MEM_SRC_ANONYMOUS_THP requires THP to be configured in the host kernel");
> -
> -               if (ret == 0) {
> -                       ret = madvise(region->host_mem, npages * vm->page_size,
> -                                     src_type == VM_MEM_SRC_ANONYMOUS ? MADV_NOHUGEPAGE : MADV_HUGEPAGE);
> -                       TEST_ASSERT(ret == 0, "madvise failed, addr: %p length: 0x%lx src_type: %x",
> -                                   region->host_mem, npages * vm->page_size, src_type);
> -               }
> +       if (src_type <= VM_MEM_SRC_ANONYMOUS_THP && thp_configured()) {

This check relies on an unstated property of the backing src type
enums where VM_MEM_SRC_ANONYMOUS and VM_MEM_SRC_ANONYMOUS_THP are
declared first.
It would probably be more readable for folks if the check was explicit:
if ((src_type == VM_MEM_SRC_ANONYMOUS || src_type ==
VM_MEM_SRC_ANONYMOUS_THP) && thp_configured()) {


> +               ret = madvise(region->host_mem, npages * vm->page_size,
> +                             src_type == VM_MEM_SRC_ANONYMOUS ? MADV_NOHUGEPAGE : MADV_HUGEPAGE);
> +               TEST_ASSERT(ret == 0, "madvise failed, addr: %p length: 0x%lx src_type: %s",
> +                           region->host_mem, npages * vm->page_size,
> +                           vm_mem_backing_src_alias(src_type)->name);
>         }
>
>         region->unused_phy_pages = sparsebit_alloc();
> --
> 2.19.1
>
Yanan Wang Feb. 26, 2021, 5:48 a.m. UTC | #2 
On 2021/2/26 7:44, Ben Gardon wrote:
> On Wed, Feb 24, 2021 at 10:03 PM Yanan Wang <wangyanan55@huawei.com> wrote:
>> With VM_MEM_SRC_ANONYMOUS_THP specified in vm_userspace_mem_region_add(),
>> we have to get the transparent hugepage size for HVA alignment. With the
>> new helpers, we can use get_backing_src_pagesz() to check whether THP is
>> configured and then get the exact configured hugepage size.
>>
>> As different architectures may have different THP page sizes configured,
>> this can get the accurate THP page sizes on any platform.
>>
>> Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
>> ---
>>   tools/testing/selftests/kvm/lib/kvm_util.c | 27 +++++++---------------
>>   1 file changed, 8 insertions(+), 19 deletions(-)
>>
>> diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c
>> index b91c8e3a7ee1..0105fbfed036 100644
>> --- a/tools/testing/selftests/kvm/lib/kvm_util.c
>> +++ b/tools/testing/selftests/kvm/lib/kvm_util.c
>> @@ -18,7 +18,6 @@
>>   #include <unistd.h>
>>   #include <linux/kernel.h>
>>
>> -#define KVM_UTIL_PGS_PER_HUGEPG 512
>>   #define KVM_UTIL_MIN_PFN       2
>>
>>   /* Aligns x up to the next multiple of size. Size must be a power of 2. */
>> @@ -686,7 +685,7 @@ void vm_userspace_mem_region_add(struct kvm_vm *vm,
>>   {
>>          int ret;
>>          struct userspace_mem_region *region;
>> -       size_t huge_page_size = KVM_UTIL_PGS_PER_HUGEPG * vm->page_size;
>> +       size_t backing_src_pagesz = get_backing_src_pagesz(src_type);
>>          size_t alignment;
>>
>>          TEST_ASSERT(vm_adjust_num_guest_pages(vm->mode, npages) == npages,
>> @@ -748,7 +747,7 @@ void vm_userspace_mem_region_add(struct kvm_vm *vm,
>>   #endif
>>
>>          if (src_type == VM_MEM_SRC_ANONYMOUS_THP)
>> -               alignment = max(huge_page_size, alignment);
>> +               alignment = max(backing_src_pagesz, alignment);
>>
>>          /* Add enough memory to align up if necessary */
>>          if (alignment > 1)
>> @@ -767,22 +766,12 @@ void vm_userspace_mem_region_add(struct kvm_vm *vm,
>>          region->host_mem = align(region->mmap_start, alignment);
>>
>>          /* As needed perform madvise */
>> -       if (src_type == VM_MEM_SRC_ANONYMOUS || src_type == VM_MEM_SRC_ANONYMOUS_THP) {
>> -               struct stat statbuf;
>> -
>> -               ret = stat("/sys/kernel/mm/transparent_hugepage", &statbuf);
>> -               TEST_ASSERT(ret == 0 || (ret == -1 && errno == ENOENT),
>> -                           "stat /sys/kernel/mm/transparent_hugepage");
>> -
>> -               TEST_ASSERT(ret == 0 || src_type != VM_MEM_SRC_ANONYMOUS_THP,
>> -                           "VM_MEM_SRC_ANONYMOUS_THP requires THP to be configured in the host kernel");
>> -
>> -               if (ret == 0) {
>> -                       ret = madvise(region->host_mem, npages * vm->page_size,
>> -                                     src_type == VM_MEM_SRC_ANONYMOUS ? MADV_NOHUGEPAGE : MADV_HUGEPAGE);
>> -                       TEST_ASSERT(ret == 0, "madvise failed, addr: %p length: 0x%lx src_type: %x",
>> -                                   region->host_mem, npages * vm->page_size, src_type);
>> -               }
>> +       if (src_type <= VM_MEM_SRC_ANONYMOUS_THP && thp_configured()) {
> This check relies on an unstated property of the backing src type
> enums where VM_MEM_SRC_ANONYMOUS and VM_MEM_SRC_ANONYMOUS_THP are
> declared first.
> It would probably be more readable for folks if the check was explicit:
> if ((src_type == VM_MEM_SRC_ANONYMOUS || src_type ==
> VM_MEM_SRC_ANONYMOUS_THP) && thp_configured()) {
>
Yes, this makes sense, I will fix it.

Thanks,

Yanan

>> +               ret = madvise(region->host_mem, npages * vm->page_size,
>> +                             src_type == VM_MEM_SRC_ANONYMOUS ? MADV_NOHUGEPAGE : MADV_HUGEPAGE);
>> +               TEST_ASSERT(ret == 0, "madvise failed, addr: %p length: 0x%lx src_type: %s",
>> +                           region->host_mem, npages * vm->page_size,
>> +                           vm_mem_backing_src_alias(src_type)->name);
>>          }
>>
>>          region->unused_phy_pages = sparsebit_alloc();
>> --
>> 2.19.1
>>
> .
diff mbox series

Patch

diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c
index b91c8e3a7ee1..0105fbfed036 100644
--- a/tools/testing/selftests/kvm/lib/kvm_util.c
+++ b/tools/testing/selftests/kvm/lib/kvm_util.c
@@ -18,7 +18,6 @@ 
 #include <unistd.h>
 #include <linux/kernel.h>
 
-#define KVM_UTIL_PGS_PER_HUGEPG 512
 #define KVM_UTIL_MIN_PFN	2
 
 /* Aligns x up to the next multiple of size. Size must be a power of 2. */
@@ -686,7 +685,7 @@  void vm_userspace_mem_region_add(struct kvm_vm *vm,
 {
 	int ret;
 	struct userspace_mem_region *region;
-	size_t huge_page_size = KVM_UTIL_PGS_PER_HUGEPG * vm->page_size;
+	size_t backing_src_pagesz = get_backing_src_pagesz(src_type);
 	size_t alignment;
 
 	TEST_ASSERT(vm_adjust_num_guest_pages(vm->mode, npages) == npages,
@@ -748,7 +747,7 @@  void vm_userspace_mem_region_add(struct kvm_vm *vm,
 #endif
 
 	if (src_type == VM_MEM_SRC_ANONYMOUS_THP)
-		alignment = max(huge_page_size, alignment);
+		alignment = max(backing_src_pagesz, alignment);
 
 	/* Add enough memory to align up if necessary */
 	if (alignment > 1)
@@ -767,22 +766,12 @@  void vm_userspace_mem_region_add(struct kvm_vm *vm,
 	region->host_mem = align(region->mmap_start, alignment);
 
 	/* As needed perform madvise */
-	if (src_type == VM_MEM_SRC_ANONYMOUS || src_type == VM_MEM_SRC_ANONYMOUS_THP) {
-		struct stat statbuf;
-
-		ret = stat("/sys/kernel/mm/transparent_hugepage", &statbuf);
-		TEST_ASSERT(ret == 0 || (ret == -1 && errno == ENOENT),
-			    "stat /sys/kernel/mm/transparent_hugepage");
-
-		TEST_ASSERT(ret == 0 || src_type != VM_MEM_SRC_ANONYMOUS_THP,
-			    "VM_MEM_SRC_ANONYMOUS_THP requires THP to be configured in the host kernel");
-
-		if (ret == 0) {
-			ret = madvise(region->host_mem, npages * vm->page_size,
-				      src_type == VM_MEM_SRC_ANONYMOUS ? MADV_NOHUGEPAGE : MADV_HUGEPAGE);
-			TEST_ASSERT(ret == 0, "madvise failed, addr: %p length: 0x%lx src_type: %x",
-				    region->host_mem, npages * vm->page_size, src_type);
-		}
+	if (src_type <= VM_MEM_SRC_ANONYMOUS_THP && thp_configured()) {
+		ret = madvise(region->host_mem, npages * vm->page_size,
+			      src_type == VM_MEM_SRC_ANONYMOUS ? MADV_NOHUGEPAGE : MADV_HUGEPAGE);
+		TEST_ASSERT(ret == 0, "madvise failed, addr: %p length: 0x%lx src_type: %s",
+			    region->host_mem, npages * vm->page_size,
+			    vm_mem_backing_src_alias(src_type)->name);
 	}
 
 	region->unused_phy_pages = sparsebit_alloc();