Re: [PULL 35/47] linux-user: Use page_find_range_empty for mmap_find_vma_reserved

[Laurent Vivier <laurent@vivier.eu>]

Hi Richard,

thank you for the linux-user PR (I have really no time to do that).

I've  run the LTP test suite (20230127) on master and found some problems introduced by 
this patch.

With armhf and Debian bionic and stretch, brk01 brk02 recvmsg01 fail:

tst_test.c:1560: TINFO: Timeout per run is 0h 00m 30s
brk01.c:24: TINFO: Testing libc variant
brk01.c:54: TFAIL: brk() failed: ENOMEM (12)
brk01.c:61: TFAIL: brk() failed to set address have 0x40049000 expected 0x4004afff
brk01.c:21: TINFO: Testing syscall variant
brk01.c:61: TFAIL: brk() failed to set address have 0x40049000 expected 0x4004afff

tst_test.c:1560: TINFO: Timeout per run is 0h 00m 30s
brk02.c:42: TINFO: Testing libc variant
brk02.c:27: TFAIL: brk() failed: ENOMEM (12)
brk02.c:60: TFAIL: Cannot expand brk() by page size: ENOMEM (12)
brk02.c:39: TINFO: Testing syscall variant
brk02.c:60: TFAIL: Cannot expand brk() by page size: EFAULT (14)

recvmsg01    1  TPASS  :  bad file descriptor successful
recvmsg01    2  TPASS  :  invalid socket successful
recvmsg01    3  TPASS  :  invalid socket buffer successful
recvmsg01    4  TPASS  :  invalid socket length successful
recvmsg01    5  TPASS  :  invalid recv buffer successful
recvmsg01    6  TPASS  :  invalid iovec buffer successful
recvmsg01    7  TPASS  :  invalid iovec count successful
recvmsg01    8  TPASS  :  rights reception successful
recvmsg01    9  TPASS  :  invalid MSG_OOB flag set successful
recvmsg01   10  TPASS  :  invalid MSG_ERRQUEUE flag set successful
recvmsg01   11  TPASS  :  invalid cmsg length successful
recvmsg01   12  TFAIL  :  recvmsg01.c:236: large cmesg length ; returned -1 (expected 0), 
errno 14 (expected 0)

With mips/stretch and mipsel/stretch I have one more, sbrk01:

sbrk01      1  TFAIL  :  sbrk01.c:102: sbrk - Increase by 8192 bytes failed: 
TEST_ERRNO=ENOMEM(12): Cannot allocate memory
sbrk01      2  TPASS  :  sbrk - Increase by -8192 bytes returned 0x4005f000

Thanks,
Laurent

On 7/15/23 15:53, Richard Henderson wrote:
> Use the interval tree to find empty space, rather than
> probing each page in turn.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> Message-Id: <20230707204054.8792-19-richard.henderson@linaro.org>
> ---
>   linux-user/mmap.c | 52 ++++++-----------------------------------------
>   1 file changed, 6 insertions(+), 46 deletions(-)
> 
> diff --git a/linux-user/mmap.c b/linux-user/mmap.c
> index c4b2515271..738b9b797d 100644
> --- a/linux-user/mmap.c
> +++ b/linux-user/mmap.c
> @@ -318,55 +318,15 @@ unsigned long last_brk;
>   static abi_ulong mmap_find_vma_reserved(abi_ulong start, abi_ulong size,
>                                           abi_ulong align)
>   {
> -    abi_ulong addr, end_addr, incr = qemu_host_page_size;
> -    int prot;
> -    bool looped = false;
> +    target_ulong ret;
>   
> -    if (size > reserved_va) {
> -        return (abi_ulong)-1;
> +    ret = page_find_range_empty(start, reserved_va, size, align);
> +    if (ret == -1 && start > mmap_min_addr) {
> +        /* Restart at the beginning of the address space. */
> +        ret = page_find_range_empty(mmap_min_addr, start - 1, size, align);
>       }
>   
> -    /* Note that start and size have already been aligned by mmap_find_vma. */
> -
> -    end_addr = start + size;
> -    /*
> -     * Start at the top of the address space, ignoring the last page.
> -     * If reserved_va == UINT32_MAX, then end_addr wraps to 0,
> -     * throwing the rest of the calculations off.
> -     * TODO: rewrite using last_addr instead.
> -     * TODO: use the interval tree instead of probing every page.
> -     */
> -    if (start > reserved_va - size) {
> -        end_addr = ((reserved_va - size) & -align) + size;
> -        looped = true;
> -    }
> -
> -    /* Search downward from END_ADDR, checking to see if a page is in use.  */
> -    addr = end_addr;
> -    while (1) {
> -        addr -= incr;
> -        if (addr > end_addr) {
> -            if (looped) {
> -                /* Failure.  The entire address space has been searched.  */
> -                return (abi_ulong)-1;
> -            }
> -            /* Re-start at the top of the address space (see above). */
> -            addr = end_addr = ((reserved_va - size) & -align) + size;
> -            looped = true;
> -        } else {
> -            prot = page_get_flags(addr);
> -            if (prot) {
> -                /* Page in use.  Restart below this page.  */
> -                addr = end_addr = ((addr - size) & -align) + size;
> -            } else if (addr && addr + size == end_addr) {
> -                /* Success!  All pages between ADDR and END_ADDR are free.  */
> -                if (start == mmap_next_start) {
> -                    mmap_next_start = addr;
> -                }
> -                return addr;
> -            }
> -        }
> -    }
> +    return ret;
>   }
>   
>   /*