| Message ID | 20210730223707.4083785-4-dmatlack@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Improve gfn-to-memslot performance during page faults | expand |
On 31/07/21 00:37, David Matlack wrote: > - if (new_spte == iter->old_spte) > + if (new_spte == iter->old_spte) { > ret = RET_PF_SPURIOUS; > - else if (!tdp_mmu_set_spte_atomic(vcpu->kvm, iter, new_spte)) > - return RET_PF_RETRY; > + } else { > + if (!tdp_mmu_set_spte_atomic_no_dirty_log(vcpu->kvm, iter, new_spte)) > + return RET_PF_RETRY; > + > + /* > + * Mark the gfn dirty here rather that through the vcpu-agnostic > + * handle_changed_spte_dirty_log to leverage vcpu->lru_slot_index. > + */ > + if (is_writable_pte(new_spte)) > + kvm_vcpu_mark_page_dirty(vcpu, iter->gfn); > + } Looking at the remaining callers of tdp_mmu_set_spte_atomic we have: * tdp_mmu_zap_spte_atomic calls it with REMOVED_SPTE as the new_spte, which is never writable * kvm_tdp_mmu_map calls it for nonleaf SPTEs, which are always writable but should not be dirty. So I think you should: * change those two to tdp_mmu_set_spte_atomic_no_dirty_log * add a WARN_ON_ONCE(iter->level > PG_LEVEL_4K) to tdp_mmu_set_spte_atomic * put the kvm_vcpu_mark_page_dirty code directly in tdp_mmu_set_spte_atomic, instead of the call to handle_changed_spte_dirty_log (I can't exclude I'm missing something though). Paolo
On Mon, Aug 02, 2021 at 04:58:17PM +0200, Paolo Bonzini wrote: > On 31/07/21 00:37, David Matlack wrote: > > - if (new_spte == iter->old_spte) > > + if (new_spte == iter->old_spte) { > > ret = RET_PF_SPURIOUS; > > - else if (!tdp_mmu_set_spte_atomic(vcpu->kvm, iter, new_spte)) > > - return RET_PF_RETRY; > > + } else { > > + if (!tdp_mmu_set_spte_atomic_no_dirty_log(vcpu->kvm, iter, new_spte)) > > + return RET_PF_RETRY; > > + > > + /* > > + * Mark the gfn dirty here rather that through the vcpu-agnostic > > + * handle_changed_spte_dirty_log to leverage vcpu->lru_slot_index. > > + */ > > + if (is_writable_pte(new_spte)) > > + kvm_vcpu_mark_page_dirty(vcpu, iter->gfn); > > + } > > Looking at the remaining callers of tdp_mmu_set_spte_atomic we have: > > * tdp_mmu_zap_spte_atomic calls it with REMOVED_SPTE as the new_spte, which > is never writable > > * kvm_tdp_mmu_map calls it for nonleaf SPTEs, which are always writable but > should not be dirty. > > > So I think you should: > > * change those two to tdp_mmu_set_spte_atomic_no_dirty_log > > * add a WARN_ON_ONCE(iter->level > PG_LEVEL_4K) to tdp_mmu_set_spte_atomic > > * put the kvm_vcpu_mark_page_dirty code directly in tdp_mmu_set_spte_atomic, > instead of the call to handle_changed_spte_dirty_log > > (I can't exclude I'm missing something though). Makes sense. I'll take a look to confirm and make those changes in v2. > > Paolo >
diff --git a/arch/x86/kvm/mmu/tdp_mmu.c b/arch/x86/kvm/mmu/tdp_mmu.c index 43f12f5d12c0..1467f99c846d 100644 --- a/arch/x86/kvm/mmu/tdp_mmu.c +++ b/arch/x86/kvm/mmu/tdp_mmu.c @@ -929,10 +929,19 @@ static int tdp_mmu_map_handle_target_level(struct kvm_vcpu *vcpu, int write, map_writable, !shadow_accessed_mask, &new_spte); - if (new_spte == iter->old_spte) + if (new_spte == iter->old_spte) { ret = RET_PF_SPURIOUS; - else if (!tdp_mmu_set_spte_atomic(vcpu->kvm, iter, new_spte)) - return RET_PF_RETRY; + } else { + if (!tdp_mmu_set_spte_atomic_no_dirty_log(vcpu->kvm, iter, new_spte)) + return RET_PF_RETRY; + + /* + * Mark the gfn dirty here rather that through the vcpu-agnostic + * handle_changed_spte_dirty_log to leverage vcpu->lru_slot_index. + */ + if (is_writable_pte(new_spte)) + kvm_vcpu_mark_page_dirty(vcpu, iter->gfn); + } /* * If the page fault was caused by a write but the page is write
The existing TDP MMU methods to handle dirty logging are vcpu-agnostic since they can be driven by MMU notifiers and other non-vcpu-specific events in addition to page faults. However this means that the TDP MMU is not benefiting from the new vcpu->lru_slot_index. Fix that by special casing dirty logging in tdp_mmu_map_handle_target_level. This improves "Populate memory time" in dirty_log_perf_test by 5%: Command | Before | After ------------------------------- | ---------------- | ------------- ./dirty_log_perf_test -v64 -x64 | 5.472321072s | 5.169832886s Signed-off-by: David Matlack <dmatlack@google.com> --- arch/x86/kvm/mmu/tdp_mmu.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-)