Messages in this thread | | | Date | Fri, 5 Apr 2024 12:14:00 +0200 | Subject | Re: [RFC PATCH 0/4] KVM: x86/mmu: Rework marking folios dirty/accessed | From | David Hildenbrand <> |
| |
On 05.04.24 11:37, Paolo Bonzini wrote: > On Fri, Apr 5, 2024 at 8:53 AM David Hildenbrand <david@redhat.com> wrote: >>> mmu_notifier_invalidate_range_start(&range); >>> tlb_start_vma(&tlb, vma); >>> walk_page_range(vma->vm_mm, range.start, range.end, >>> &madvise_free_walk_ops, &tlb); >>> tlb_end_vma(&tlb, vma); >>> mmu_notifier_invalidate_range_end(&range); >>> >> >> Indeed, we do setup the MMU notifier invalidation. We do the start/end >> ... I was looking for PTE notifications. >> >> I spotted the set_pte_at(), not a set_pte_at_notify() like we do in >> other code. Maybe that's not required here (digging through >> documentation I'm still left clueless). [...] >> Absolutely unclear to me when we *must* to use it, or if it is. Likely >> its a pure optimization when we're *changing* a PTE. > > Yes, .change_pte() is just an optimization. The original point of it > was for KSM, so that KVM could flip the sPTE to a new location without > first zapping it. At the time there was also an .invalidate_page() > callback, and both of them were *not* bracketed by calls to > mmu_notifier_invalidate_range_{start,end}() > > Later on, both callbacks were changed to occur within an > invalidate_range_start/end() block. > > Commit 6bdb913f0a70 ("mm: wrap calls to set_pte_at_notify with > invalidate_range_start and invalidate_range_end", 2012-10-09) did so > for .change_pte(). The reason to do so was a bit roundabout, namely to > allow sleepable .invalidate_page() hooks (because .change_pte() is not > sleepable and at the time .invalidate_page() was used as a fallback > for .change_pte()). > > This however made KVM's usage of the .change_pte() callback completely > moot, because KVM unmaps the sPTEs during .invalidate_range_start() > and therefore .change_pte() has no hope of succeeding. > > (Commit 369ea8242c0f ("mm/rmap: update to new mmu_notifier semantic > v2", 2017-08-31) is where the other set of non-bracketed calls to MMU > notifier callbacks was changed; calls to > mmu_notifier_invalidate_page() were replaced by calls to > mmu_notifier_invalidate_range(), bracketed by calls to > mmu_notifier_invalidate_range_{start,end}()). > > Since KVM is the only user of .change_pte(), we can remove > .change_pte() and set_pte_at_notify() completely.
Nice, thanks for all that information!
-- Cheers,
David / dhildenb
| |