Messages in this thread | | | Date | Tue, 26 Jan 2021 16:04:33 -0800 | From | Sean Christopherson <> | Subject | Re: [PATCH] KVM: x86/mmu: consider the hva in mmu_notifer retry |
| |
On Tue, Jan 26, 2021, David Stevens wrote: > > This needs a comment to explicitly state that 'count > 1' cannot be done at > > this time. My initial thought is that it would be more intuitive to check for > > 'count > 1' here, but that would potentially check the wrong wrange when count > > goes from 2->1. The comment about persistence in invalidate_range_start() is a > > good hint, but I think it's worth being explicit to avoid bad "cleanup" in the > > future. > > > > > + if (unlikely(kvm->mmu_notifier_count)) { > > > + if (kvm->mmu_notifier_range_start <= hva && > > > + hva < kvm->mmu_notifier_range_end) > > I'm not sure I understand what you're suggesting here. How exactly > would 'count > 1' be used incorrectly here? I'm fine with adding a > comment, but I'm not sure what the comment needs to clarify.
There's no guarantee that the remaining in-progress invalidation when the count goes from 2->1 is the same invalidation call that set range_start/range_end.
E.g. given two invalidations, A and B, the order of calls could be:
kvm_mmu_notifier_invalidate_range_start(A) kvm_mmu_notifier_invalidate_range_start(B) kvm_mmu_notifier_invalidate_range_end(A) kvm_mmu_notifier_invalidate_range_end(B) <-- ???
or
kvm_mmu_notifier_invalidate_range_start(A) kvm_mmu_notifier_invalidate_range_start(B) kvm_mmu_notifier_invalidate_range_end(B) kvm_mmu_notifier_invalidate_range_end(A) <-- ???
In the first case, "A" is in-progress when the count goes 2->1, in the second case "B" is still in-progress. Checking for "count > 1" in the consumer instead of handling it in the producer (as you did) would lead to the consumer checking against the wrong range. I don't see a way to solve that without adding some amount of history, which I agree is unnecessary.
| |