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

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

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

quoted

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

quoted

No, see below.


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.


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.

quoted

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.

quoted

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.

quoted

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.

I think it's perfectly fine to instead have helper like

	vma_is_dumpable()

that maybe have a single purpose but are centralized and well documented. If we
can construct them out of other helpers, great.

quoted

vma_is_user_memory() && !vma_is_droppable_memory()

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

If we can find some way to describe "anon+pagecache" ... maybe something around
folios (future oriented).

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

Sure, no strong opinion on that. It's a bit easier on the eye than bit checks.
(and can have a nicer description maybe).

quoted

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

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?

I'd be very happy if we could find a clear way to describe "this is what we call
user memory: anon+pagecache", if that could come handy in such a context.

In the end, it's mostly all about: this is nothing special (lol), it's just
ordinary user memory (anon+pagecache), and in some cases we want to also ignore
weird droppable mappings, because why e.g., dump them if the context is just
irrelevant.

-- 
Cheers,

David