* [Qemu-devel] [PATCH v2] exec.c: Ensure right alignment also for file backed ram
@ 2016-04-07 21:31 Dominik Dingel
2016-04-12 21:45 ` Paolo Bonzini
0 siblings, 1 reply; 3+ messages in thread
From: Dominik Dingel @ 2016-04-07 21:31 UTC (permalink / raw)
To: qemu-devel
Cc: Paolo Bonzini, Peter Crosthwaite, Richard Henderson, Halil Pasic
While in the anonymous ram case we already take care of the right alignment
such an alignment gurantee does not exist for file backed ram allocation.
Instead, pagesize is used for alignment. On s390 this is not enough for gmap,
as we need to satisfy an alignment up to segments.
Reported-by: Halil Pasic <pasic@linux.vnet.ibm.com>
Signed-off-by: Dominik Dingel <dingel@linux.vnet.ibm.com>
---
v1 -> v2:
While enforcing alignments we allow memory sizes on page_size.
On mmap the memory size will be round up to alignments.
I thought about moving this alignment into qemu_ram_mmap but the result
was a lot of code churn, the other possibility was to create an additional
define ending up with two defines with the same semantics.
---
exec.c | 8 +++++---
include/qemu/osdep.h | 12 ++++++++++++
util/oslib-posix.c | 12 ------------
3 files changed, 17 insertions(+), 15 deletions(-)
diff --git a/exec.c b/exec.c
index c4f9036..1ae98e4 100644
--- a/exec.c
+++ b/exec.c
@@ -1241,6 +1241,7 @@ static void *file_ram_alloc(RAMBlock *block,
void *area;
int fd = -1;
int64_t page_size;
+ int64_t alignment;
if (kvm_enabled() && !kvm_has_sync_mmu()) {
error_setg(errp,
@@ -1296,7 +1297,8 @@ static void *file_ram_alloc(RAMBlock *block,
}
page_size = qemu_fd_getpagesize(fd);
- block->mr->align = page_size;
+ alignment = MAX(page_size, QEMU_VMALLOC_ALIGN);
+ block->mr->align = alignment;
if (memory < page_size) {
error_setg(errp, "memory size 0x" RAM_ADDR_FMT " must be equal to "
@@ -1305,7 +1307,7 @@ static void *file_ram_alloc(RAMBlock *block,
goto error;
}
- memory = ROUND_UP(memory, page_size);
+ memory = ROUND_UP(memory, alignment);
/*
* ftruncate is not supported by hugetlbfs in older
@@ -1317,7 +1319,7 @@ static void *file_ram_alloc(RAMBlock *block,
perror("ftruncate");
}
- area = qemu_ram_mmap(fd, memory, page_size, block->flags & RAM_SHARED);
+ area = qemu_ram_mmap(fd, memory, alignment, block->flags & RAM_SHARED);
if (area == MAP_FAILED) {
error_setg_errno(errp, errno,
"unable to map backing store for guest RAM");
diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index 408783f..a472372 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -247,6 +247,18 @@ void qemu_anon_ram_free(void *ptr, size_t size);
#endif
+#if defined(__linux__) && (defined(__x86_64__) || defined(__arm__))
+ /* Use 2 MiB alignment so transparent hugepages can be used by KVM.
+ Valgrind does not support alignments larger than 1 MiB,
+ therefore we need special code which handles running on Valgrind. */
+# define QEMU_VMALLOC_ALIGN (512 * 4096)
+#elif defined(__linux__) && defined(__s390x__)
+ /* Use 1 MiB (segment size) alignment so gmap can be used by KVM. */
+# define QEMU_VMALLOC_ALIGN (256 * 4096)
+#else
+# define QEMU_VMALLOC_ALIGN getpagesize()
+#endif
+
int qemu_madvise(void *addr, size_t len, int advice);
int qemu_open(const char *name, int flags, ...);
diff --git a/util/oslib-posix.c b/util/oslib-posix.c
index 20ca141..4adde93 100644
--- a/util/oslib-posix.c
+++ b/util/oslib-posix.c
@@ -26,18 +26,6 @@
* THE SOFTWARE.
*/
-#if defined(__linux__) && (defined(__x86_64__) || defined(__arm__))
- /* Use 2 MiB alignment so transparent hugepages can be used by KVM.
- Valgrind does not support alignments larger than 1 MiB,
- therefore we need special code which handles running on Valgrind. */
-# define QEMU_VMALLOC_ALIGN (512 * 4096)
-#elif defined(__linux__) && defined(__s390x__)
- /* Use 1 MiB (segment size) alignment so gmap can be used by KVM. */
-# define QEMU_VMALLOC_ALIGN (256 * 4096)
-#else
-# define QEMU_VMALLOC_ALIGN getpagesize()
-#endif
-
#include "qemu/osdep.h"
#include <termios.h>
#include <termios.h>
--
2.6.6
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [Qemu-devel] [PATCH v2] exec.c: Ensure right alignment also for file backed ram
2016-04-07 21:31 [Qemu-devel] [PATCH v2] exec.c: Ensure right alignment also for file backed ram Dominik Dingel
@ 2016-04-12 21:45 ` Paolo Bonzini
2016-04-13 7:19 ` Dominik Dingel
0 siblings, 1 reply; 3+ messages in thread
From: Paolo Bonzini @ 2016-04-12 21:45 UTC (permalink / raw)
To: Dominik Dingel, qemu-devel
Cc: Peter Crosthwaite, Richard Henderson, Halil Pasic
On 07/04/2016 23:31, Dominik Dingel wrote:
> diff --git a/exec.c b/exec.c
> index c4f9036..1ae98e4 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -1241,6 +1241,7 @@ static void *file_ram_alloc(RAMBlock *block,
> void *area;
> int fd = -1;
> int64_t page_size;
> + int64_t alignment;
>
> if (kvm_enabled() && !kvm_has_sync_mmu()) {
> error_setg(errp,
> @@ -1296,7 +1297,8 @@ static void *file_ram_alloc(RAMBlock *block,
> }
>
> page_size = qemu_fd_getpagesize(fd);
> - block->mr->align = page_size;
> + alignment = MAX(page_size, QEMU_VMALLOC_ALIGN);
> + block->mr->align = alignment;
>
> if (memory < page_size) {
> error_setg(errp, "memory size 0x" RAM_ADDR_FMT " must be equal to "
> @@ -1305,7 +1307,7 @@ static void *file_ram_alloc(RAMBlock *block,
> goto error;
> }
>
> - memory = ROUND_UP(memory, page_size);
> + memory = ROUND_UP(memory, alignment);
I think this change is not necessary either; it is enough to change the
qemu_ram_mmap below.
Paolo
> /*
> * ftruncate is not supported by hugetlbfs in older
> @@ -1317,7 +1319,7 @@ static void *file_ram_alloc(RAMBlock *block,
> perror("ftruncate");
> }
>
> - area = qemu_ram_mmap(fd, memory, page_size, block->flags & RAM_SHARED);
> + area = qemu_ram_mmap(fd, memory, alignment, block->flags & RAM_SHARED);
> if (area == MAP_FAILED) {
> error_setg_errno(errp, errno,
> "unable to map backing store for guest RAM");
> diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
> index 408783f..a472372 100644
> --- a/include/qemu/osdep.h
> +++ b/include/qemu/osdep.h
> @@ -247,6 +247,18 @@ void qemu_anon_ram_free(void *ptr, size_t size);
>
> #endif
>
> +#if defined(__linux__) && (defined(__x86_64__) || defined(__arm__))
> + /* Use 2 MiB alignment so transparent hugepages can be used by KVM.
> + Valgrind does not support alignments larger than 1 MiB,
> + therefore we need special code which handles running on Valgrind. */
> +# define QEMU_VMALLOC_ALIGN (512 * 4096)
> +#elif defined(__linux__) && defined(__s390x__)
> + /* Use 1 MiB (segment size) alignment so gmap can be used by KVM. */
> +# define QEMU_VMALLOC_ALIGN (256 * 4096)
> +#else
> +# define QEMU_VMALLOC_ALIGN getpagesize()
> +#endif
> +
> int qemu_madvise(void *addr, size_t len, int advice);
>
> int qemu_open(const char *name, int flags, ...);
> diff --git a/util/oslib-posix.c b/util/oslib-posix.c
> index 20ca141..4adde93 100644
> --- a/util/oslib-posix.c
> +++ b/util/oslib-posix.c
> @@ -26,18 +26,6 @@
> * THE SOFTWARE.
> */
>
> -#if defined(__linux__) && (defined(__x86_64__) || defined(__arm__))
> - /* Use 2 MiB alignment so transparent hugepages can be used by KVM.
> - Valgrind does not support alignments larger than 1 MiB,
> - therefore we need special code which handles running on Valgrind. */
> -# define QEMU_VMALLOC_ALIGN (512 * 4096)
> -#elif defined(__linux__) && defined(__s390x__)
> - /* Use 1 MiB (segment size) alignment so gmap can be used by KVM. */
> -# define QEMU_VMALLOC_ALIGN (256 * 4096)
> -#else
> -# define QEMU_VMALLOC_ALIGN getpagesize()
> -#endif
> -
> #include "qemu/osdep.h"
> #include <termios.h>
> #include <termios.h>
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [Qemu-devel] [PATCH v2] exec.c: Ensure right alignment also for file backed ram
2016-04-12 21:45 ` Paolo Bonzini
@ 2016-04-13 7:19 ` Dominik Dingel
0 siblings, 0 replies; 3+ messages in thread
From: Dominik Dingel @ 2016-04-13 7:19 UTC (permalink / raw)
To: Paolo Bonzini
Cc: qemu-devel, Peter Crosthwaite, Richard Henderson, Halil Pasic
On Tue, 12 Apr 2016 23:45:18 +0200
Paolo Bonzini <pbonzini@redhat.com> wrote:
>
>
> On 07/04/2016 23:31, Dominik Dingel wrote:
> > diff --git a/exec.c b/exec.c
> > index c4f9036..1ae98e4 100644
> > --- a/exec.c
> > +++ b/exec.c
> > @@ -1241,6 +1241,7 @@ static void *file_ram_alloc(RAMBlock *block,
> > void *area;
> > int fd = -1;
> > int64_t page_size;
> > + int64_t alignment;
> >
> > if (kvm_enabled() && !kvm_has_sync_mmu()) {
> > error_setg(errp,
> > @@ -1296,7 +1297,8 @@ static void *file_ram_alloc(RAMBlock *block,
> > }
> >
> > page_size = qemu_fd_getpagesize(fd);
> > - block->mr->align = page_size;
> > + alignment = MAX(page_size, QEMU_VMALLOC_ALIGN);
> > + block->mr->align = alignment;
> >
> > if (memory < page_size) {
> > error_setg(errp, "memory size 0x" RAM_ADDR_FMT " must be equal to "
> > @@ -1305,7 +1307,7 @@ static void *file_ram_alloc(RAMBlock *block,
> > goto error;
> > }
> >
> > - memory = ROUND_UP(memory, page_size);
> > + memory = ROUND_UP(memory, alignment);
>
> I think this change is not necessary either; it is enough to change the
> qemu_ram_mmap below.
You are right "memory" will never be back propagated to anything related to memory slots.
On which we have such a size constraint.
Thanks,
Dominik
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2016-04-13 7:19 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-04-07 21:31 [Qemu-devel] [PATCH v2] exec.c: Ensure right alignment also for file backed ram Dominik Dingel
2016-04-12 21:45 ` Paolo Bonzini
2016-04-13 7:19 ` Dominik Dingel
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.