Messages in this thread | | | Date | Mon, 17 Jun 2013 19:59:15 +0800 | From | Xiao Guangrong <> | Subject | Re: [PATCH v3 0/6] KVM: MMU: fast invalidate all mmio sptes |
| |
Sorry for the delay reply since i was on vacation.
On 06/15/2013 10:22 AM, Takuya Yoshikawa wrote: > On Thu, 13 Jun 2013 21:08:21 -0300 > Marcelo Tosatti <mtosatti@redhat.com> wrote: > >> On Fri, Jun 07, 2013 at 04:51:22PM +0800, Xiao Guangrong wrote: > >> - Where is the generation number increased? > > Looks like when a new slot is installed in update_memslots() because > it's based on slots->generation. This is not restricted to "create" > and "move".
Yes. It reuses slots->generation to avoid unnecessary synchronizations (RCU, memory barrier).
Increasing mmio generation number in the case of "create" and "move" is ok - it is no addition work unless mmio generation number is overflow which is hardly triggered (since the valid mmio generation number is large enough and zap_all is scale well now.) and the mmio spte is updated only when it is used in the future.
> >> - Should use spinlock breakable code in kvm_mmu_zap_mmio_sptes() >> (picture guest with 512GB of RAM, even walking all those pages is >> expensive) (ah, patch to remove kvm_mmu_zap_mmio_sptes does that). >> - Is -13 enough to test wraparound? Its highly likely the guest has >> not began executing by the time 13 kvm_set_memory_calls are made >> (so no sptes around). Perhaps -2000 is more sensible (should confirm >> though). > > In the future, after we've tested enough, we should change the testing > code to be executed only for some debugging configs. Especially, if we > change zap_mmio_sptes() to zap_all_shadows(), very common guests, even > without huge memory like 512GB, can see the effect induced by sudden page > faults unnecessarily. > > If necessary, developers can test the wraparound code by lowering the > max_gen itself anyway.
I agree.
> >> - Why remove "if (change == KVM_MR_CREATE) || (change >> == KVM_MR_MOVE)" from kvm_arch_commit_memory_region? >> Its instructive. > > There may be a chance that we miss generation wraparounds if we don't > check other cases: seems unlikely, but theoretically possible. > > In short, all memory slot changes make mmio sptes stored in shadow pages > obsolete, or zapped for wraparounds, in the new way -- am I right?
Yes. You are definitely right. :)
Takuya-san, thank you very much for you answering the questions for me and thanks all of you for patiently reviewing my patches.
Marcelo, your points?
| |