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

Liam R. Howlett
2026-02-09T19:29:08Z

* Lorenzo Stoakes <lorenzo.stoakes@oracle.com> [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 <pfalcato@suse.de> 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)) {
...
}