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 (!mmu_has_feature(MMU_FTR_PKEY))
 		return 0x0UL;
diff --git a/arch/sparc/mm/init_64.c b/arch/sparc/mm/init_64.c
index 25ae4c897aae..7ed58bf3aaca 100644
--- a/arch/sparc/mm/init_64.c
+++ b/arch/sparc/mm/init_64.c
@@ -3201,7 +3201,7 @@ void copy_highpage(struct page *to, struct page *from)
 }
 EXPORT_SYMBOL(copy_highpage);
 
-pgprot_t vm_get_page_prot(unsigned long vm_flags)
+pgprot_t vm_get_page_prot(vm_flags_t vm_flags)
 {
 	unsigned long prot = pgprot_val(protection_map[vm_flags &
 					(VM_READ|VM_WRITE|VM_EXEC|VM_SHARED)]);
diff --git a/arch/x86/mm/pgprot.c b/arch/x86/mm/pgprot.c
index c84bd9540b16..dc1afd5c839d 100644
--- a/arch/x86/mm/pgprot.c
+++ b/arch/x86/mm/pgprot.c
@@ -32,7 +32,7 @@ void add_encrypt_protection_map(void)
 		protection_map[i] = pgprot_encrypted(protection_map[i]);
 }
 
-pgprot_t vm_get_page_prot(unsigned long vm_flags)
+pgprot_t vm_get_page_prot(vm_flags_t vm_flags)
 {
 	unsigned long val = pgprot_val(protection_map[vm_flags &
 				      (VM_READ|VM_WRITE|VM_EXEC|VM_SHARED)]);
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 98a606908307..7a7cd2e1b2af 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -3487,10 +3487,10 @@ static inline bool range_in_vma(struct vm_area_struct *vma,
 }
 
 #ifdef CONFIG_MMU
-pgprot_t vm_get_page_prot(unsigned long vm_flags);
+pgprot_t vm_get_page_prot(vm_flags_t vm_flags);
 void vma_set_page_prot(struct vm_area_struct *vma);
 #else
-static inline pgprot_t vm_get_page_prot(unsigned long vm_flags)
+static inline pgprot_t vm_get_page_prot(vm_flags_t vm_flags)
 {
 	return __pgprot(0);
 }
diff --git a/include/linux/pgtable.h b/include/linux/pgtable.h
index 1d4439499503..cf1515c163e2 100644
--- a/include/linux/pgtable.h
+++ b/include/linux/pgtable.h
@@ -2001,7 +2001,7 @@ typedef unsigned int pgtbl_mod_mask;
  *								x: (yes) yes
  */
 #define DECLARE_VM_GET_PAGE_PROT					\
-pgprot_t vm_get_page_prot(unsigned long vm_flags)			\
+pgprot_t vm_get_page_prot(vm_flags_t vm_flags)				\
 {									\
 		return protection_map[vm_flags &			\
 			(VM_READ | VM_WRITE | VM_EXEC | VM_SHARED)];	\
diff --git a/tools/testing/vma/vma_internal.h b/tools/testing/vma/vma_internal.h
index d7fea56e3bb3..4e3a2f1ac09e 100644
--- a/tools/testing/vma/vma_internal.h
+++ b/tools/testing/vma/vma_internal.h
@@ -581,7 +581,7 @@ static inline pgprot_t pgprot_modify(pgprot_t oldprot, pgprot_t newprot)
 	return __pgprot(pgprot_val(oldprot) | pgprot_val(newprot));
 }
 
-static inline pgprot_t vm_get_page_prot(unsigned long vm_flags)
+static inline pgprot_t vm_get_page_prot(vm_flags_t vm_flags)
 {
 	return __pgprot(vm_flags);
 }
-- 
2.49.0

On 19/06/25 1:12 AM, 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 (!mmu_has_feature(MMU_FTR_PKEY))
>  		return 0x0UL;
> diff --git a/arch/sparc/mm/init_64.c b/arch/sparc/mm/init_64.c
> index 25ae4c897aae..7ed58bf3aaca 100644
> --- a/arch/sparc/mm/init_64.c
> +++ b/arch/sparc/mm/init_64.c
> @@ -3201,7 +3201,7 @@ void copy_highpage(struct page *to, struct page *from)
>  }
>  EXPORT_SYMBOL(copy_highpage);
>  
> -pgprot_t vm_get_page_prot(unsigned long vm_flags)
> +pgprot_t vm_get_page_prot(vm_flags_t vm_flags)
>  {
>  	unsigned long prot = pgprot_val(protection_map[vm_flags &
>  					(VM_READ|VM_WRITE|VM_EXEC|VM_SHARED)]);
> diff --git a/arch/x86/mm/pgprot.c b/arch/x86/mm/pgprot.c
> index c84bd9540b16..dc1afd5c839d 100644
> --- a/arch/x86/mm/pgprot.c
> +++ b/arch/x86/mm/pgprot.c
> @@ -32,7 +32,7 @@ void add_encrypt_protection_map(void)
>  		protection_map[i] = pgprot_encrypted(protection_map[i]);
>  }
>  
> -pgprot_t vm_get_page_prot(unsigned long vm_flags)
> +pgprot_t vm_get_page_prot(vm_flags_t vm_flags)
>  {
>  	unsigned long val = pgprot_val(protection_map[vm_flags &
>  				      (VM_READ|VM_WRITE|VM_EXEC|VM_SHARED)]);
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 98a606908307..7a7cd2e1b2af 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -3487,10 +3487,10 @@ static inline bool range_in_vma(struct vm_area_struct *vma,
>  }
>  
>  #ifdef CONFIG_MMU
> -pgprot_t vm_get_page_prot(unsigned long vm_flags);
> +pgprot_t vm_get_page_prot(vm_flags_t vm_flags);
>  void vma_set_page_prot(struct vm_area_struct *vma);
>  #else
> -static inline pgprot_t vm_get_page_prot(unsigned long vm_flags)
> +static inline pgprot_t vm_get_page_prot(vm_flags_t vm_flags)
>  {
>  	return __pgprot(0);
>  }
> diff --git a/include/linux/pgtable.h b/include/linux/pgtable.h
> index 1d4439499503..cf1515c163e2 100644
> --- a/include/linux/pgtable.h
> +++ b/include/linux/pgtable.h
> @@ -2001,7 +2001,7 @@ typedef unsigned int pgtbl_mod_mask;
>   *								x: (yes) yes
>   */
>  #define DECLARE_VM_GET_PAGE_PROT					\
> -pgprot_t vm_get_page_prot(unsigned long vm_flags)			\
> +pgprot_t vm_get_page_prot(vm_flags_t vm_flags)				\
>  {									\
>  		return protection_map[vm_flags &			\
>  			(VM_READ | VM_WRITE | VM_EXEC | VM_SHARED)];	\
> diff --git a/tools/testing/vma/vma_internal.h b/tools/testing/vma/vma_internal.h
> index d7fea56e3bb3..4e3a2f1ac09e 100644
> --- a/tools/testing/vma/vma_internal.h
> +++ b/tools/testing/vma/vma_internal.h
> @@ -581,7 +581,7 @@ static inline pgprot_t pgprot_modify(pgprot_t oldprot, pgprot_t newprot)
>  	return __pgprot(pgprot_val(oldprot) | pgprot_val(newprot));
>  }
>  
> -static inline pgprot_t vm_get_page_prot(unsigned long vm_flags)
> +static inline pgprot_t vm_get_page_prot(vm_flags_t vm_flags)
>  {
>  	return __pgprot(vm_flags);
>  }

LGTM

Reviewed-by: Anshuman Khandual <anshuman.khandual@arm.com>

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 +-

For arm64:

Acked-by: Catalin Marinas <catalin.marinas@arm.com>

On 18 Jun 2025, at 15:42, 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(-)
>
Acked-by: Zi Yan <ziy@nvidia.com>
--
Best Regards,
Yan, Zi

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>

Nice little cleanup, thanks!

Reviewed-by: Pedro Falcato <pfalcato@suse.de>

-- 
Pedro

Hi Andrew,

I enclose a quick fix-patch to address a case I missed and to avoid any risk of
circular dependency in a header include.

Thanks to Vlastimil and Oscar for spotting this! :)

