On Thu, Dec 16, 2021 at 11:19:17AM -0400, Jason Gunthorpe wrote: > On Thu, Dec 16, 2021 at 01:56:57PM +0000, Matthew Wilcox wrote: > > p = mmap(x, 2MB, PROT_READ|PROT_WRITE, ...): THP allocated > > mprotect(p, 4KB, PROT_READ): THP split. > > > > And in that case, I would say the THP now has mapcount of 2 because > > there are 2 VMAs mapping it. > > At least today mapcount is only loosely connected to VMAs. It really > counts the number of PUD/PTEs that point at parts of the memory. Careful. Currently, you need to distinguish between total_mapcount(), page_trans_huge_mapcount() and page_mapcount(). Take a look at __page_mapcount() to be sure you really know what the mapcount "really" counts today ... (also I'm going to assume that when you said PUD you really mean PMD throughout) > If, under the PTL, you observe a mapcount of 1 then you know that the > PUD/PTE you have under lock is the ONLY PUD/PTE that refers to this > page and will remain so while the lock is held. > > So, today the above ends up with a mapcount of 1 and when we take a > COW fault we can re-use the page. > > If the above ends up with a mapcount of 2 then COW will copy not > re-use, which will cause unexpected data corruption in all those > annoying side cases. As I understood David's presentation yesterday, we actually have data corruption issues in all the annoying side cases with THPs in current upstream, so that's no worse than we have now. But let's see if we can avoid them. It feels like what we want from a COW perspective is a count of the number of MMs mapping a page, not the number of VMAs, PTEs or PMDs mapping the page. Right? So here's a corner case ... p = mmap(x, 2MB, PROT_READ|PROT_WRITE, ...): THP allocated mremap(p + 128K, 128K, 128K, MREMAP_MAYMOVE | MREMAP_FIXED, p + 2MB): PMD split Should mapcount be 1 or 2 at this point? Does the answer change if it's an anonymous or file page? (I think it might; perhaps if you do this to an anon THP, we split the anon THP and each page is now mapped once, whereas if you do it to a file page, the page cache does not split the page, but it does count the extra mapping. also the answer may be different between MAP_PRIVATE and MAP_SHARED) > The actual value of mapcount doesn't matter, the main issue is if it > is 1 or not, and if 1 the determination needs to be reliable and race > free. > > Putting the mapcount on the head page is sort of an optimization 'all > tail pages share the same mapcount' and the doublemap thing is about > exiting that optimization with minimal locking. > > There is other stuff going on too that can possibly do other things > but this seems to be the primary difficult task. > > (IIRC anyhow, from when I looked at this with David a few months ago) > > > I'm just trying to learn enough to make sensible suggestions for > > simplification. As yesterday's call proved, there are all kinds of > > corner cases when messing with mapcount and refcount. > > It would be amazing to get some better idea! > > Jason >
