Re: [PATCH 1/3] mm: change vm_get_page_prot() to accept vm_flags_t argument

Christian Brauner
2025-06-19T08:42:29Z

On Wed, Jun 18, 2025 at 08:42:52PM +0100, Lorenzo Stoakes wrote:
> We abstract the type of the VMA flags to vm_flags_t, however in may places
> it is simply assumed this is unsigned long, which is simply incorrect.
>
> At the moment this is simply an incongruity, however in future we plan to
> change this type and therefore this change is a critical requirement for
> doing so.
>
> Overall, this patch does not introduce any functional change.
>
> Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
> ---
> arch/arm64/mm/mmap.c | 2 +-
> arch/powerpc/include/asm/book3s/64/pkeys.h | 3 ++-
> arch/sparc/mm/init_64.c | 2 +-
> arch/x86/mm/pgprot.c | 2 +-
> include/linux/mm.h | 4 ++--
> include/linux/pgtable.h | 2 +-
> tools/testing/vma/vma_internal.h | 2 +-
> 7 files changed, 9 insertions(+), 8 deletions(-)
>
> diff --git a/arch/arm64/mm/mmap.c b/arch/arm64/mm/mmap.c
> index c86c348857c4..08ee177432c2 100644
> --- a/arch/arm64/mm/mmap.c
> +++ b/arch/arm64/mm/mmap.c
> @@ -81,7 +81,7 @@ static int __init adjust_protection_map(void)
> }
> arch_initcall(adjust_protection_map);
>
> -pgprot_t vm_get_page_prot(unsigned long vm_flags)
> +pgprot_t vm_get_page_prot(vm_flags_t vm_flags)
> {
> ptdesc_t prot;
>
> diff --git a/arch/powerpc/include/asm/book3s/64/pkeys.h b/arch/powerpc/include/asm/book3s/64/pkeys.h
> index 5b178139f3c0..6f2075636591 100644
> --- a/arch/powerpc/include/asm/book3s/64/pkeys.h
> +++ b/arch/powerpc/include/asm/book3s/64/pkeys.h
> @@ -4,8 +4,9 @@
> #define _ASM_POWERPC_BOOK3S_64_PKEYS_H
>
> #include <asm/book3s/64/hash-pkey.h>
> +#include <linux/mm_types.h>
>
> -static inline u64 vmflag_to_pte_pkey_bits(u64 vm_flags)
> +static inline u64 vmflag_to_pte_pkey_bits(vm_flags_t vm_flags)

If you change vm_flags_t to u64 you probably want to compile with some
of these integer truncation options when you're doing the conversion.
Because otherwise you risk silently truncating the upper 32bits when
assigning to a 32bit variable. We've had had a patch series that almost
introduced a very subtle bug when it tried to add the first flag outside
the 32bit range in the lookup code a while ago. That series never made
it but it just popped back into my head when I read your series.

Acked-by: Christian Brauner <brauner@kernel.org>