Re: [PATCH] linux-user/elfload: use MAP_FIXED in pgb_reserved_va

[Richard Henderson <richard.henderson@linaro.org>]

On 6/30/20 7:41 AM, Alex Bennée wrote:
> 
> Peter Maydell <peter.maydell@linaro.org> writes:
> 
>> On Tue, 30 Jun 2020 at 11:36, Alex Bennée <alex.bennee@linaro.org> wrote:
>>>
>>> Given we assert the requested address matches what we asked we should
>>> also make that clear in the mmap flags. Otherwise we see failures in
>>> the GitLab environment for some currently unknown but allowable
>>> reason.
>>
>> Adding MAP_FIXED will mean that instead of failing if there's
>> something else already at that address, the kernel will now
>> silently blow that away in favour of the new mapping. Is
>> that definitely what we want here ?
> 
> Hmm maybe not.

Definitely not.

> But hey I just noticed that we have MAP_FIXED_NOREPLACE
> (since Linux 4.17) which says:
> 
>    This flag provides behavior that is similar  to  MAP_FIXED  with
>    respect   to   the   addr   enforcement,  but  differs  in  that
>    MAP_FIXED_NOREPLACE never clobbers a preexisting  mapped  range.
>    If  the  requested range would collide with an existing mapping,
>    then this call fails with  the  error  EEXIST.   This  flag  can
>    therefore  be used as a way to atomically (with respect to other
>    threads) attempt to map an address range: one thread  will  suc‐
>    ceed; all others will report failure.
> 
>    Note   that   older   kernels   which   do   not  recognize  the
>    MAP_FIXED_NOREPLACE flag will typically (upon detecting a colli‐
>    sion  with a preexisting mapping) fall back to a "non-MAP_FIXED"
>    type of behavior: they will return an address that is  different
>    from  the  requested  address.   Therefore,  backward-compatible
>    software should check the returned address against the requested
>    address.
> 
> So maybe that is what we should do?

Yes, that would be better, because those are the exact semantics that we want.
 Though it would be Really Nice to know what's up with gitlab...

> Now you've pointed that out I wonder if we need to fix
> pgd_find_hole_fallback as well?

Yes, that could benefit from MAP_FIXED_NOREPLACE.

I do think there's a way we could streamline the 32-on-64 case.  At present we
are groveling through /proc/self/maps, or mmaping+unmaping, and then mmaping.
Whereas we could just mmap once and be done -- it's the 32-on-32 case that
requires the song and dance.


r~