Cheers, Lorenzo

----8<----
From d66fe0b934098ccc2ba45f739277fffe86c91442 Mon Sep 17 00:00:00 2001
From: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
Date: Thu, 19 Jun 2025 13:21:15 +0100
Subject: [PATCH] mm: add missing vm_get_page_prot() instance, remove include

I missed a case for powerpc, also remove #include (that is not in practice
necessary) to avoid any risk of circular dependency.

Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
---
 arch/powerpc/include/asm/book3s/64/pkeys.h | 1 -
 arch/powerpc/mm/book3s64/pgtable.c         | 2 +-
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/powerpc/include/asm/book3s/64/pkeys.h b/arch/powerpc/include/asm/book3s/64/pkeys.h
index 6f2075636591..ff911b4251d9 100644
--- a/arch/powerpc/include/asm/book3s/64/pkeys.h
+++ b/arch/powerpc/include/asm/book3s/64/pkeys.h
@@ -4,7 +4,6 @@
 #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(vm_flags_t vm_flags)
 {
diff --git a/arch/powerpc/mm/book3s64/pgtable.c b/arch/powerpc/mm/book3s64/pgtable.c
index b38cd0b6af13..c9431ae7f78a 100644
--- a/arch/powerpc/mm/book3s64/pgtable.c
+++ b/arch/powerpc/mm/book3s64/pgtable.c
@@ -642,7 +642,7 @@ unsigned long memremap_compat_align(void)
 EXPORT_SYMBOL_GPL(memremap_compat_align);
 #endif

-pgprot_t vm_get_page_prot(unsigned long vm_flags)
+pgprot_t vm_get_page_prot(vm_flags_t vm_flags)
 {
 	unsigned long prot;

--
2.49.0

On 19.06.25 14:25, Lorenzo Stoakes wrote:
> Hi Andrew,
> 
> I enclose a quick fix-patch to address a case I missed and to avoid any risk of
> circular dependency in a header include.
> 
> Thanks to Vlastimil and Oscar for spotting this! :)
> 
> Cheers, Lorenzo
> 
> ----8<----
>  From d66fe0b934098ccc2ba45f739277fffe86c91442 Mon Sep 17 00:00:00 2001
> From: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
> Date: Thu, 19 Jun 2025 13:21:15 +0100
> Subject: [PATCH] mm: add missing vm_get_page_prot() instance, remove include
> 
> I missed a case for powerpc, also remove #include (that is not in practice
> necessary) to avoid any risk of circular dependency.
> 
> Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
> ---

