Re: [PATCH v7 00/14] linux-user: brk fixes

[Helge Deller <deller@gmx.de>]

On 8/3/23 17:11, Richard Henderson wrote:
> On 8/3/23 08:01, Helge Deller wrote:
>> * Joel Stanley <joel@jms.id.au>:
>>> On Thu, 3 Aug 2023 at 13:55, Helge Deller <deller@gmx.de> wrote:
>>>>> 143551 brk(NULL) = 0x0009b000
>>>>> 143551 brk(0x0009b8fc) = 0x0009b000
>>>>
>>>> I think the problem is the brk with 9b000 here.
>>>> It's not 64k aligned (=pages size of your ppc64le).
>>>>
>>>> Please try with this patch on top of Richard's series:
>>>>
>>>>> @@ -3229,7 +3208,8 @@ static void load_elf_image(const char *image_name, int image_fd,
>>>>>        info->end_code = 0;
>>>>>        info->start_data = -1;
>>>>>        info->end_data = 0;
>>>>> -    info->brk = .....
>>>> change that to become:
>>>>       info->brk = HOST_PAGE_ALIGN(hiaddr);
>>>
>>> That stopped the crashing, and the binaries seem to run fine. I tested
>>> on two hosts: ppc64le (64K) and arm64 (16K).
>>
>> Great!
>>
>> That made re-read Akihiko's patch:
>> ----
>> Author: Akihiko Odaki <akihiko.odaki@daynix.com>
>>      linux-user: Do not align brk with host page size
>>
>>      do_brk() minimizes calls into target_mmap() by aligning the address
>>      with host page size, which is potentially larger than the target page
>>      size. However, the current implementation of this optimization has two
>>      bugs:
>>
>>      - The start of brk is rounded up with the host page size while brk
>>        advertises an address aligned with the target page size as the
>>        beginning of brk. This makes the beginning of brk unmapped.
>> ----
>> this patch has wrong assumptions.
>>
>> The start of brk always needs to be host page aligned.
>
>
> There is a bunch of code in target_mmap that attempts to manage
> adjacent guest pages that fall into the same host page.  Akihiko's
> patch assumes that code actually works.  Which I think is entirely
> reasonable.

Ok.

> You can't move brk up like this either (without other adjustments to
> the binary mapping), since that will leave a hole in the guest
> address space, which can get filled with something else later, which
> will definitely cause problems.

Ah, true. I have to admit that I didn't thought of that yet.
What is a possible solution to increase brk then (if you agree
that it should be increased here if necessary) ?
Call target_mmap() on the area from current brk to new brk?

Helge