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

[David Hildenbrand (Arm)]

On 10/2/26 14:48, Lorenzo Stoakes (ARM) wrote:
> On Fri, Oct 02, 2026 at 02:35:06PM +0200, David Hildenbrand (Arm) wrote:
>> On 10/2/26 14:08, Lorenzo Stoakes (ARM) wrote:
>>>
>>> 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.
> > 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.

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.

> 
>>
>>
>> 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).


> 
>>
>> 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.
>>
>>
>>> 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