Re: [Qemu-devel] [PATCH] linux-user: Remove extra mapping

[Laurent Vivier]

Le 16/05/2018 à 16:47, Steve Mcpolin a écrit :
> When a guest mmap()'d a file, a transient MAP_ANONYMOUS mapping was
> created, which required the kernel to reserve this memory, then
> subsequently released by applying a mapping with just the requested
> flags and fd.
> This transient mapping causes spurious failures when the available
> memory is smaller than the mapping.  This patch avoids this transient
> mapping.
> 
> Signed-off-by: Steve Mcpolin 
> ---
>  linux-user/mmap.c | 31 +++
>  1 file changed, 19 insertions(+), 12 deletions(-)
> 
> diff --git a/linux-user/mmap.c b/linux-user/mmap.c
> index 9168a20..f91841f 100644
> --- a/linux-user/mmap.c
> +++ b/linux-user/mmap.c
> @@ -453,21 +453,28 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, 
> int prot,
>  /* Note: we prefer to control the mapping address. It is
> especially important if qemu_host_page_size >
> qemu_real_host_page_size */
> -p = mmap(g2h(start), host_len, prot,
> - flags | MAP_FIXED | MAP_ANONYMOUS, -1, 0);
> -if (p == MAP_FAILED)
> -goto fail;
> -/* update start so that it points to the file position at 'offset' */
> -host_start = (unsigned long)p;
> -if (!(flags & MAP_ANONYMOUS)) {
> -p = mmap(g2h(start), len, prot,
> +if (flags & MAP_ANONYMOUS) {
> +offset = 0;
> +host_offset = 0;
> +fd = -1;
> +}
> +p = mmap(g2h(start), len, prot,
>   flags | MAP_FIXED, fd, host_offset);
> -if (p == MAP_FAILED) {
> -munmap(g2h(start), host_len);
> -goto fail;
> +host_start = (uintptr_t)p;
> +if (p != MAP_FAILED && host_len > HOST_PAGE_ALIGN(len)) {
> +void *q;
> +q = mmap(g2h(start + len), host_len - HOST_PAGE_ALIGN(len),

I think you should use REAL_HOST_PAGE_ALIGN(len) above.

This is the page size used by the host mmap() (it comes from
getpagesize()), whereas HOST_PAGE_ALIGN() can come from the command line
arguments.

Thanks,
Laurent

[Qemu-devel] [PATCH] linux-user: Remove extra mapping

[Steve Mcpolin]

When a guest mmap()'d a file, a transient MAP_ANONYMOUS mapping was
created, which required the kernel to reserve this memory, then
subsequently released by applying a mapping with just the requested
flags and fd.
This transient mapping causes spurious failures when the available
memory is smaller than the mapping.  This patch avoids this transient
mapping.

Signed-off-by: Steve Mcpolin 
---
 linux-user/mmap.c | 31 +++
 1 file changed, 19 insertions(+), 12 deletions(-)

diff --git a/linux-user/mmap.c b/linux-user/mmap.c
index 9168a20..f91841f 100644
--- a/linux-user/mmap.c
+++ b/linux-user/mmap.c
@@ -453,21 +453,28 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int 
prot,
 /* Note: we prefer to control the mapping address. It is
especially important if qemu_host_page_size >
qemu_real_host_page_size */
-p = mmap(g2h(start), host_len, prot,
- flags | MAP_FIXED | MAP_ANONYMOUS, -1, 0);
-if (p == MAP_FAILED)
-goto fail;
-/* update start so that it points to the file position at 'offset' */
-host_start = (unsigned long)p;
-if (!(flags & MAP_ANONYMOUS)) {
-p = mmap(g2h(start), len, prot,
+if (flags & MAP_ANONYMOUS) {
+offset = 0;
+host_offset = 0;
+fd = -1;
+}
+p = mmap(g2h(start), len, prot,
  flags | MAP_FIXED, fd, host_offset);
-if (p == MAP_FAILED) {
-munmap(g2h(start), host_len);
-goto fail;
+host_start = (uintptr_t)p;
+if (p != MAP_FAILED && host_len > HOST_PAGE_ALIGN(len)) {
+void *q;
+q = mmap(g2h(start + len), host_len - HOST_PAGE_ALIGN(len),
+ prot, MAP_FIXED | MAP_ANONYMOUS, -1, 0);
+if (q == MAP_FAILED) {
+p = MAP_FAILED;
 }
-host_start += offset - host_offset;
 }
+if (p == MAP_FAILED) {
+munmap(g2h(start), host_len);
+goto fail;
+}
+host_start += offset - host_offset;
+/* update start so that it points to the file position at 'offset' */
 start = h2g(host_start);
 } else {
 if (start & ~TARGET_PAGE_MASK) {
-- 
2.7.4