[RFC,v4,5/9] KVM: selftests: Add a helper to get system configured THP page size
diff mbox series

Message ID 20210302125751.19080-6-wangyanan55@huawei.com
State New, archived
Headers show
Series
  • KVM: selftests: some improvement and a new test for kvm page table
Related show

Commit Message

Yanan Wang March 2, 2021, 12:57 p.m. UTC
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(+)

Comments

Andrew Jones March 12, 2021, 11:31 a.m. UTC | #1
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
Yanan Wang March 22, 2021, 6:42 a.m. UTC | #2
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
>
> .

Patch
diff mbox series

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;