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

Lorenzo Stoakes <[email protected]>
Newsgroups org.kernel.vger.linux-sgx,dev.linux.lists.ntfs3,dev.linux.lists.nvdimm,org.freedesktop.lists.dri-devel,org.freedesktop.lists.intel-gfx,org.kernel.vger.keyrings,org.kernel.vger.linux-cxl,org.kernel.vger.linux-ext4,org.kernel.vger.linux-fsdevel,org.kernel.vger.linux-kernel,org.kernel.vger.linux-security-module,org.kernel.vger.linux-xfs,org.kvack.linux-mm,org.ozlabs.lists.linux-erofs
Message-ID <[email protected]>
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?

If it looks like a duck, walks like a duck and quacks like a duck, but isn't
there when the pond is first dug out, can we still call it a duck? ;)

>
> It looks like we can make this a nice inlined (commented!) lower-cased
> C function as a little low-priority cleanup.
>
> > Rest LGTM though.
> >
> > Acked-by: Pedro Falcato <[email protected]>
>
> Great, thanks.

Cheers, Lorenzo
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.