Re: [PATCH RESEND 09/12] mm: make vm_area_desc utilise vma_flags_t only

[Lorenzo Stoakes <lorenzo.stoakes@oracle.com>]

On Mon, Jan 19, 2026 at 07:14:03PM -0400, Jason Gunthorpe wrote:
> On Mon, Jan 19, 2026 at 09:19:11PM +0000, Lorenzo Stoakes wrote:
> > +static inline bool is_shared_maywrite(vma_flags_t flags)
> > +{
>
> I'm not sure it is ideal to pass this array by value? Seems like it
> might invite some negative optimizations since now the compiler has to
> optimze away a copy too.

I really don't think so? This is inlined and thus collapses to a totally
standard vma_flags_test_all() which passes by value anyway.

Which is in itself passing-an-array-by-value, and also inlined (though we force
the inline).

I have done a bunch of assembly generation and it has all indicated the compiler
handles this perfectly well.

It in fact is the basis of being able to pretty well like-for-like replace
vm_flags_t with vma_flags_t which makes the transition significantly better (and
allows for the mk_vma_flags() helper and associated helper macros).

Do you have specific examples or evidence the compiler will optimise poorly here
on that basis as compared to pass by reference? And pass by reference would
necessitate:

vma_flags_test_all(*flags, ...);

Or changing the whole approach? My experience generating assembly doesn't
suggest any of this is necessary.

Explicitly for this case, generating the invocation from
generic_file_readonly_mmap_prepare():

	if (is_shared_maywrite(desc->vma_flags))
		return -EINVAL;

Is:

	notl	%ecx
	testb	$40, %cl
	je	.LBB106_6 [ return -EINVAL ]

In this implementation and:

	notl	%ecx
	testb	$40, %cl
	je	.LBB106_6 [ return -EINVAL ]

Without my series (I am omitting movl $-22, %eax in both as this just sets up
the -EINVAL return).

i.e. exactly identical.

My experience so far is this is _always_ the case, the compiler is very adept at
optimising these kinds of things.

Thanks, Lorenzo