diff mbox series

[v2,4/4] backends/hostmem: Report error when memory size is unaligned

Message ID 58295d1f6c995c0c444e375348436e799689126c.1717140354.git.mprivozn@redhat.com (mailing list archive)
State New, archived
Headers show
Series backends/hostmem: Report more errors on failures | expand

Commit Message

Michal Privoznik May 31, 2024, 7:29 a.m. UTC
If memory-backend-{file,ram} has a size that's not aligned to
underlying page size it is not only wasteful, but also may lead
to hard to debug behaviour. For instance, in case
memory-backend-file and hugepages, madvise() and mbind() fail.
Rightfully so, page is the smallest unit they can work with. And
even though an error is reported, the root cause it not very
clear:

  qemu-system-x86_64: Couldn't set property 'dump' on 'memory-backend-file': Invalid argument

After this commit:

  qemu-system-x86_64: backend memory size must be multiple of 0x200000

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 backends/hostmem.c | 8 ++++++++
 1 file changed, 8 insertions(+)

Comments

Philippe Mathieu-Daudé May 31, 2024, 7:53 a.m. UTC | #1
Hi Michal,

On 31/5/24 09:29, Michal Privoznik wrote:
> If memory-backend-{file,ram} has a size that's not aligned to
> underlying page size it is not only wasteful, but also may lead
> to hard to debug behaviour. For instance, in case
> memory-backend-file and hugepages, madvise() and mbind() fail.
> Rightfully so, page is the smallest unit they can work with. And
> even though an error is reported, the root cause it not very
> clear:
> 
>    qemu-system-x86_64: Couldn't set property 'dump' on 'memory-backend-file': Invalid argument
> 
> After this commit:
> 
>    qemu-system-x86_64: backend memory size must be multiple of 0x200000
> 
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
>   backends/hostmem.c | 8 ++++++++
>   1 file changed, 8 insertions(+)
> 
> diff --git a/backends/hostmem.c b/backends/hostmem.c
> index 012a8c190f..5f9c9ea8f5 100644
> --- a/backends/hostmem.c
> +++ b/backends/hostmem.c
> @@ -337,6 +337,7 @@ host_memory_backend_memory_complete(UserCreatable *uc, Error **errp)
>       HostMemoryBackendClass *bc = MEMORY_BACKEND_GET_CLASS(uc);
>       void *ptr;
>       uint64_t sz;
> +    size_t pagesize;
>       bool async = !phase_check(PHASE_LATE_BACKENDS_CREATED);
>   
>       if (!bc->alloc) {
> @@ -348,6 +349,13 @@ host_memory_backend_memory_complete(UserCreatable *uc, Error **errp)
>   
>       ptr = memory_region_get_ram_ptr(&backend->mr);
>       sz = memory_region_size(&backend->mr);
> +    pagesize = qemu_ram_pagesize(backend->mr.ram_block);
> +
> +    if (!QEMU_IS_ALIGNED(sz, pagesize)) {
> +        error_setg(errp, "backend memory size must be multiple of 0x%"
> +                   PRIx64, pagesize);

Can we use size_to_str() instead?

Also display backend name:

"backend '%s' memory size must be multiple of %s"

> +        return;
> +    }
>   
>       if (backend->merge &&
>           qemu_madvise(ptr, sz, QEMU_MADV_MERGEABLE)) {
Mario Casquero June 5, 2024, 7:12 a.m. UTC | #2
This patch has been successfully tested by QE. After allocating some
hugepages in the host, try to boot up a VM with the memory backed by a
file and the size unaligned, check now the message displayed by QEMU:
qemu-system-x86_64: backend memory size must be multiple of 0x200000

Tested-by: Mario Casquero <mcasquer@redhat.com>


On Fri, May 31, 2024 at 9:29 AM Michal Privoznik <mprivozn@redhat.com> wrote:
>
> If memory-backend-{file,ram} has a size that's not aligned to
> underlying page size it is not only wasteful, but also may lead
> to hard to debug behaviour. For instance, in case
> memory-backend-file and hugepages, madvise() and mbind() fail.
> Rightfully so, page is the smallest unit they can work with. And
> even though an error is reported, the root cause it not very
> clear:
>
>   qemu-system-x86_64: Couldn't set property 'dump' on 'memory-backend-file': Invalid argument
>
> After this commit:
>
>   qemu-system-x86_64: backend memory size must be multiple of 0x200000
>
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
>  backends/hostmem.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/backends/hostmem.c b/backends/hostmem.c
> index 012a8c190f..5f9c9ea8f5 100644
> --- a/backends/hostmem.c
> +++ b/backends/hostmem.c
> @@ -337,6 +337,7 @@ host_memory_backend_memory_complete(UserCreatable *uc, Error **errp)
>      HostMemoryBackendClass *bc = MEMORY_BACKEND_GET_CLASS(uc);
>      void *ptr;
>      uint64_t sz;
> +    size_t pagesize;
>      bool async = !phase_check(PHASE_LATE_BACKENDS_CREATED);
>
>      if (!bc->alloc) {
> @@ -348,6 +349,13 @@ host_memory_backend_memory_complete(UserCreatable *uc, Error **errp)
>
>      ptr = memory_region_get_ram_ptr(&backend->mr);
>      sz = memory_region_size(&backend->mr);
> +    pagesize = qemu_ram_pagesize(backend->mr.ram_block);
> +
> +    if (!QEMU_IS_ALIGNED(sz, pagesize)) {
> +        error_setg(errp, "backend memory size must be multiple of 0x%"
> +                   PRIx64, pagesize);
> +        return;
> +    }
>
>      if (backend->merge &&
>          qemu_madvise(ptr, sz, QEMU_MADV_MERGEABLE)) {
> --
> 2.44.1
>
>
diff mbox series

Patch

diff --git a/backends/hostmem.c b/backends/hostmem.c
index 012a8c190f..5f9c9ea8f5 100644
--- a/backends/hostmem.c
+++ b/backends/hostmem.c
@@ -337,6 +337,7 @@  host_memory_backend_memory_complete(UserCreatable *uc, Error **errp)
     HostMemoryBackendClass *bc = MEMORY_BACKEND_GET_CLASS(uc);
     void *ptr;
     uint64_t sz;
+    size_t pagesize;
     bool async = !phase_check(PHASE_LATE_BACKENDS_CREATED);
 
     if (!bc->alloc) {
@@ -348,6 +349,13 @@  host_memory_backend_memory_complete(UserCreatable *uc, Error **errp)
 
     ptr = memory_region_get_ram_ptr(&backend->mr);
     sz = memory_region_size(&backend->mr);
+    pagesize = qemu_ram_pagesize(backend->mr.ram_block);
+
+    if (!QEMU_IS_ALIGNED(sz, pagesize)) {
+        error_setg(errp, "backend memory size must be multiple of 0x%"
+                   PRIx64, pagesize);
+        return;
+    }
 
     if (backend->merge &&
         qemu_madvise(ptr, sz, QEMU_MADV_MERGEABLE)) {