Re: [PATCH 1/4] arm64, mm: Move generic mmap layout functions to mm

From: Christoph Hellwig
Date: Fri Mar 22 2019 - 09:21:42 EST


> It then introduces a new define ARCH_WANT_DEFAULT_TOPDOWN_MMAP_LAYOUT
> that can be defined by other architectures to benefit from those functions.

Can you make this a Kconfig option defined in arch/Kconfig or mm/Kconfig
and selected by the architectures?

> -#ifndef STACK_RND_MASK
> -#define STACK_RND_MASK (0x7ff >> (PAGE_SHIFT - 12)) /* 8MB of VA */
> -#endif
> -
> -static unsigned long randomize_stack_top(unsigned long stack_top)
> -{
> - unsigned long random_variable = 0;
> -
> - if (current->flags & PF_RANDOMIZE) {
> - random_variable = get_random_long();
> - random_variable &= STACK_RND_MASK;
> - random_variable <<= PAGE_SHIFT;
> - }
> -#ifdef CONFIG_STACK_GROWSUP
> - return PAGE_ALIGN(stack_top) + random_variable;
> -#else
> - return PAGE_ALIGN(stack_top) - random_variable;
> -#endif
> -}
> -

Maybe the move of this function can be split into another prep patch,
as it is only very lightly related?

> +#if defined(HAVE_ARCH_PICK_MMAP_LAYOUT) || \
> + defined(ARCH_WANT_DEFAULT_TOPDOWN_MMAP_LAYOUT)

Not sure if it is wrÑtten down somehwere or just convention, but I
general see cpp defined statements aligned with spaces to the
one on the previous line.

Except for these nitpicks this looks very nice to me, thanks for doing
this work!