On Wed, Apr 26, 2023 at 10:49:47AM -0600, Khalid Aziz wrote:
> This patch series adds a new flag to mmap() call - MAP_SHARED_PT.
Since hugetlb has this, it'll be very helpful if you can provide a
comparison between this approach and hugetlb's - especially on the
differences - and reasonings about those.
Merging anything like this definitely should also pave way for hugetlb's
future, so it even seems to be an "requirement" of such patchset even
though implicitily.. considering the "hotness" that hugetlb recently has
on refactoring demand (if not a rewrite).
Some high level questions:
- Why mmap() flag?
For this one, I agree it should be opt-in - sharing pgtable definitely
means sharing of a lot of privileges operating on current mm, so one
should be aware and be prepared to be messed up.
IIUC hugetlb doesn't do this but instead when something "racy" happens
itt just unshares by default. To me opt-in makes more sense if to
start from zero, because I don't think by default a process wants to
leak its mm to others.
I think you mentioned allowing pgtable to be shared even for mprotect()
from one MM then all MMs can see; but if so then DONTNEED should really
do the same - when one MM DONTNEED it it should go away for all. It
doesn't make a lot of sense to me to treat it differently with a
DONTNEED refcount anywhere..
- Can guest MM opt-out?
Should we allow guest MM to opt-out when they want? It sounds like a
good thing to have to me, especially for file that sounds like as
simple as zapping the pgtable. But then mmap flag will stop working
iiuc, so goes back to that question (e.g. what about madvise or prctl?).
- Why mm_struct to represent shared pgtable?
IIUC hugetlb used the pgtable page itself plus some refcounting (the
refcounting is racy with gup-fast that Jann used to point out, but
that's another story..). My question is do you think that won't work?
Are there reasons to explain? Why mm_struct is the structure you chose
for representing a shared pgtable? Why per-file?
Thanks,
--
Peter Xu
