Re: [PATCH v4 1/3] bsd-user: Always use mmap_find_vma_aligned() in target_mmap()

[Warner Losh <imp@bsdimp.com>]

[-- Attachment #1: Type: text/plain, Size: 1590 bytes --]

On Sat, Mar 8, 2025 at 5:28 AM Philippe Mathieu-Daudé <philmd@linaro.org>
wrote:

> Massage target_mmap(): calculate alignment once, then
> unconditionally call mmap_find_vma_aligned().
>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  bsd-user/mmap.c | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
>

Reviewed-by: Warner Losh <imp@bsdimp.com>

Here I get why you have a new variable since it's a  little easier to read.

Warner


> diff --git a/bsd-user/mmap.c b/bsd-user/mmap.c
> index 346f2cefd32..dfa6e728ab5 100644
> --- a/bsd-user/mmap.c
> +++ b/bsd-user/mmap.c
> @@ -489,13 +489,12 @@ abi_long target_mmap(abi_ulong start, abi_ulong len,
> int prot,
>       * before we truncate the length for mapping files below.
>       */
>      if (!(flags & MAP_FIXED)) {
> +        abi_ulong alignment;
> +
>          host_len = len + offset - host_offset;
>          host_len = HOST_PAGE_ALIGN(host_len);
> -        if ((flags & MAP_ALIGNMENT_MASK) != 0)
> -            start = mmap_find_vma_aligned(real_start, host_len,
> -                (flags & MAP_ALIGNMENT_MASK) >> MAP_ALIGNMENT_SHIFT);
> -        else
> -            start = mmap_find_vma(real_start, host_len);
> +        alignment = (flags & MAP_ALIGNMENT_MASK) >> MAP_ALIGNMENT_SHIFT;
> +        start = mmap_find_vma_aligned(real_start, host_len, alignment);
>          if (start == (abi_ulong)-1) {
>              errno = ENOMEM;
>              goto fail;
> --
> 2.47.1
>
>

[-- Attachment #2: Type: text/html, Size: 2470 bytes --]