Re: [PATCH 3/3] riscv: mm: Do not restrict mmap address based on hint

[Levi Zim <rsworktech@outlook.com>]

On 2024-08-27 00:36, Charlie Jenkins wrote:
> The hint address should not forcefully restrict the addresses returned
> by mmap as this causes mmap to report ENOMEM when there is memory still
> available.
>
> Signed-off-by: Charlie Jenkins <charlie@rivosinc.com>
> Fixes: b5b4287accd7 ("riscv: mm: Use hint address in mmap if available")
> Fixes: add2cc6b6515 ("RISC-V: mm: Restrict address space for sv39,sv48,sv57")
> Closes: https://lore.kernel.org/linux-kernel/ZbxTNjQPFKBatMq+@ghost/T/#mccb1890466bf5a488c9ce7441e57e42271895765
> ---
>   arch/riscv/include/asm/processor.h | 26 ++------------------------
>   1 file changed, 2 insertions(+), 24 deletions(-)
>
> diff --git a/arch/riscv/include/asm/processor.h b/arch/riscv/include/asm/processor.h
> index 8702b8721a27..efa1b3519b23 100644
> --- a/arch/riscv/include/asm/processor.h
> +++ b/arch/riscv/include/asm/processor.h
> @@ -14,36 +14,14 @@
>   
>   #include <asm/ptrace.h>
>   
> -/*
> - * addr is a hint to the maximum userspace address that mmap should provide, so
> - * this macro needs to return the largest address space available so that
> - * mmap_end < addr, being mmap_end the top of that address space.
> - * See Documentation/arch/riscv/vm-layout.rst for more details.
> - */
>   #define arch_get_mmap_end(addr, len, flags)			\
>   ({								\
> -	unsigned long mmap_end;					\
> -	typeof(addr) _addr = (addr);				\
> -	if ((_addr) == 0 || is_compat_task() ||			\
> -	    ((_addr + len) > BIT(VA_BITS - 1)))			\
> -		mmap_end = STACK_TOP_MAX;			\
> -	else							\
> -		mmap_end = (_addr + len);			\
> -	mmap_end;						\
> +	STACK_TOP_MAX;						\
>   })
>   
>   #define arch_get_mmap_base(addr, base)				\
>   ({								\
> -	unsigned long mmap_base;				\
> -	typeof(addr) _addr = (addr);				\
> -	typeof(base) _base = (base);				\
> -	unsigned long rnd_gap = DEFAULT_MAP_WINDOW - (_base);	\
> -	if ((_addr) == 0 || is_compat_task() || 		\
> -	    ((_addr + len) > BIT(VA_BITS - 1)))			\
> -		mmap_base = (_base);				\
> -	else							\
> -		mmap_base = (_addr + len) - rnd_gap;		\
> -	mmap_base;						\
> +	base;							\
>   })
>   
>   #ifdef CONFIG_64BIT
>
I tested this patch on 6.10.2 kernel and could confirm that it fixes the 
crash of chromium. But I think I prefer Yangyu Chen's approach because 
that would avoid breaking some applications on sv57.

Tested-by: Levi Zim <rsworktech@outlook.com> # Chromium, sv39