Acked-by: David Hildenbrand <david@redhat.com>

-- 
Cheers,

David / dhildenb

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
                                                                  many?
> 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>

Reviewed-by: Oscar Salvador <osalvador@suse.de>

Same comments as Vlastimil, you want to push vmflag_to_pte_pkey_bits
further to patch#2 and bring 

arch/powerpc/mm/book3s64/pgtable.c:pgprot_t vm_get_page_prot(unsigned long vm_flags)


-- 
Oscar Salvador
SUSE Labs

On Thu, Jun 19, 2025 at 02:12:49PM +0200, Oscar Salvador wrote:
> 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
>                                                                   many?
> > 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>
>
> Reviewed-by: Oscar Salvador <osalvador@suse.de>
>
> Same comments as Vlastimil, you want to push vmflag_to_pte_pkey_bits
> further to patch#2 and bring

As I said to him, this is invoked by vm_get_page_prot() so has to be updated too
as a consequence, so it belongs here imo. Also to be (extremely) pedantic if we
were to push it it would be to patch 3/3 :P

>
> arch/powerpc/mm/book3s64/pgtable.c:pgprot_t vm_get_page_prot(unsigned long vm_flags)

Yeah I checked so much and of course, of course... :P

>
>
> --
> Oscar Salvador
> SUSE Labs

On 6/18/25 21:42, 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>

> 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>

Hopefully not causing a circular header include.

> -static inline u64 vmflag_to_pte_pkey_bits(u64 vm_flags)
> +static inline u64 vmflag_to_pte_pkey_bits(vm_flags_t vm_flags)

Is this change rather for patch 3? It's not changing vm_get_page_prot().
OTOH git grep shows me you missed:

arch/powerpc/mm/book3s64/pgtable.c:pgprot_t vm_get_page_prot(unsigned long
vm_flags)

With that sorted out, feel free to add:

Reviewed-by: Vlastimil Babka <vbabka@suse.cz>

Thanks!

On Thu, Jun 19, 2025 at 01:31:50PM +0200, Vlastimil Babka wrote:
> On 6/18/25 21:42, 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>
>
> > 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>
>
> Hopefully not causing a circular header include.

