[PATCH v2] linux-user: Passthrough MADV_DONTNEED for certain file mappings

[PATCH v2] linux-user: Passthrough MADV_DONTNEED for certain file mappings

[Ilya Leoshkevich]

This is a follow-up for commit 892a4f6a750a ("linux-user: Add partial
support for MADV_DONTNEED"), which added passthrough for anonymous
mappings. File mappings can be handled in a similar manner.

In order to do that, mark pages, for which mmap() was passed through,
with PAGE_PASSTHROUGH, and then allow madvise() passthrough for these
pages as well.

Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---

v1: https://lists.gnu.org/archive/html/qemu-devel/2022-07/msg00112.html
v1 -> v2: Fix PAGE_PASSTHROUGH value; make checks consistent with
          page_set_flags() (Laurent).

 include/exec/cpu-all.h |  6 ++++++
 linux-user/mmap.c      | 25 +++++++++++++++++++++----
 2 files changed, 27 insertions(+), 4 deletions(-)

diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
index f5bda2c3ca..2d29ba13c0 100644
--- a/include/exec/cpu-all.h
+++ b/include/exec/cpu-all.h
@@ -262,6 +262,12 @@ extern const TargetPageBits target_page;
 #define PAGE_TARGET_1  0x0200
 #define PAGE_TARGET_2  0x0400
 
+/*
+ * For linux-user, indicates that the page is mapped with the same semantics
+ * in both guest and host.
+ */
+#define PAGE_PASSTHROUGH 0x0800
+
 #if defined(CONFIG_USER_ONLY)
 void page_dump(FILE *f);
 
diff --git a/linux-user/mmap.c b/linux-user/mmap.c
index 4e7a6be6ee..79928e3ae5 100644
--- a/linux-user/mmap.c
+++ b/linux-user/mmap.c
@@ -424,7 +424,8 @@ abi_ulong mmap_find_vma(abi_ulong start, abi_ulong size, abi_ulong align)
 abi_long target_mmap(abi_ulong start, abi_ulong len, int target_prot,
                      int flags, int fd, abi_ulong offset)
 {
-    abi_ulong ret, end, real_start, real_end, retaddr, host_offset, host_len;
+    abi_ulong ret, end, real_start, real_end, retaddr, host_offset, host_len,
+              passthrough_start = -1, passthrough_end = -1;
     int page_flags, host_prot;
 
     mmap_lock();
@@ -537,6 +538,8 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int target_prot,
             host_start += offset - host_offset;
         }
         start = h2g(host_start);
+        passthrough_start = start;
+        passthrough_end = start + len;
     } else {
         if (start & ~TARGET_PAGE_MASK) {
             errno = EINVAL;
@@ -619,6 +622,8 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int target_prot,
                      host_prot, flags, fd, offset1);
             if (p == MAP_FAILED)
                 goto fail;
+            passthrough_start = real_start;
+            passthrough_end = real_end;
         }
     }
  the_end1:
@@ -626,7 +631,18 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int target_prot,
         page_flags |= PAGE_ANON;
     }
     page_flags |= PAGE_RESET;
