On 21.03.22 17:18, Jason Gunthorpe wrote: > On Mon, Mar 21, 2022 at 05:15:06PM +0100, David Hildenbrand wrote: >> On 19.03.22 00:30, Jason Gunthorpe wrote: >>> On Tue, Mar 15, 2022 at 11:47:39AM +0100, David Hildenbrand wrote: >>>> Whenever GUP currently ends up taking a R/O pin on an anonymous page that >>>> might be shared -- mapped R/O and !PageAnonExclusive() -- any write fault >>>> on the page table entry will end up replacing the mapped anonymous page >>>> due to COW, resulting in the GUP pin no longer being consistent with the >>>> page actually mapped into the page table. >>>> >>>> The possible ways to deal with this situation are: >>>> (1) Ignore and pin -- what we do right now. >>>> (2) Fail to pin -- which would be rather surprising to callers and >>>> could break user space. >>>> (3) Trigger unsharing and pin the now exclusive page -- reliable R/O >>>> pins. >>>> >>>> We want to implement 3) because it provides the clearest semantics and >>>> allows for checking in unpin_user_pages() and friends for possible BUGs: >>>> when trying to unpin a page that's no longer exclusive, clearly >>>> something went very wrong and might result in memory corruptions that >>>> might be hard to debug. So we better have a nice way to spot such >>>> issues. >>>> >>>> To implement 3), we need a way for GUP to trigger unsharing: >>>> FAULT_FLAG_UNSHARE. FAULT_FLAG_UNSHARE is only applicable to R/O mapped >>>> anonymous pages and resembles COW logic during a write fault. However, in >>>> contrast to a write fault, GUP-triggered unsharing will, for example, still >>>> maintain the write protection. >>> >>> Given the way this series has developed you might want to call this >>> FAULT_FLAG_MAKE_ANON_EXCLUSIVE >>> >>> Which strikes me as more directly connected to what it is trying to >>> do. >> >> I thought about something similar along those lines, and I think it >> would apply even when extending that mechanism to anything !anon inside >> a MAP_PRIVATE mapping. >> >> The whole >> >> const bool unshare = vmf->flags & FAULT_FLAG_UNSHARE; > > I think the extra words are worthwhile, share makes me think about > MAP_SHARED as we don't really use shared anywhere else FWICT.. Yeah, my point would be that you can only "unshare" in MAP_PRIVATE (!MAP_SHARED) :) -- Thanks, David / dhildenb
