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