On 05.04.24 11:37, Paolo Bonzini wrote:
On Fri, Apr 5, 2024 at 8:53 AM David Hildenbrand <david@xxxxxxxxxx> 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
