| Message ID | 20210302125751.19080-6-wangyanan55@huawei.com |
|---|---|
| State | New |
| Headers | show |
| Series | KVM: selftests: some improvement and a new test for kvm page table | expand |
On Tue, Mar 02, 2021 at 08:57:47PM +0800, Yanan Wang wrote: > If we want to have some tests about transparent hugepages, the system > configured THP hugepage size should better be known by the tests, which > can be used for kinds of alignment or guest memory accessing of vcpus... > So it makes sense to add a helper to get the transparent hugepage size. > > With VM_MEM_SRC_ANONYMOUS_THP specified in vm_userspace_mem_region_add(), > we now stat /sys/kernel/mm/transparent_hugepage to check whether THP is > configured in the host kernel before madvise(). Based on this, we can also > read file /sys/kernel/mm/transparent_hugepage/hpage_pmd_size to get THP > hugepage size. > > Signed-off-by: Yanan Wang <wangyanan55@huawei.com> > Reviewed-by: Ben Gardon <bgardon@google.com> > --- > .../testing/selftests/kvm/include/test_util.h | 2 ++ > tools/testing/selftests/kvm/lib/test_util.c | 36 +++++++++++++++++++ > 2 files changed, 38 insertions(+) > > diff --git a/tools/testing/selftests/kvm/include/test_util.h b/tools/testing/selftests/kvm/include/test_util.h > index b7f41399f22c..ef24c76ba89a 100644 > --- a/tools/testing/selftests/kvm/include/test_util.h > +++ b/tools/testing/selftests/kvm/include/test_util.h > @@ -78,6 +78,8 @@ struct vm_mem_backing_src_alias { > enum vm_mem_backing_src_type type; > }; > > +bool thp_configured(void); > +size_t get_trans_hugepagesz(void); > 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/test_util.c b/tools/testing/selftests/kvm/lib/test_util.c > index c7c0627c6842..f2d133f76c67 100644 > --- a/tools/testing/selftests/kvm/lib/test_util.c > +++ b/tools/testing/selftests/kvm/lib/test_util.c > @@ -10,6 +10,7 @@ > #include <limits.h> > #include <stdlib.h> > #include <time.h> > +#include <sys/stat.h> > #include "linux/kernel.h" > > #include "test_util.h" > @@ -117,6 +118,41 @@ const struct vm_mem_backing_src_alias backing_src_aliases[] = { > {"anonymous_hugetlb", VM_MEM_SRC_ANONYMOUS_HUGETLB,}, > }; > > +bool thp_configured(void) > +{ > + int ret; > + struct stat statbuf; > + > + ret = stat("/sys/kernel/mm/transparent_hugepage", &statbuf); > + TEST_ASSERT(ret == 0 || (ret == -1 && errno == ENOENT), > + "Error in stating /sys/kernel/mm/transparent_hugepage: %d", > + errno); TEST_ASSERT will already output errno's string. Is that not sufficient? If not, I think extending TEST_ASSERT to output errno too would be fine. > + > + return ret == 0; > +} > + > +size_t get_trans_hugepagesz(void) > +{ > + size_t size; > + char buf[16]; > + FILE *f; > + > + TEST_ASSERT(thp_configured(), "THP is not configured in host kernel"); > + > + f = fopen("/sys/kernel/mm/transparent_hugepage/hpage_pmd_size", "r"); > + TEST_ASSERT(f != NULL, > + "Error in opening transparent_hugepage/hpage_pmd_size: %d", > + errno); Same comment as above. > + > + if (fread(buf, sizeof(char), sizeof(buf), f) == 0) { > + fclose(f); > + TEST_FAIL("Unable to read transparent_hugepage/hpage_pmd_size"); > + } > + > + size = strtoull(buf, NULL, 10); fscanf with %lld? > + return size; > +} > + > void backing_src_help(void) > { > int i; > -- > 2.23.0 > Thanks, drew
Hi Drew, Thanks for your attention to this series! On 2021/3/12 19:31, Andrew Jones wrote: > On Tue, Mar 02, 2021 at 08:57:47PM +0800, Yanan Wang wrote: >> If we want to have some tests about transparent hugepages, the system >> configured THP hugepage size should better be known by the tests, which >> can be used for kinds of alignment or guest memory accessing of vcpus... >> So it makes sense to add a helper to get the transparent hugepage size. >> >> With VM_MEM_SRC_ANONYMOUS_THP specified in vm_userspace_mem_region_add(), >> we now stat /sys/kernel/mm/transparent_hugepage to check whether THP is >> configured in the host kernel before madvise(). Based on this, we can also >> read file /sys/kernel/mm/transparent_hugepage/hpage_pmd_size to get THP >> hugepage size. >> >> Signed-off-by: Yanan Wang <wangyanan55@huawei.com> >> Reviewed-by: Ben Gardon <bgardon@google.com> >> --- >> .../testing/selftests/kvm/include/test_util.h | 2 ++ >> tools/testing/selftests/kvm/lib/test_util.c | 36 +++++++++++++++++++ >> 2 files changed, 38 insertions(+) >> >> diff --git a/tools/testing/selftests/kvm/include/test_util.h b/tools/testing/selftests/kvm/include/test_util.h >> index b7f41399f22c..ef24c76ba89a 100644 >> --- a/tools/testing/selftests/kvm/include/test_util.h >> +++ b/tools/testing/selftests/kvm/include/test_util.h >> @@ -78,6 +78,8 @@ struct vm_mem_backing_src_alias { >> enum vm_mem_backing_src_type type; >> }; >> >> +bool thp_configured(void); >> +size_t get_trans_hugepagesz(void); >> 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/test_util.c b/tools/testing/selftests/kvm/lib/test_util.c >> index c7c0627c6842..f2d133f76c67 100644 >> --- a/tools/testing/selftests/kvm/lib/test_util.c >> +++ b/tools/testing/selftests/kvm/lib/test_util.c >> @@ -10,6 +10,7 @@ >> #include <limits.h> >> #include <stdlib.h> >> #include <time.h> >> +#include <sys/stat.h> >> #include "linux/kernel.h" >> >> #include "test_util.h" >> @@ -117,6 +118,41 @@ const struct vm_mem_backing_src_alias backing_src_aliases[] = { >> {"anonymous_hugetlb", VM_MEM_SRC_ANONYMOUS_HUGETLB,}, >> }; >> >> +bool thp_configured(void) >> +{ >> + int ret; >> + struct stat statbuf; >> + >> + ret = stat("/sys/kernel/mm/transparent_hugepage", &statbuf); >> + TEST_ASSERT(ret == 0 || (ret == -1 && errno == ENOENT), >> + "Error in stating /sys/kernel/mm/transparent_hugepage: %d", >> + errno); > TEST_ASSERT will already output errno's string. Is that not sufficient? If > not, I think extending TEST_ASSERT to output errno too would be fine. I think it's a good idea to output the errno together with it's string in TEST_ASSERT, it will explicitly indicate that the string is an error information and the errno is much easier to be used for debugging than the string. I will make this change a separate patch in next version and add your S-b tag. >> + >> + return ret == 0; >> +} >> + >> +size_t get_trans_hugepagesz(void) >> +{ >> + size_t size; >> + char buf[16]; >> + FILE *f; >> + >> + TEST_ASSERT(thp_configured(), "THP is not configured in host kernel"); >> + >> + f = fopen("/sys/kernel/mm/transparent_hugepage/hpage_pmd_size", "r"); >> + TEST_ASSERT(f != NULL, >> + "Error in opening transparent_hugepage/hpage_pmd_size: %d", >> + errno); > Same comment as above. > >> + >> + if (fread(buf, sizeof(char), sizeof(buf), f) == 0) { >> + fclose(f); >> + TEST_FAIL("Unable to read transparent_hugepage/hpage_pmd_size"); >> + } >> + >> + size = strtoull(buf, NULL, 10); > fscanf with %lld? This makes senses. But it should be %ld corresponding to size_t. Thanks, Yanan. >> + return size; >> +} >> + >> void backing_src_help(void) >> { >> int i; >> -- >> 2.23.0 >> > Thanks, > drew > > .
diff --git a/tools/testing/selftests/kvm/include/test_util.h b/tools/testing/selftests/kvm/include/test_util.h index b7f41399f22c..ef24c76ba89a 100644 --- a/tools/testing/selftests/kvm/include/test_util.h +++ b/tools/testing/selftests/kvm/include/test_util.h @@ -78,6 +78,8 @@ struct vm_mem_backing_src_alias { enum vm_mem_backing_src_type type; }; +bool thp_configured(void); +size_t get_trans_hugepagesz(void); 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/test_util.c b/tools/testing/selftests/kvm/lib/test_util.c index c7c0627c6842..f2d133f76c67 100644 --- a/tools/testing/selftests/kvm/lib/test_util.c +++ b/tools/testing/selftests/kvm/lib/test_util.c @@ -10,6 +10,7 @@ #include <limits.h> #include <stdlib.h> #include <time.h> +#include <sys/stat.h> #include "linux/kernel.h" #include "test_util.h" @@ -117,6 +118,41 @@ const struct vm_mem_backing_src_alias backing_src_aliases[] = { {"anonymous_hugetlb", VM_MEM_SRC_ANONYMOUS_HUGETLB,}, }; +bool thp_configured(void) +{ + int ret; + struct stat statbuf; + + ret = stat("/sys/kernel/mm/transparent_hugepage", &statbuf); + TEST_ASSERT(ret == 0 || (ret == -1 && errno == ENOENT), + "Error in stating /sys/kernel/mm/transparent_hugepage: %d", + errno); + + return ret == 0; +} + +size_t get_trans_hugepagesz(void) +{ + size_t size; + char buf[16]; + FILE *f; + + TEST_ASSERT(thp_configured(), "THP is not configured in host kernel"); + + f = fopen("/sys/kernel/mm/transparent_hugepage/hpage_pmd_size", "r"); + TEST_ASSERT(f != NULL, + "Error in opening transparent_hugepage/hpage_pmd_size: %d", + errno); + + if (fread(buf, sizeof(char), sizeof(buf), f) == 0) { + fclose(f); + TEST_FAIL("Unable to read transparent_hugepage/hpage_pmd_size"); + } + + size = strtoull(buf, NULL, 10); + return size; +} + void backing_src_help(void) { int i;