Messages in this thread | | | Subject | Re: [RFC PATCH v3 6/8] KVM: selftests: List all hugetlb src types specified with page sizes | From | "wangyanan (Y)" <> | Date | Tue, 2 Mar 2021 20:21:15 +0800 |
| |
Hi Ben,
On 2021/3/2 1:09, Ben Gardon wrote: > On Sun, Feb 28, 2021 at 11:00 PM Yanan Wang <wangyanan55@huawei.com> wrote: >> With VM_MEM_SRC_ANONYMOUS_HUGETLB, we currently can only use system >> default hugetlb pages to back the testing guest memory. In order to >> add flexibility, now list all the known hugetlb backing src types with >> different page sizes, so that we can specify use of hugetlb pages of the >> exact granularity that we want. And as all the known hugetlb page sizes >> are listed, it's appropriate for all architectures. >> >> Besides, the helper get_backing_src_pagesz() is added to get the >> granularity of different backing src types(anonumous, thp, hugetlb). >> >> Suggested-by: Ben Gardon <bgardon@google.com> >> Signed-off-by: Yanan Wang <wangyanan55@huawei.com> >> --- >> .../testing/selftests/kvm/include/test_util.h | 19 ++++++- >> tools/testing/selftests/kvm/lib/kvm_util.c | 2 +- >> tools/testing/selftests/kvm/lib/test_util.c | 56 +++++++++++++++---- >> 3 files changed, 63 insertions(+), 14 deletions(-) >> >> diff --git a/tools/testing/selftests/kvm/include/test_util.h b/tools/testing/selftests/kvm/include/test_util.h >> index ef24c76ba89a..be5d08bcdca7 100644 >> --- a/tools/testing/selftests/kvm/include/test_util.h >> +++ b/tools/testing/selftests/kvm/include/test_util.h >> @@ -70,16 +70,31 @@ struct timespec timespec_div(struct timespec ts, int divisor); >> enum vm_mem_backing_src_type { >> VM_MEM_SRC_ANONYMOUS, >> VM_MEM_SRC_ANONYMOUS_THP, >> - VM_MEM_SRC_ANONYMOUS_HUGETLB, > I apologize I didn't catch this in v2, but it looks like this patch > removes a default hugetlb size option. I could see this being > intentional if we want to force developers to think about there being > multiple page sizes, but it might also be nice for folks to have an > option to use the system default hugepage size. Thanks for pointing out this. I was trying to let developers to use the accurate page size in all cases if they want to use hugetlb pages, so I removed the default enum of VM_MEM_SRC_ANONYMOUS_HUGETLB. But maybe it's not right to do so.
It's possible that a program just wants to use hugetlb pages and doesn't really care the page size and the default option is the best in this case. Anyway, I will add VM_MEM_SRC_ANONYMOUS_HUGETLB in next version :). As for the default hugetlb page size, it can be got by reading file /proc/meminfo. > Otherwise, this series looks good to me. Please feel free to add > Reviewed-by: Ben Gardon <bgardon@google.com>.
Thanks for your review of this series and the good suggestions,
Yanan
>> + VM_MEM_SRC_ANONYMOUS_HUGETLB_16KB, >> + VM_MEM_SRC_ANONYMOUS_HUGETLB_64KB, >> + VM_MEM_SRC_ANONYMOUS_HUGETLB_512KB, >> + VM_MEM_SRC_ANONYMOUS_HUGETLB_1MB, >> + VM_MEM_SRC_ANONYMOUS_HUGETLB_2MB, >> + VM_MEM_SRC_ANONYMOUS_HUGETLB_8MB, >> + VM_MEM_SRC_ANONYMOUS_HUGETLB_16MB, >> + VM_MEM_SRC_ANONYMOUS_HUGETLB_32MB, >> + VM_MEM_SRC_ANONYMOUS_HUGETLB_256MB, >> + VM_MEM_SRC_ANONYMOUS_HUGETLB_512MB, >> + VM_MEM_SRC_ANONYMOUS_HUGETLB_1GB, >> + VM_MEM_SRC_ANONYMOUS_HUGETLB_2GB, >> + VM_MEM_SRC_ANONYMOUS_HUGETLB_16GB, >> + NUM_SRC_TYPES, >> }; >> >> struct vm_mem_backing_src_alias { >> const char *name; >> - enum vm_mem_backing_src_type type; >> + uint32_t flag; >> }; >> >> bool thp_configured(void); >> size_t get_trans_hugepagesz(void); >> +const struct vm_mem_backing_src_alias *vm_mem_backing_src_alias(uint32_t i); >> +size_t get_backing_src_pagesz(uint32_t i); >> void backing_src_help(void); >> enum vm_mem_backing_src_type parse_backing_src_type(const char *type_name); >> >> diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c >> index cc22c4ab7d67..b91c8e3a7ee1 100644 >> --- a/tools/testing/selftests/kvm/lib/kvm_util.c >> +++ b/tools/testing/selftests/kvm/lib/kvm_util.c >> @@ -757,7 +757,7 @@ void vm_userspace_mem_region_add(struct kvm_vm *vm, >> region->mmap_start = mmap(NULL, region->mmap_size, >> PROT_READ | PROT_WRITE, >> MAP_PRIVATE | MAP_ANONYMOUS >> - | (src_type == VM_MEM_SRC_ANONYMOUS_HUGETLB ? MAP_HUGETLB : 0), >> + | vm_mem_backing_src_alias(src_type)->flag, >> -1, 0); >> TEST_ASSERT(region->mmap_start != MAP_FAILED, >> "test_malloc failed, mmap_start: %p errno: %i", >> diff --git a/tools/testing/selftests/kvm/lib/test_util.c b/tools/testing/selftests/kvm/lib/test_util.c >> index f2d133f76c67..1f5e7241c80e 100644 >> --- a/tools/testing/selftests/kvm/lib/test_util.c >> +++ b/tools/testing/selftests/kvm/lib/test_util.c >> @@ -11,6 +11,7 @@ >> #include <stdlib.h> >> #include <time.h> >> #include <sys/stat.h> >> +#include <linux/mman.h> >> #include "linux/kernel.h" >> >> #include "test_util.h" >> @@ -112,12 +113,6 @@ void print_skip(const char *fmt, ...) >> puts(", skipping test"); >> } >> >> -const struct vm_mem_backing_src_alias backing_src_aliases[] = { >> - {"anonymous", VM_MEM_SRC_ANONYMOUS,}, >> - {"anonymous_thp", VM_MEM_SRC_ANONYMOUS_THP,}, >> - {"anonymous_hugetlb", VM_MEM_SRC_ANONYMOUS_HUGETLB,}, >> -}; >> - >> bool thp_configured(void) >> { >> int ret; >> @@ -153,22 +148,61 @@ size_t get_trans_hugepagesz(void) >> return size; >> } >> >> +const struct vm_mem_backing_src_alias *vm_mem_backing_src_alias(uint32_t i) >> +{ >> + static const struct vm_mem_backing_src_alias aliases[] = { >> + { "anonymous", 0 }, >> + { "anonymous_thp", 0 }, >> + { "anonymous_hugetlb_16kb", MAP_HUGETLB | MAP_HUGE_16KB }, >> + { "anonymous_hugetlb_64kb", MAP_HUGETLB | MAP_HUGE_64KB }, >> + { "anonymous_hugetlb_512kb", MAP_HUGETLB | MAP_HUGE_512KB }, >> + { "anonymous_hugetlb_1mb", MAP_HUGETLB | MAP_HUGE_1MB }, >> + { "anonymous_hugetlb_2mb", MAP_HUGETLB | MAP_HUGE_2MB }, >> + { "anonymous_hugetlb_8mb", MAP_HUGETLB | MAP_HUGE_8MB }, >> + { "anonymous_hugetlb_16mb", MAP_HUGETLB | MAP_HUGE_16MB }, >> + { "anonymous_hugetlb_32mb", MAP_HUGETLB | MAP_HUGE_32MB }, >> + { "anonymous_hugetlb_256mb", MAP_HUGETLB | MAP_HUGE_256MB }, >> + { "anonymous_hugetlb_512mb", MAP_HUGETLB | MAP_HUGE_512MB }, >> + { "anonymous_hugetlb_1gb", MAP_HUGETLB | MAP_HUGE_1GB }, >> + { "anonymous_hugetlb_2gb", MAP_HUGETLB | MAP_HUGE_2GB }, >> + { "anonymous_hugetlb_16gb", MAP_HUGETLB | MAP_HUGE_16GB }, >> + }; >> + _Static_assert(ARRAY_SIZE(aliases) == NUM_SRC_TYPES, >> + "Missing new backing src types?"); >> + >> + TEST_ASSERT(i < NUM_SRC_TYPES, "Backing src type ID %d too big", i); >> + >> + return &aliases[i]; >> +} >> + >> +size_t get_backing_src_pagesz(uint32_t i) >> +{ >> + uint32_t flag = vm_mem_backing_src_alias(i)->flag; >> + >> + if (i == VM_MEM_SRC_ANONYMOUS) >> + return getpagesize(); >> + if (i == VM_MEM_SRC_ANONYMOUS_THP) >> + return get_trans_hugepagesz(); >> + >> + return MAP_HUGE_PAGE_SIZE(flag); >> +} >> + >> void backing_src_help(void) >> { >> int i; >> >> printf("Available backing src types:\n"); >> - for (i = 0; i < ARRAY_SIZE(backing_src_aliases); i++) >> - printf("\t%s\n", backing_src_aliases[i].name); >> + for (i = 0; i < NUM_SRC_TYPES; i++) >> + printf("\t%s\n", vm_mem_backing_src_alias(i)->name); >> } >> >> enum vm_mem_backing_src_type parse_backing_src_type(const char *type_name) >> { >> int i; >> >> - for (i = 0; i < ARRAY_SIZE(backing_src_aliases); i++) >> - if (!strcmp(type_name, backing_src_aliases[i].name)) >> - return backing_src_aliases[i].type; >> + for (i = 0; i < NUM_SRC_TYPES; i++) >> + if (!strcmp(type_name, vm_mem_backing_src_alias(i)->name)) >> + return i; >> >> backing_src_help(); >> TEST_FAIL("Unknown backing src type: %s", type_name); >> -- >> 2.23.0 >> > .
| |