Re: [PATCH for-8.1 v10 10/14] util/selfmap: Rewrite using qemu/interval-tree.h

[Ilya Leoshkevich <iii@linux.ibm.com>]

On Mon, 2023-08-07 at 11:17 -0700, Richard Henderson wrote:
> On 8/7/23 09:37, Richard Henderson wrote:
> > We will want to be able to search the set of mappings.
> > For this patch, the two users iterate the tree in order.
> > 
> > Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> > ---
> >   include/qemu/selfmap.h |  20 ++++----
> >   linux-user/elfload.c   |  14 +++--
> >   linux-user/syscall.c   |  15 +++---
> >   util/selfmap.c         | 114 +++++++++++++++++++++++++-----------
> > -----
> >   4 files changed, 96 insertions(+), 67 deletions(-)
> 
> I should note that, for 8.2, this will enable a rewrite of
> open_self_maps_1 so that it 
> does not require page-by-page checking of page_get_flags.
> 
> My idea is that open_self_maps_1 would use walk_memory_regions to see
> all guest memory 
> regions.  The per-region callback would cross-check with the host-
> region interval tree to 
> find the dev+inode+path.
> 
> Cc Ilya and Helge, since there are two outstanding changes to
> open_self_maps.
> 
> 
> r~

My outstanding change should not be sensitive to this; it should be
possible to put it in both before or after the rewrite.



I really like this idea though, since I looked into ppc64le and there
printing maps is quite broken: it's not just that QEMU can't determine
the names of the mapped files, but also a number of regions are simply
missing. This also affects core dumps generated by GDB attached to
gdbstub.

For example, cat /proc/self/maps has the following internal page
layout:

start            end              size             prot
0000000010000000-000000001000d000 000000000000d000 r-x
000000001000d000-0000000010010000 0000000000003000 ---
0000000010010000-000000001001f000 000000000000f000 r--
000000001001f000-0000000010020000 0000000000001000 r--
0000000010020000-0000000010021000 0000000000001000 rw-
0000100000000000-0000100000010000 0000000000010000 ---
0000100000010000-0000100000810000 0000000000800000 rw-
0000100000810000-0000100000830000 0000000000020000 r-x
0000100000830000-000010000083d000 000000000000d000 r-x
000010000083d000-0000100000840000 0000000000003000 ---
0000100000840000-000010000084f000 000000000000f000 r--
000010000084f000-0000100000850000 0000000000001000 r--
0000100000850000-0000100000851000 0000000000001000 rw-
0000100000851000-0000100000852000 0000000000001000 rw-
0000100000860000-0000100000861000 0000000000001000 r-x
0000100000880000-0000100000a50000 00000000001d0000 r-x
0000100000a50000-0000100000a60000 0000000000010000 r--
0000100000a60000-0000100000a70000 0000000000010000 rw-
0000100000a70000-0000100000b70000 0000000000100000 rw-
0000100000b70000-000010000742d000 00000000068bd000 r--
00007fffb22b0000-00007fffb22e0000 0000000000030000 rw-

but prints only:

100000000000-100000010000 ---p 00000000 00:00 0                       
100000010000-100000810000 rw-p 00000000 00:00 0                       
[stack]
100000810000-100000830000 r-xp 00000000 fd:00 3049136                 
/usr/lib64/ld-2.17.so
100000880000-100000a50000 r-xp 00000000 fd:00 3017372                 
/usr/lib64/libc-2.17.so
100000a50000-100000a60000 r--p 001c0000 fd:00 3017372                 
/usr/lib64/libc-2.17.so
100000a60000-100000a70000 rw-p 001d0000 fd:00 3017372                 
/usr/lib64/libc-2.17.so
100000a70000-100000b70000 rw-p 00000000 00:00 0                       
7fffb22b0000-7fffb22e0000 rw-p 00000000 00:00 0                       

I don't see a good way to prevent page_check_range() from rejecting
most of the mappings with the current code structure, but I think that
after the proposed rewrite it should begin to just work.