Re: [PATCH v3 31/40] mm/vma: introduce vma[_flags]_is_persistent()

On Fri, Oct 02, 2026 at 02:35:06PM +0200, David Hildenbrand (Arm) wrote:

On 10/2/26 14:08, Lorenzo Stoakes (ARM) wrote:

quoted

On Fri, Oct 02, 2026 at 09:05:15AM +0200, David Hildenbrand (Arm) wrote:

quoted

On 10/2/26 09:02, David Hildenbrand (Arm) wrote:

No, see below.

quoted

quoted

It's also about droppable mappings AFAIKs. How many more users will we have for
that function?

Anything that requires stuff not to be dropped behind the user's back, which is
at least 4 cases!

That being open-coded all over the place is a problem I think, and I think stuff
like the PMD device private are a reminder that open-coding all over can cause
problems.

quoted

quoted

If it's "no others" then please don't add a helper function with misleading
names for it and just keep the special "dumpable" check in the new form in
madvise_vma_behavior().

Talking to myself ... the more usage I see of the vma_is_persistent() the more I
think this shouldn't be a helper at all. Especially not one with such a
confusing name :P

There are 4 open-coded checks that test four ad-hoc flag combinations checking
for the same thing - 'can the kernel or a driver change things or discard stuff
behind my back?'

So abstracting that to a helper, alongside the other 'let's ask based on
semantics' helpers, seems sensible.

Maybe invert the meaning to make it clearer?

	vma_kernel_may_change_contents()?

This is all super confusing and I don't think we should try to describe the
semantics that way.

Just imagine having udmabuf use a PFNMAP of folios obtained from shmem. For sure
the kernel could now change the shmem pages that are mapped in some ordinary VMA.

There's always edge cases like this.

In each of these conditionals that this helper replaces, that's exactly what's
being checked for.

A driver could GUP anything and access the page raw and do things like that
too...

Without this we lose all that and are back to arbitrary flag checks again. It
seems unwise.

Likely we don't have to squeeze everything into a single helper that is hard to
describe.

I'm not trying to do that :)

There are plenty of edge cases I left alone.

This one explicitly is one that can end up very easily broken in the future.

Already there's been issues as I recall with people forgetting about droppable.

Maybe we can pull parts of it into a separate helper with semantics that are
easier to describe?

But then you lose the whole purpose of this, which is to not miss things.

vma_is_user_memory() && !vma_is_droppable_memory()

I find vma_is_user_memory() _very_ confusing :) I have no idea what that means.

And vma_is_droppable_memory() is a semi-pointless wrapper for checking the
droppable flag no?

Although I am not sure user_memory is exactly precise (pagecache+anon) and what
we want? It's all super confusing (thanks for deciphering it).

That name isn't great, as userland memory is defined as that which can be mapped
in the userland portion of the virtual memory address map, and all VMAs describe
that :)

And then you can go on from there as to pedantic issues, the very same shmem
stuff you mention above can be put forward as an argument, etc. etc.

quoted

vma_contents_may_change() is shorter but easily confused with something being
writable by userland etc.

Or maybe:

	vma_is_volatile()

?

Which is analogous to the meaning of the volatile keyword.

This is all confusing because persistent and volatile are established concept
when talking about memory. And see my example above, it's not even clear what it
means that "the kernel can modify something".

Yep I guess volatile vs. non-volatile.

But a VMA describes a mapping not backing and 'volatile' in C is pretty clear -
something can be touched by something else so don't make assumptions.

I mean if people are confused they can look at the comment. We argue about
details meanwhile the code is full of terrible naming that's duplicated 100
times :)

And you can come up with edge cases to a lot of things. That doesn't invalidate
the intent.

Right now the code is duplicative and I found multiple instances of e.g. drivers
doing completely the wrong thing.

In any case, you seem to feel very strongly about this - so maybe best I drop
this patch from the series to be revisited later?

--
Cheers,

David

--
Cheers, Lorenzo