[Qemu-devel] [PATCH] mmap-alloc: tweak a comment on ppc64

[Qemu-devel] [PATCH] mmap-alloc: tweak a comment on ppc64

[Michael S. Tsirkin]

The comment I put in mmap-alloc to document the ppc64 rules
refers to the previous revision of the patch:
we don't look at memory alignment anymore, we check
the fs from which the fd is mapped, instead.

It's also not clear what does "in this case" refer
to, rearrange text to make it clearer.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 util/mmap-alloc.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/util/mmap-alloc.c b/util/mmap-alloc.c
index 54793a5..5cd7f71 100644
--- a/util/mmap-alloc.c
+++ b/util/mmap-alloc.c
@@ -50,10 +50,11 @@ void *qemu_ram_mmap(int fd, size_t size, size_t align, bool shared)
 #if defined(__powerpc64__) && defined(__linux__)
     /* On ppc64 mappings in the same segment (aka slice) must share the same
      * page size. Since we will be re-allocating part of this segment
-     * from the supplied fd, we should make sure to use the same page size,
-     * unless we are using the system page size, in which case anonymous memory
-     * is OK. Use align as a hint for the page size.
-     * In this case, set MAP_NORESERVE to avoid allocating backing store memory.
+     * from the supplied fd, we should make sure to use the same page size, to
+     * this end we mmap the supplied fd.  In this case, set MAP_NORESERVE to
+     * avoid allocating backing store memory.
+     * We do this unless we are using the system page size, in which case
+     * anonymous memory is OK.
      */
     int anonfd = fd == -1 || qemu_fd_getpagesize(fd) == getpagesize() ? -1 : fd;
     int flags = anonfd == -1 ? MAP_ANONYMOUS : MAP_NORESERVE;
-- 
MST

Re: [Qemu-devel] [PATCH] mmap-alloc: tweak a comment on ppc64

[Greg Kurz]

On Thu, 3 Dec 2015 10:38:10 +0200
"Michael S. Tsirkin" <mst@redhat.com> wrote:

> The comment I put in mmap-alloc to document the ppc64 rules
> refers to the previous revision of the patch:
> we don't look at memory alignment anymore, we check
> the fs from which the fd is mapped, instead.
> 
> It's also not clear what does "in this case" refer
> to, rearrange text to make it clearer.
> 
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---

This is better indeed. Thanks !

Reviewed-by: Greg Kurz <gkurz@linux.vnet.ibm.com>

>  util/mmap-alloc.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/util/mmap-alloc.c b/util/mmap-alloc.c
> index 54793a5..5cd7f71 100644
> --- a/util/mmap-alloc.c
> +++ b/util/mmap-alloc.c
> @@ -50,10 +50,11 @@ void *qemu_ram_mmap(int fd, size_t size, size_t align, bool shared)
>  #if defined(__powerpc64__) && defined(__linux__)
>      /* On ppc64 mappings in the same segment (aka slice) must share the same
>       * page size. Since we will be re-allocating part of this segment
> -     * from the supplied fd, we should make sure to use the same page size,
> -     * unless we are using the system page size, in which case anonymous memory
> -     * is OK. Use align as a hint for the page size.
> -     * In this case, set MAP_NORESERVE to avoid allocating backing store memory.
> +     * from the supplied fd, we should make sure to use the same page size, to
> +     * this end we mmap the supplied fd.  In this case, set MAP_NORESERVE to
> +     * avoid allocating backing store memory.
> +     * We do this unless we are using the system page size, in which case
> +     * anonymous memory is OK.
>       */
>      int anonfd = fd == -1 || qemu_fd_getpagesize(fd) == getpagesize() ? -1 : fd;
>      int flags = anonfd == -1 ? MAP_ANONYMOUS : MAP_NORESERVE;