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

[Helge Deller]

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