-    page_set_flags(start, start + len, page_flags);
+    if (passthrough_start == passthrough_end) {
+        page_set_flags(start, start + len, page_flags);
+    } else {
+        if (start < passthrough_start) {
+            page_set_flags(start, passthrough_start, page_flags);
+        }
+        page_set_flags(passthrough_start, passthrough_end,
+                       page_flags | PAGE_PASSTHROUGH);
+        if (passthrough_end < start + len) {
+            page_set_flags(passthrough_end, start + len, page_flags);
+        }
+    }
  the_end:
     trace_target_mmap_complete(start);
     if (qemu_loglevel_mask(CPU_LOG_PAGE)) {
@@ -845,7 +861,7 @@ static bool can_passthrough_madv_dontneed(abi_ulong start, abi_ulong end)
     }
 
     for (addr = start; addr < end; addr += TARGET_PAGE_SIZE) {
-        if (!(page_get_flags(addr) & PAGE_ANON)) {
+        if (!(page_get_flags(addr) & (PAGE_ANON | PAGE_PASSTHROUGH))) {
             return false;
         }
     }
@@ -888,7 +904,8 @@ abi_long target_madvise(abi_ulong start, abi_ulong len_in, int advice)
      *
      * This breaks MADV_DONTNEED, completely implementing which is quite
      * complicated. However, there is one low-hanging fruit: host-page-aligned
-     * anonymous mappings. In this case passthrough is safe, so do it.
+     * anonymous mappings or mappings that are known to have the same semantics
+     * in the host and the guest. In this case passthrough is safe, so do it.
      */
     mmap_lock();
     if ((advice & MADV_DONTNEED) &&
-- 
2.35.3

[PING] [PATCH v2] linux-user: Passthrough MADV_DONTNEED for certain file mappings

[Ilya Leoshkevich]

On Mon, 2022-07-25 at 14:50 +0200, Ilya Leoshkevich wrote:
> This is a follow-up for commit 892a4f6a750a ("linux-user: Add partial
> support for MADV_DONTNEED"), which added passthrough for anonymous
> mappings. File mappings can be handled in a similar manner.
> 
> In order to do that, mark pages, for which mmap() was passed through,
> with PAGE_PASSTHROUGH, and then allow madvise() passthrough for these
> pages as well.
> 
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> ---
> 
> v1:
> https://lists.gnu.org/archive/html/qemu-devel/2022-07/msg00112.html
> v1 -> v2: Fix PAGE_PASSTHROUGH value; make checks consistent with
>           page_set_flags() (Laurent).
> 
>  include/exec/cpu-all.h |  6 ++++++
>  linux-user/mmap.c      | 25 +++++++++++++++++++++----
>  2 files changed, 27 insertions(+), 4 deletions(-)
> 
> diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
> index f5bda2c3ca..2d29ba13c0 100644
> --- a/include/exec/cpu-all.h
> +++ b/include/exec/cpu-all.h
> @@ -262,6 +262,12 @@ extern const TargetPageBits target_page;
>  #define PAGE_TARGET_1  0x0200
>  #define PAGE_TARGET_2  0x0400
>  
> +/*
> + * For linux-user, indicates that the page is mapped with the same
> semantics
> + * in both guest and host.
> + */
> +#define PAGE_PASSTHROUGH 0x0800
> +
>  #if defined(CONFIG_USER_ONLY)
>  void page_dump(FILE *f);
>  
> diff --git a/linux-user/mmap.c b/linux-user/mmap.c
> index 4e7a6be6ee..79928e3ae5 100644
> --- a/linux-user/mmap.c
> +++ b/linux-user/mmap.c
> @@ -424,7 +424,8 @@ abi_ulong mmap_find_vma(abi_ulong start,
> abi_ulong size, abi_ulong align)
>  abi_long target_mmap(abi_ulong start, abi_ulong len, int
> target_prot,
>                       int flags, int fd, abi_ulong offset)
>  {
> -    abi_ulong ret, end, real_start, real_end, retaddr, host_offset,
> host_len;
> +    abi_ulong ret, end, real_start, real_end, retaddr, host_offset,
> host_len,
> +              passthrough_start = -1, passthrough_end = -1;
>      int page_flags, host_prot;
>  
>      mmap_lock();
> @@ -537,6 +538,8 @@ abi_long target_mmap(abi_ulong start, abi_ulong
> len, int target_prot,
>              host_start += offset - host_offset;
>          }
>          start = h2g(host_start);
> +        passthrough_start = start;
> +        passthrough_end = start + len;
>      } else {
>          if (start & ~TARGET_PAGE_MASK) {
>              errno = EINVAL;
> @@ -619,6 +622,8 @@ abi_long target_mmap(abi_ulong start, abi_ulong
> len, int target_prot,
>                       host_prot, flags, fd, offset1);
>              if (p == MAP_FAILED)
>                  goto fail;
> +            passthrough_start = real_start;
> +            passthrough_end = real_end;
>          }
>      }
>   the_end1:
> @@ -626,7 +631,18 @@ abi_long target_mmap(abi_ulong start, abi_ulong
> len, int target_prot,
>          page_flags |= PAGE_ANON;
>      }
>      page_flags |= PAGE_RESET;
> -    page_set_flags(start, start + len, page_flags);
> +    if (passthrough_start == passthrough_end) {
> +        page_set_flags(start, start + len, page_flags);
> +    } else {
> +        if (start < passthrough_start) {
> +            page_set_flags(start, passthrough_start, page_flags);
> +        }
> +        page_set_flags(passthrough_start, passthrough_end,
> +                       page_flags | PAGE_PASSTHROUGH);
> +        if (passthrough_end < start + len) {
> +            page_set_flags(passthrough_end, start + len,
> page_flags);
> +        }
> +    }
>   the_end:
>      trace_target_mmap_complete(start);
>      if (qemu_loglevel_mask(CPU_LOG_PAGE)) {
> @@ -845,7 +861,7 @@ static bool
> can_passthrough_madv_dontneed(abi_ulong start, abi_ulong end)
>      }
>  
>      for (addr = start; addr < end; addr += TARGET_PAGE_SIZE) {
> -        if (!(page_get_flags(addr) & PAGE_ANON)) {
> +        if (!(page_get_flags(addr) & (PAGE_ANON |
> PAGE_PASSTHROUGH))) {
>              return false;
>          }
>      }
> @@ -888,7 +904,8 @@ abi_long target_madvise(abi_ulong start,
> abi_ulong len_in, int advice)
>       *
>       * This breaks MADV_DONTNEED, completely implementing which is
> quite
>       * complicated. However, there is one low-hanging fruit: host-
> page-aligned
> -     * anonymous mappings. In this case passthrough is safe, so do
> it.
> +     * anonymous mappings or mappings that are known to have the
> same semantics
> +     * in the host and the guest. In this case passthrough is safe,
> so do it.
>       */
>      mmap_lock();
>      if ((advice & MADV_DONTNEED) &&

Hi,

I know it's quite late, but since you are going to take the aarch64
MADV_DONTNEED patch, would it be reasonable to take this one as well?

Best regards,
Ilya

[PING PATCH v2] linux-user: Passthrough MADV_DONTNEED for certain file mappings

[Ilya Leoshkevich]

On Mon, 2022-07-25 at 14:50 +0200, Ilya Leoshkevich wrote:
> This is a follow-up for commit 892a4f6a750a ("linux-user: Add partial
> support for MADV_DONTNEED"), which added passthrough for anonymous
> mappings. File mappings can be handled in a similar manner.
> 
> In order to do that, mark pages, for which mmap() was passed through,
> with PAGE_PASSTHROUGH, and then allow madvise() passthrough for these
> pages as well.
> 
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> ---
> 
> v1:
> https://lists.gnu.org/archive/html/qemu-devel/2022-07/msg00112.html
> v1 -> v2: Fix PAGE_PASSTHROUGH value; make checks consistent with
>           page_set_flags() (Laurent).
> 
>  include/exec/cpu-all.h |  6 ++++++
>  linux-user/mmap.c      | 25 +++++++++++++++++++++----
>  2 files changed, 27 insertions(+), 4 deletions(-)
> 
> diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
> index f5bda2c3ca..2d29ba13c0 100644
> --- a/include/exec/cpu-all.h
> +++ b/include/exec/cpu-all.h
> @@ -262,6 +262,12 @@ extern const TargetPageBits target_page;
>  #define PAGE_TARGET_1  0x0200
>  #define PAGE_TARGET_2  0x0400
>  
> +/*
> + * For linux-user, indicates that the page is mapped with the same
> semantics
> + * in both guest and host.
> + */
> +#define PAGE_PASSTHROUGH 0x0800
> +
>  #if defined(CONFIG_USER_ONLY)
>  void page_dump(FILE *f);
>  
> diff --git a/linux-user/mmap.c b/linux-user/mmap.c
> index 4e7a6be6ee..79928e3ae5 100644
> --- a/linux-user/mmap.c
> +++ b/linux-user/mmap.c
> @@ -424,7 +424,8 @@ abi_ulong mmap_find_vma(abi_ulong start,
> abi_ulong size, abi_ulong align)
>  abi_long target_mmap(abi_ulong start, abi_ulong len, int
> target_prot,
>                       int flags, int fd, abi_ulong offset)
>  {
> -    abi_ulong ret, end, real_start, real_end, retaddr, host_offset,
> host_len;
> +    abi_ulong ret, end, real_start, real_end, retaddr, host_offset,
> host_len,
> +              passthrough_start = -1, passthrough_end = -1;
>      int page_flags, host_prot;
>  
>      mmap_lock();
> @@ -537,6 +538,8 @@ abi_long target_mmap(abi_ulong start, abi_ulong
> len, int target_prot,
>              host_start += offset - host_offset;
>          }
>          start = h2g(host_start);
> +        passthrough_start = start;
> +        passthrough_end = start + len;
>      } else {
>          if (start & ~TARGET_PAGE_MASK) {
>              errno = EINVAL;
> @@ -619,6 +622,8 @@ abi_long target_mmap(abi_ulong start, abi_ulong
> len, int target_prot,
>                       host_prot, flags, fd, offset1);
>              if (p == MAP_FAILED)
>                  goto fail;
> +            passthrough_start = real_start;
> +            passthrough_end = real_end;
>          }
>      }
>   the_end1:
> @@ -626,7 +631,18 @@ abi_long target_mmap(abi_ulong start, abi_ulong
> len, int target_prot,
>          page_flags |= PAGE_ANON;
>      }
>      page_flags |= PAGE_RESET;
> -    page_set_flags(start, start + len, page_flags);
> +    if (passthrough_start == passthrough_end) {
> +        page_set_flags(start, start + len, page_flags);
> +    } else {
> +        if (start < passthrough_start) {
> +            page_set_flags(start, passthrough_start, page_flags);
> +        }
> +        page_set_flags(passthrough_start, passthrough_end,
> +                       page_flags | PAGE_PASSTHROUGH);
> +        if (passthrough_end < start + len) {
> +            page_set_flags(passthrough_end, start + len,
> page_flags);
> +        }
> +    }
>   the_end:
>      trace_target_mmap_complete(start);
>      if (qemu_loglevel_mask(CPU_LOG_PAGE)) {
> @@ -845,7 +861,7 @@ static bool
> can_passthrough_madv_dontneed(abi_ulong start, abi_ulong end)
>      }
>  
>      for (addr = start; addr < end; addr += TARGET_PAGE_SIZE) {
> -        if (!(page_get_flags(addr) & PAGE_ANON)) {
> +        if (!(page_get_flags(addr) & (PAGE_ANON |
> PAGE_PASSTHROUGH))) {
>              return false;
>          }
>      }
> @@ -888,7 +904,8 @@ abi_long target_madvise(abi_ulong start,
> abi_ulong len_in, int advice)
>       *
>       * This breaks MADV_DONTNEED, completely implementing which is
> quite
>       * complicated. However, there is one low-hanging fruit: host-
> page-aligned
> -     * anonymous mappings. In this case passthrough is safe, so do
> it.
> +     * anonymous mappings or mappings that are known to have the
> same semantics
> +     * in the host and the guest. In this case passthrough is safe,
> so do it.
>       */
>      mmap_lock();
>      if ((advice & MADV_DONTNEED) &&

I would like to ping this patch and two others that I used for
debugging it:

[PATCH] linux-user: Fix stracing in-memory mmap arguments
https://patchew.org/QEMU/20220630165901.2459135-1-iii@linux.ibm.com/

[PATCH] linux-user: Implement stracing madvise()
https://patchew.org/QEMU/20220725134440.172892-1-iii@linux.ibm.com/

[PATCH v2] linux-user: Passthrough MADV_DONTNEED for certain file
mappings
https://patchew.org/QEMU/20220725125043.43048-1-iii@linux.ibm.com/

Re: [PING PATCH v2] linux-user: Passthrough MADV_DONTNEED for certain file mappings

[Richard Henderson]

On 9/1/22 09:45, Ilya Leoshkevich wrote:
>> +/*
>> + * For linux-user, indicates that the page is mapped with the same
>> semantics
>> + * in both guest and host.
>> + */
>> +#define PAGE_PASSTHROUGH 0x0800

I would expect a change to PAGE_STICKY in accel/tcg/translate-all.c, so that this bit is 
preserved across mprotect.  Yes?

>> @@ -845,7 +861,7 @@ static bool
>> can_passthrough_madv_dontneed(abi_ulong start, abi_ulong end)
>>       }
>>   
>>       for (addr = start; addr < end; addr += TARGET_PAGE_SIZE) {
>> -        if (!(page_get_flags(addr) & PAGE_ANON)) {
>> +        if (!(page_get_flags(addr) & (PAGE_ANON |
>> PAGE_PASSTHROUGH))) {

Do you want both PAGE_ANON and PAGE_PASSTHROUGH?
If not, is PAGE_PASSTHOUGH is sufficient by itself, why check PAGE_ANON?

> I would like to ping this patch and two others that I used for
> debugging it:
> 
> [PATCH] linux-user: Fix stracing in-memory mmap arguments
> https://patchew.org/QEMU/20220630165901.2459135-1-iii@linux.ibm.com/

Queued to linux-user-next.

> [PATCH] linux-user: Implement stracing madvise()
> https://patchew.org/QEMU/20220725134440.172892-1-iii@linux.ibm.com/

There are many more MADV_* than just the 5 you list.


r~