Re: [PATCH v2 09/13] mm: update all remaining mmap_prepare users to use vma_flags_t

* Lorenzo Stoakes <[email protected]> [260206 20:01]:
> On Fri, Feb 06, 2026 at 11:31:53AM -0800, Andrew Morton wrote:
> > On Fri, 6 Feb 2026 17:46:36 +0000 Pedro Falcato <[email protected]> wrote:
> >
> > > > -#define VM_REMAP_FLAGS (VM_IO | VM_PFNMAP | VM_DONTEXPAND | VM_DONTDUMP)
> > > > +#define VMA_REMAP_FLAGS mk_vma_flags(VMA_IO_BIT, VMA_PFNMAP_BIT,	\
> > > > +				     VMA_DONTEXPAND_BIT, VMA_DONTDUMP_BIT)
> > >
> > > as a sidenote, these flags are no longer constant expressions and thus
> > >
> > > static vma_flags_t flags = VMA_REMAP_FLAGS;
> 
> I mean this would be a code smell anyway :) but point taken.
> 
> > >
> > > can't compile.
> >
> > Yup, that isn't nice.  An all-caps thing with no () is a compile-time
> > constant.
> 
> There is precedence for this, e.g. TASK_SIZE_MAX and other arch defines like
> that:
> 
>  error: initializer element is not a compile-time constant
>  3309 | static unsigned long task_max = TASK_SIZE_MAX;
>       |                                 ^~~~~~~~~~~~~
> 
> And this will almost certainly (and certainly in everything I tested) become a
> compile-time constant via the optimiser so to all intents and purposes it _is_
> essentially compile-time.
> 
> But the point of doing it this way is to maintain, as much as possible,
> one-to-one translation between the previous approach and the new with as little
> noise/friction as possible.
> 
> Making this a function makes things really horrible honestly.
> 
> Because vma_remap_flags() suddenly because a vague thing - I'd assume this was a
> function doing something. So now do we call it get_vma_remap_flags()? Suddenly
> something nice-ish like:
> 
> 	if (vma_flags_test(flags, VMA_REMAP_FLAGS)) {
> 		...
> 	}
> 
> Become:
> 
> 	if (vma_flags_test(flags, get_vma_remap_flags())) {
> 		...
> 	}
> 
> And now it's SUPER ambiguous as to what you're doing there. I'd assume right
> away that get_vma_remap_flags() was going off and doing something or referencing
> a static variable or something.
> 
> Given the compile will treat the former _exactly_ as if it were a compile-time
> constant it's just adding unnecessary ambiguity.
> 
> So is it something we can live with?

How many of these are there going to be?  Could we do something like:

        static inline remap_vma_flags_test(..) {
                return vma_flags_test_all(vma_flags, ...);
        }

        if (remap_vma_flags_test(vma_flags)) {
                ...
        }