| Message ID | 20240910163433.2100295-1-david@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v1] virtio-mem: don't warn about THP sizes on a kernel without THP support | expand |
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
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!
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>
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.
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(+)