Well bots should say if so :) these headers would surely be broken if that were
the case.

However, since the only caller is arch/powerpc/mm/book3s64/pgtable.c and that
already imports mm_types.h I will drop this to be safe.

>
> > -static inline u64 vmflag_to_pte_pkey_bits(u64 vm_flags)
> > +static inline u64 vmflag_to_pte_pkey_bits(vm_flags_t vm_flags)
>
> Is this change rather for patch 3? It's not changing vm_get_page_prot().
> OTOH git grep shows me you missed:

No it's necessary as it's called by vm_get_page_prot().

>
> arch/powerpc/mm/book3s64/pgtable.c:pgprot_t vm_get_page_prot(unsigned long
> vm_flags)

Ugh I checked and double checked this and yet somehow... :)

Let me send a fix-patch for removing the include and converting this.

>
> With that sorted out, feel free to add:
>
> Reviewed-by: Vlastimil Babka <vbabka@suse.cz>
>
> Thanks!

Thanks!

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>

On Thu, Jun 19, 2025 at 10:42:14AM +0200, Christian Brauner wrote:
> 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.

Yeah am very wary of this, it's a real concern. I'm not sure how precisely we
might enable such options but only in this instance? Because presumably we are
intentionally narrowing in probably quite a few places.

Pedro mentioned that there might be compiler options to help so I'm guessing
this is the same thing as to what you're thinking here?

I also considered a sparse flag, Pedro mentioned bitwise, but then I worry that
we'd have to __force in a million places to make that work and it'd be
non-obvious.

Matthew's original concept for this was to simply wrap an array, but that'd
require a complete rework of every single place where we use VMA flags (perhaps
we could mitigate it a _bit_ with a vm_flags_val() helper that grabs a u64?)

Something like this would be safest, but might be a lot of churn even for me ;)

The 'quickest' solution is to use u64 and somehow have a means of telling the
compiler 'error out if anybody narrows this'.

At any rate, I've gone with doing the safest thing _first_ which is to fixup use
of this typedef and neatly deferred all these messy decisions until later :>)

I am likely to fiddle around with the different proposed solutions and find the
most workable.

But I think overall we need _something_ here.

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

Thanks!

On Thu, Jun 19, 2025 at 09:49:03AM +0100, Lorenzo Stoakes wrote:
> On Thu, Jun 19, 2025 at 10:42:14AM +0200, Christian Brauner wrote:
> > 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.
> 
> Yeah am very wary of this, it's a real concern. I'm not sure how precisely we
> might enable such options but only in this instance? Because presumably we are
> intentionally narrowing in probably quite a few places.
> 
> Pedro mentioned that there might be compiler options to help so I'm guessing
> this is the same thing as to what you're thinking here?

I was thinking about -Wnarrowing but sadly it seems that this is only for C++
code. Also MSVC is quite strict (even in C) when it comes to this stuff, so you
could also add MSVC support to the kernel, small task :P

One could in theory add support for this stuff in GCC, but I would expect it
to flag almost everything in the kernel (e.g long -> int implicit conversions).
> 
> I also considered a sparse flag, Pedro mentioned bitwise, but then I worry that
> we'd have to __force in a million places to make that work and it'd be
> non-obvious.

Here's an example for __bitwise usage taken from block:

typedef unsigned int __bitwise blk_insert_t;
#define BLK_MQ_INSERT_AT_HEAD		((__force blk_insert_t)0x01)


then in block/blk-mq.c:

	if (flags & BLK_MQ_INSERT_AT_HEAD)
		list_add(&rq->queuelist, &hctx->dispatch);


So doing regular old flag checks with the bitwise & operator seems to work fine.
Assignment itself should also just work. So as long as we're vm_flags_t-typesafe
there should be no problem? I think.

> 
> Matthew's original concept for this was to simply wrap an array, but that'd
> require a complete rework of every single place where we use VMA flags (perhaps
> we could mitigate it a _bit_ with a vm_flags_val() helper that grabs a u64?)
> 

I think the real question is whether we expect to ever require > 64 flags for
VMAs? If so, going with an array would be the best option here. Though in that
case I would guess we probably want to hide the current vm_flags member in
vm_area_struct first, providing some vm_flags_is_set() and whatnot.

-- 
Pedro