There is no reason to treat VM_SEALED in a special way, in each other case
in which a VMA flag is unavailable due to configuration, we simply assign
that flag to VM_NONE, so make VM_SEALED consistent with all other VMA
flags in this respect.
Additionally, use the next available bit for VM_SEALED, 42, rather than
arbitrarily putting it at 63 and update the declaration to match all other
VMA flags.
No functional change intended.
Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
Reviewed-by: Liam R. Howlett <Liam.Howlett@oracle.com>
Reviewed-by: Pedro Falcato <pfalcato@suse.de>
Acked-by: David Hildenbrand <david@redhat.com>
---
include/linux/mm.h | 6 ++++--
tools/testing/vma/vma_internal.h | 6 ++++--
2 files changed, 8 insertions(+), 4 deletions(-)
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 805108d7bbc3..fbf2a1f7ffc6 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -414,8 +414,10 @@ extern unsigned int kobjsize(const void *objp);
#endif
#ifdef CONFIG_64BIT
-/* VM is sealed, in vm_flags */
-#define VM_SEALED _BITUL(63)
+#define VM_SEALED_BIT 42
+#define VM_SEALED BIT(VM_SEALED_BIT)
+#else
+#define VM_SEALED VM_NONE
#endif
/* Bits set in the VMA until the stack is in its final location */
diff --git a/tools/testing/vma/vma_internal.h b/tools/testing/vma/vma_internal.h
index 991022e9e0d3..0fe52fd6782b 100644
--- a/tools/testing/vma/vma_internal.h
+++ b/tools/testing/vma/vma_internal.h
@@ -108,8 +108,10 @@ extern unsigned long dac_mmap_min_addr;
#define CAP_IPC_LOCK 14
#ifdef CONFIG_64BIT
-/* VM is sealed, in vm_flags */
-#define VM_SEALED _BITUL(63)
+#define VM_SEALED_BIT 42
+#define VM_SEALED BIT(VM_SEALED_BIT)
+#else
+#define VM_SEALED VM_NONE
#endif
#define FIRST_USER_ADDRESS 0UL
--
2.50.1
Hi Lorenzo, On Wed, Jul 16, 2025 at 10:38 AM Lorenzo Stoakes <lorenzo.stoakes@oracle.com> wrote: > > There is no reason to treat VM_SEALED in a special way, in each other case > in which a VMA flag is unavailable due to configuration, we simply assign > that flag to VM_NONE, so make VM_SEALED consistent with all other VMA > flags in this respect. > Originally, I wanted to avoid using VM_NONE for VM_SEALED to catch compiler errors in 32-bit builds. This would serve as a safeguard against inadvertently using VM_SEALED in code paths shared between 32-bit and 64-bit architectures. Take an example of show_smap_vma_flags [1] : #ifdef CONFIG_64BIT [ilog2(VM_SEALED)] = "sl", #endif If a developer forgets to add #ifdef CONFIG_64BIT, the build will fail for 32-bit systems. With VM_SEALED redefined as VM_NONE, the problem will go unnoticed. This coding practice is more defensive and helps catch errors early on. It seems that you're working on introducing VM_SEALED for 32-bit systems. If that happens, we won't need this safeguard anymore. But until then, is it OK to keep it in place? That said, if VM_SEALED support for 32-bit is coming really soon, we can merge this change, however, perhaps you could update the description to explain why we're removing this safeguard, i.e. due to 32-bit support will soon be in place. Link: https://elixir.bootlin.com/linux/v6.15.7/source/fs/proc/task_mmu.c#L1001 [1] Thanks and regards, -Jeff > Additionally, use the next available bit for VM_SEALED, 42, rather than > arbitrarily putting it at 63 and update the declaration to match all other > VMA flags. > > No functional change intended. > > Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com> > Reviewed-by: Liam R. Howlett <Liam.Howlett@oracle.com> > Reviewed-by: Pedro Falcato <pfalcato@suse.de> > Acked-by: David Hildenbrand <david@redhat.com> > --- > include/linux/mm.h | 6 ++++-- > tools/testing/vma/vma_internal.h | 6 ++++-- > 2 files changed, 8 insertions(+), 4 deletions(-) > > diff --git a/include/linux/mm.h b/include/linux/mm.h > index 805108d7bbc3..fbf2a1f7ffc6 100644 > --- a/include/linux/mm.h > +++ b/include/linux/mm.h > @@ -414,8 +414,10 @@ extern unsigned int kobjsize(const void *objp); > #endif > > #ifdef CONFIG_64BIT > -/* VM is sealed, in vm_flags */ > -#define VM_SEALED _BITUL(63) > +#define VM_SEALED_BIT 42 > +#define VM_SEALED BIT(VM_SEALED_BIT) > +#else > +#define VM_SEALED VM_NONE > #endif > > /* Bits set in the VMA until the stack is in its final location */ > diff --git a/tools/testing/vma/vma_internal.h b/tools/testing/vma/vma_internal.h > index 991022e9e0d3..0fe52fd6782b 100644 > --- a/tools/testing/vma/vma_internal.h > +++ b/tools/testing/vma/vma_internal.h > @@ -108,8 +108,10 @@ extern unsigned long dac_mmap_min_addr; > #define CAP_IPC_LOCK 14 > > #ifdef CONFIG_64BIT > -/* VM is sealed, in vm_flags */ > -#define VM_SEALED _BITUL(63) > +#define VM_SEALED_BIT 42 > +#define VM_SEALED BIT(VM_SEALED_BIT) > +#else > +#define VM_SEALED VM_NONE > #endif > > #define FIRST_USER_ADDRESS 0UL > -- > 2.50.1 >
On Thu, Jul 24, 2025 at 11:34:31AM -0700, Jeff Xu wrote: > Hi Lorenzo, > > On Wed, Jul 16, 2025 at 10:38 AM Lorenzo Stoakes > <lorenzo.stoakes@oracle.com> wrote: > > > > There is no reason to treat VM_SEALED in a special way, in each other case > > in which a VMA flag is unavailable due to configuration, we simply assign > > that flag to VM_NONE, so make VM_SEALED consistent with all other VMA > > flags in this respect. > > > Originally, I wanted to avoid using VM_NONE for VM_SEALED to catch > compiler errors in 32-bit builds. This would serve as a safeguard > against inadvertently using VM_SEALED in code paths shared between > 32-bit and 64-bit architectures. I understand why you did it, and I fundamentally disagree. This would make this the only VMA flag in the entire kernel having this special treatment for something with very little implementation code, it simply makes no sense. The commit message makes this clear. > > Take an example of show_smap_vma_flags [1] : > > #ifdef CONFIG_64BIT > [ilog2(VM_SEALED)] = "sl", > #endif > > If a developer forgets to add #ifdef CONFIG_64BIT, the build will fail This is a really silly thing to worry about. > for 32-bit systems. With VM_SEALED redefined as VM_NONE, the problem > will go unnoticed. What problem exactly? > > This coding practice is more defensive and helps catch errors early It's silly. > on. It seems that you're working on introducing VM_SEALED for 32-bit > systems. If that happens, we won't need this safeguard anymore. But > until then, is it OK to keep it in place? That said, if VM_SEALED > support for 32-bit is coming really soon, we can merge this change, > however, perhaps you could update the description to explain why we're > removing this safeguard, i.e. due to 32-bit support will soon be in > place. No I won't, because that's not why I did it.
© 2016 - 2025 Red Hat, Inc.