On Wed, Sep 22, 2021 at 08:21:40PM +0200, David Hildenbrand wrote: > On 22.09.21 19:51, Peter Xu wrote: > > We forbid merging thps for uffd-wp enabled regions, by breaking the khugepaged > > scanning right after we detected a uffd-wp armed pte (either present, or swap). > > > > It works, but it's less efficient, because those ptes only exist for VM_UFFD_WP > > enabled VMAs. Checking against the vma flag would be more efficient, and good > > enough. To be explicit, we could still be able to merge some thps for > > VM_UFFD_WP regions before this patch as long as they have zero uffd-wp armed > > ptes, however that's not a major target for thp collapse anyways. > > > > Hm, are we sure there are no users that could benefit from the current > handling? > > I'm thinking about long-term uffd-wp users that effectively end up wp-ing on > only a small fraction of a gigantic vma, or always wp complete blocks in a > certain granularity in the range of THP. Yes, that's a good question. > > Databases come to mind ... One thing to mention is that this patch didn't forbid thp being used within a uffd-wp-ed range. E.g., we still allow thp exist, we can uffd-wp a thp and it'll split only until when the thp is written. While what this patch does is it stops khugepaged from proactively merging those small pages into thps as long as VM_UFFD_WP|VM_UFFD_MINOR is set. It may still affect some user, but it's not a complete disable on thp. > > In the past, I played with the idea of using uffd-wp to protect access to > logically unplugged memory regions part of virtio-mem devices in QEMU -- > which would exactly do something as described above. But I'll most probably > be using ordinary uffd once any users that might read such logically > unplugged memory have been "fixed". Yes, even if you'd like to keep using uffd-wp that sounds like a very reasonable scenario. > > The change itself looks sane to me AFAIKT. So one major motivation of this patch of mine is to prepare for shmem, because the old commit obviously only covered anonymous. But after a 2nd thought, I just noticed shmem shouldn't have a problem with khugepaged merging at all! The thing is, when khugepaged is merging a shmem thp, unlike anonymous, it'll not merge the ptes into a pmd, but it'll simply zap the ptes. It means all uffd-wp tracking information won't be lost even if merging happened, those info will still be kept inside pgtables using (the upcoming) pte markers. When faulted, we'll just do small page mappings while it won't stop the shmem thp from being mapped hugely in other mm, afaict. With that in mind, indeed I see this patch less necessary to be merged; so for sparsely wr-protected vmas like virtio-mem we can still keep some of the ranges mergeable, that sounds a good thing to keep it as-is. NACK myself for now: let's not lose that good property of both thp+uffd-wp so easily, and I'll think more of it. (To Axel: my question to minor mode handling thp still stands, I think..) Thanks, -- Peter Xu
