[PATCH v1] virtio-mem: don't warn about THP sizes on a kernel without THP support

[PATCH v1] virtio-mem: don't warn about THP sizes on a kernel without THP support

[David Hildenbrand]

If the config directory in sysfs does not exist at all, we are dealing
with a system that does not support THPs. Simply use 1 MiB block size
then, instead of warning "Could not detect THP size, falling back to
..." and falling back to the default THP size.

Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Gavin Shan <gshan@redhat.com>
Cc: Juraj Marcin <jmarcin@redhat.com>
Signed-off-by: David Hildenbrand <david@redhat.com>
---
 hw/virtio/virtio-mem.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/hw/virtio/virtio-mem.c b/hw/virtio/virtio-mem.c
index ef64bf1b4a..4075f3d4ce 100644
--- a/hw/virtio/virtio-mem.c
+++ b/hw/virtio/virtio-mem.c
@@ -88,6 +88,7 @@ static uint32_t virtio_mem_default_thp_size(void)
 static uint32_t thp_size;
 
 #define HPAGE_PMD_SIZE_PATH "/sys/kernel/mm/transparent_hugepage/hpage_pmd_size"
+#define HPAGE_PATH "/sys/kernel/mm/transparent_hugepage/"
 static uint32_t virtio_mem_thp_size(void)
 {
     gchar *content = NULL;
@@ -98,6 +99,12 @@ static uint32_t virtio_mem_thp_size(void)
         return thp_size;
     }
 
+    /* No THP -> no restrictions. */
+    if (!g_file_test(HPAGE_PATH, G_FILE_TEST_EXISTS)) {
+        thp_size = VIRTIO_MEM_MIN_BLOCK_SIZE;
+        return thp_size;
+    }
+
     /*
      * Try to probe the actual THP size, fallback to (sane but eventually
      * incorrect) default sizes.
-- 
2.46.0

Re: [PATCH v1] virtio-mem: don't warn about THP sizes on a kernel without THP support

[Michael S. Tsirkin]

On Tue, Sep 10, 2024 at 06:34:33PM +0200, David Hildenbrand wrote:
> If the config directory in sysfs does not exist at all, we are dealing
> with a system that does not support THPs. Simply use 1 MiB block size
> then, instead of warning "Could not detect THP size, falling back to
> ..." and falling back to the default THP size.
> 
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Cc: Gavin Shan <gshan@redhat.com>
> Cc: Juraj Marcin <jmarcin@redhat.com>
> Signed-off-by: David Hildenbrand <david@redhat.com>

Okay, I picked it up. But I have a question

> ---
>  hw/virtio/virtio-mem.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/hw/virtio/virtio-mem.c b/hw/virtio/virtio-mem.c
> index ef64bf1b4a..4075f3d4ce 100644
> --- a/hw/virtio/virtio-mem.c
> +++ b/hw/virtio/virtio-mem.c
> @@ -88,6 +88,7 @@ static uint32_t virtio_mem_default_thp_size(void)
>  static uint32_t thp_size;
>  
>  #define HPAGE_PMD_SIZE_PATH "/sys/kernel/mm/transparent_hugepage/hpage_pmd_size"
> +#define HPAGE_PATH "/sys/kernel/mm/transparent_hugepage/"


If this code runs e.g. on windows, it will poke at cwd root with
unpredictable results.
It doesn't look like this is linux specific, did I miss anything?

>  static uint32_t virtio_mem_thp_size(void)
>  {
>      gchar *content = NULL;
> @@ -98,6 +99,12 @@ static uint32_t virtio_mem_thp_size(void)
>          return thp_size;
>      }
>  
> +    /* No THP -> no restrictions. */
> +    if (!g_file_test(HPAGE_PATH, G_FILE_TEST_EXISTS)) {
> +        thp_size = VIRTIO_MEM_MIN_BLOCK_SIZE;
> +        return thp_size;
> +    }
> +
>      /*
>       * Try to probe the actual THP size, fallback to (sane but eventually
>       * incorrect) default sizes.
> -- 
> 2.46.0

Re: [PATCH v1] virtio-mem: don't warn about THP sizes on a kernel without THP support

[David Hildenbrand]

On 10.09.24 20:26, Michael S. Tsirkin wrote:
> On Tue, Sep 10, 2024 at 06:34:33PM +0200, David Hildenbrand wrote:
>> If the config directory in sysfs does not exist at all, we are dealing
>> with a system that does not support THPs. Simply use 1 MiB block size
>> then, instead of warning "Could not detect THP size, falling back to
>> ..." and falling back to the default THP size.
>>
>> Cc: "Michael S. Tsirkin" <mst@redhat.com>
>> Cc: Gavin Shan <gshan@redhat.com>
>> Cc: Juraj Marcin <jmarcin@redhat.com>
>> Signed-off-by: David Hildenbrand <david@redhat.com>
> 
> Okay, I picked it up. But I have a question

Thanks, I can also route it via my memory device tree, whatever you prefer.

> 
>> ---
>>   hw/virtio/virtio-mem.c | 7 +++++++
>>   1 file changed, 7 insertions(+)
>>
>> diff --git a/hw/virtio/virtio-mem.c b/hw/virtio/virtio-mem.c
>> index ef64bf1b4a..4075f3d4ce 100644
>> --- a/hw/virtio/virtio-mem.c
>> +++ b/hw/virtio/virtio-mem.c
>> @@ -88,6 +88,7 @@ static uint32_t virtio_mem_default_thp_size(void)
>>   static uint32_t thp_size;
>>   
>>   #define HPAGE_PMD_SIZE_PATH "/sys/kernel/mm/transparent_hugepage/hpage_pmd_size"
>> +#define HPAGE_PATH "/sys/kernel/mm/transparent_hugepage/"
> 
> 
> If this code runs e.g. on windows, it will poke at cwd root with
> unpredictable results.
> It doesn't look like this is linux specific, did I miss anything?


VIRTIO_MEM has

"depends on LINUX"

so that code will never run on Windows.

Thanks!

-- 
Cheers,

David / dhildenb

Re: [PATCH v1] virtio-mem: don't warn about THP sizes on a kernel without THP support

[Gavin Shan]

On 9/11/24 2:34 AM, David Hildenbrand wrote:
> If the config directory in sysfs does not exist at all, we are dealing
> with a system that does not support THPs. Simply use 1 MiB block size
> then, instead of warning "Could not detect THP size, falling back to
> ..." and falling back to the default THP size.
> 
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Cc: Gavin Shan <gshan@redhat.com>
> Cc: Juraj Marcin <jmarcin@redhat.com>
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>   hw/virtio/virtio-mem.c | 7 +++++++
>   1 file changed, 7 insertions(+)
> 

Reviewed-by: Gavin Shan <gshan@redhat.com>