On 10.12.24 10:51, Lorenzo Stoakes wrote:
David,
Hi Lorenzo,
sorry for the late reply.
To avoid an infinite back-and-forth...
I think you're stuck on the idea that we are sat in a VMA 'vacuum', perhaps
because I put so much effort into separating out the VMA logic, for the
purpose of being able to userland test it, it's almost given the wrong
impression (I should perhaps have put less effort into this? :)
Yes, that nicely summarizes it.
In particular, because the patch description says "group all VMA-related
files", and I am having a hard time to even identify *what* a
"VMA-related" file is, really.
For example, why not mempolicy.c->mbind()? Not that I would suggest
that, because the file is filled with non-vma-related stuff.
See below of what might make sense to me.
But we have for quite some time now de facto been expected to maintain all
aspects of mapping, remapping, etc. INCLUDING page table considerations.
> > We are more or less de facto maintaining all that (modulo madvise.c - I
grant you this is the odd one out).
So you can imagine it's a bit frustrating, when that's the case, to be told
in effect - no this isn't for you - right?
It isn't "VMA" group for me, independent of "who" is currently listed
there. And maybe we now agree on that, maybe not.
Talking about things that are frustrating: being called a "senior member
of the kernel". :)
> > For instance, as I said before, we have spent large parts of the
6.12 cycle
dealing with practical concerns specifically around page table
manipulation.
> > Maintainership is more than setting up lei rules, obviously. One
can set
lei rules for anything. It means that our say has more impact, and it also
means that we are (co-)responsible for the listed files, and that's acked
rather than disregarded.
> > So, again, I politely disagree with your assessment re: page tables.
> > I think their manipulation under circumstances where you
map/remap/mprotect/mlock is -inseparable- from memory mapping/VMA
logic. Otherwise, what's the point right?
We'll likely have to agree to disagree. :) But again, my main point is
that the VMA group is misleading.
As a side note, I believe the code can be structures accordingly (call
that separating it). mmap/munmap handling is the prime example right now
for me.
For example, I really enjoy how:
munmap() (mmap.c) routes to __vm_munmap (vma.c) makes use of abstracted
page table logic like free_pgtables() and unmap_vmas() (memory.c).
This way it is clear what the rather high-level MM syscall
implementation is (mmap.c), what the VMA handling is (vma.c) and what
the abstracted page table handling logic is (memory.c). I don't think
that page table handling code should be moved to either mmap.c or vma.c.
I would enjoy if we would handle e.g., mprotect.c in a similar way, such
that the helper like change_protection() -- again, used by completely
mprotect()-unrelated code outside of mprotect.c -- would not reside in
mprotect.c. But that code just evolved naturally after we kept reusing
change_protection() outside of the file.
Regarding mremap logic there once was a discussion about merging it with
the uffdio_move logic, but not sure if that really makes sense.
My suggestion is that:
a. we put everything in MEMORY MAPPING
b. We drop mm/madvise.c from the list
Sounds better, although I am still fuzzy on what is supposed to be in
there and what not. See my mbind() example above ..
I see how mmap/munmap/mprotect/mremap fall into the same category of MM
syscalls that mainly affect the /proc/self/maps output (in contrast to a
bunch of others). Which make sense to me to group in such a way.
mseal.c and mlock.c likely as well.
msync.c is a simple VMA walker ... not sure if it belongs in there, but
who am I to tell.
--
Cheers,
David / dhildenb
