On Fri, Mar 20, 2026 at 10:57:32AM +0100, Vlastimil Babka (SUSE) wrote: > On 3/18/26 16:50, Lorenzo Stoakes (Oracle) wrote: > > In order to be able to do this, we need to change VM_DATA_DEFAULT_FLAGS > > and friends and update the architecture-specific definitions also. > > > > We then have to update some KSM logic to handle VMA flags, and introduce > > VMA_STACK_FLAGS to define the vma_flags_t equivalent of VM_STACK_FLAGS. > > > > We also introduce two helper functions for use during the time we are > > converting legacy flags to vma_flags_t values - vma_flags_to_legacy() and > > legacy_to_vma_flags(). > > Nit: this was done by an earlier patch.
Ack will fix up. > > > This enables us to iteratively make changes to break these changes up into > > separate parts. > > > > We use these explicitly here to keep VM_STACK_FLAGS around for certain > > users which need to maintain the legacy vm_flags_t values for the time > > being. > > > > We are no longer able to rely on the simple VM_xxx being set to zero if > > the feature is not enabled, so in the case of VM_DROPPABLE we introduce > > VMA_DROPPABLE as the vma_flags_t equivalent, which is set to > > EMPTY_VMA_FLAGS if the droppable flag is not available. > > > > While we're here, we make the description of do_brk_flags() into a kdoc > > comment, as it almost was already. > > > > We use vma_flags_to_legacy() to not need to update the vm_get_page_prot() > > logic as this time. > > > > Note that in create_init_stack_vma() we have to replace the BUILD_BUG_ON() > > with a VM_WARN_ON_ONCE() as the tested values are no longer build time > > available. > > > > We also update mprotect_fixup() to use VMA flags where possible, though we > > have to live with a little duplication between vm_flags_t and vma_flags_t > > values for the time being until further conversions are made. > > > > Finally, we update the VMA tests to reflect these changes. > > > > Acked-by: Paul Moore <[email protected]> [SELinux] > > Signed-off-by: Lorenzo Stoakes (Oracle) <[email protected]> > > Acked-by: Vlastimil Babka (SUSE) <[email protected]> Thanks! > > More nits below: > > > diff --git a/arch/arm64/include/asm/page.h b/arch/arm64/include/asm/page.h > > index b39cc1127e1f..e25d0d18f6d7 100644 > > --- a/arch/arm64/include/asm/page.h > > +++ b/arch/arm64/include/asm/page.h > > @@ -46,7 +46,12 @@ int pfn_is_map_memory(unsigned long pfn); > > > > #endif /* !__ASSEMBLER__ */ > > > > -#define VM_DATA_DEFAULT_FLAGS (VM_DATA_FLAGS_TSK_EXEC | > > VM_MTE_ALLOWED) > > +#ifdef CONFIG_ARM64_MTE > > +#define VMA_DATA_DEFAULT_FLAGS > > append_vma_flags(VMA_DATA_FLAGS_TSK_EXEC, \ > > + VMA_MTE_ALLOWED_BIT) > > I wonder what's the bloat-o-meter impact of these #define's (this > arm64-specific one isn't the only one) being no longer compile-time-constants? I mean there's a precedent for this, but the compiler _should_ figure out this as a constant value, I have repeatedly confirmed that it's good at that in godbolt, via make foo/bar.S etc. So it should have no measureable impact at 64-bit VMA flags anyway, but I can give it a go and see before/after. > > > +#else > > +#define VMA_DATA_DEFAULT_FLAGS VMA_DATA_FLAGS_TSK_EXEC > > +#endif > > > > #include <asm-generic/getorder.h> > > > > > --- a/include/linux/mm.h > > +++ b/include/linux/mm.h > > > > > +#define VMA_STACK_FLAGS append_vma_flags(VMA_STACK_DEFAULT_FLAGS, > > \ > > + VMA_STACK_BIT, VMA_ACCOUNT_BIT) > > + > > +/* Temporary until VMA flags conversion complete. */ > > +#define VM_STACK_FLAGS vma_flags_to_legacy(VMA_STACK_FLAGS) > > + > > #define VM_STARTGAP_FLAGS (VM_GROWSDOWN | VM_SHADOW_STACK) > > > > #ifdef CONFIG_MSEAL_SYSTEM_MAPPINGS > > @@ -536,8 +547,6 @@ enum { > > #define VM_SEALED_SYSMAP VM_NONE > > #endif > > > > -#define VM_STACK_FLAGS (VM_STACK | VM_STACK_DEFAULT_FLAGS | VM_ACCOUNT) > > - > > /* VMA basic access permission flags */ > > #define VM_ACCESS_FLAGS (VM_READ | VM_WRITE | VM_EXEC) > > #define VMA_ACCESS_FLAGS mk_vma_flags(VMA_READ_BIT, VMA_WRITE_BIT, > > VMA_EXEC_BIT) > > @@ -547,6 +556,9 @@ enum { > > */ > > #define VM_SPECIAL (VM_IO | VM_DONTEXPAND | VM_PFNMAP | VM_MIXEDMAP) > > > > +#define VMA_SPECIAL_FLAGS mk_vma_flags(VMA_IO_BIT, VMA_DONTEXPAND_BIT, \ > > + VMA_PFNMAP_BIT, VMA_MIXEDMAP_BIT) > > Should VM_SPECIAL be also redefined using vma_flags_to_legacy()? Could do! It should be a pretty clear indicator this is legacy. > > > + > > /* > > * Physically remapped pages are special. Tell the > > * rest of the world about it: > > @@ -1412,7 +1424,7 @@ static __always_inline void > > vma_desc_set_flags_mask(struct vm_area_desc *desc, > > * vm_area_desc object describing a proposed VMA, e.g.: > > * > > * vma_desc_set_flags(desc, VMA_IO_BIT, VMA_PFNMAP_BIT, VMA_DONTEXPAND_BIT, > > - * VMA_DONTDUMP_BIT); > > + * VMA_DONTDUMP_BIT); > > Looks like spurious tabs-to-space change inconsistent with other instances. Yeah that's a mistake, will fixup. > > > */ > > #define vma_desc_set_flags(desc, ...) \ > > vma_desc_set_flags_mask(desc, mk_vma_flags(__VA_ARGS__)) > > @@ -4059,7 +4071,6 @@ extern int replace_mm_exe_file(struct mm_struct *mm, > > struct file *new_exe_file); > > extern struct file *get_mm_exe_file(struct mm_struct *mm); > > extern struct file *get_task_exe_file(struct task_struct *task); > > > > -extern bool may_expand_vm(struct mm_struct *, vm_flags_t, unsigned long > > npages); > > extern void vm_stat_account(struct mm_struct *, vm_flags_t, long npages); > > > > extern bool vma_is_special_mapping(const struct vm_area_struct *vma, > > diff --git a/mm/internal.h b/mm/internal.h > > index f98f4746ac41..80d8651441a7 100644 > > > diff --git a/mm/mprotect.c b/mm/mprotect.c > > index 9681f055b9fc..eaa724b99908 100644 > > --- a/mm/mprotect.c > > +++ b/mm/mprotect.c > > > @@ -773,19 +778,24 @@ mprotect_fixup(struct vma_iterator *vmi, struct > > mmu_gather *tlb, > > > > change_protection(tlb, vma, start, end, mm_cp_flags); > > > > - if ((oldflags & VM_ACCOUNT) && !(newflags & VM_ACCOUNT)) > > + if (vma_flags_test(&old_vma_flags, VMA_ACCOUNT_BIT) && > > + !vma_flags_test(&new_vma_flags, VMA_ACCOUNT_BIT)) > > vm_unacct_memory(nrpages); > > > > /* > > * Private VM_LOCKED VMA becoming writable: trigger COW to avoid major > > * fault on access. > > */ > > - if ((oldflags & (VM_WRITE | VM_SHARED | VM_LOCKED)) == VM_LOCKED && > > - (newflags & VM_WRITE)) { > > - populate_vma_page_range(vma, start, end, NULL); > > + if (vma_flags_test(&new_vma_flags, VMA_WRITE_BIT)) { > > + const vma_flags_t mask = > > + vma_flags_and(&old_vma_flags, VMA_WRITE_BIT, > > + VMA_SHARED_BIT, VMA_LOCKED_BIT); > > + > > + if (vma_flags_same(&mask, VMA_LOCKED_BIT)) > > That converts the original logic 1:1, but I wonder if it's now feasible to > write it more obviously as "VMA_LOCKED_BIT must be set, VM_WRITE_BIT and > VM_SHARED_BIT must not" ? Hmm, I'm not sure if I can express this more clearly, it's a pain either way. Could do: if (vma_flags_test(&new_vma_flags, VMA_WRITE_BIT) && !vma_flags_test_any(&old_vma_flags, VMA_WRITE_BIT, VMA_SHARED_BIT)) populate_vma_page_range(vma, start, end, NULL); > > > + populate_vma_page_range(vma, start, end, NULL); > > } > > > > - vm_stat_account(mm, oldflags, -nrpages); > > + vm_stat_account(mm, vma_flags_to_legacy(old_vma_flags), -nrpages); > > vm_stat_account(mm, newflags, nrpages); > > perf_event_mmap(vma); > > return 0; > > > diff --git a/mm/vma.h b/mm/vma.h > > index cf8926558bf6..1f2de6cb3b97 100644 > > --- a/mm/vma.h > > +++ b/mm/vma.h > > > +static inline bool is_data_mapping_vma_flags(const vma_flags_t *vma_flags) > > +{ > > + const vma_flags_t mask = vma_flags_and(vma_flags, > > + VMA_WRITE_BIT, VMA_SHARED_BIT, VMA_STACK_BIT); > > + > > + return vma_flags_same(&mask, VMA_WRITE_BIT); > > Ditto? I may do both as a follow up patch given the series is a pain to rework at this point and I want at least the first version to be like-for-like intentionally. > > > +} > > > > static inline void vma_iter_config(struct vma_iterator *vmi, > > unsigned long index, unsigned long last) > > diff --git a/mm/vma_exec.c b/mm/vma_exec.c > > index 8134e1afca68..5cee8b7efa0f 100644 Thanks, Lorenzo
