lkml.org 
[lkml]   [2012]   [Jul]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v2 3/7] KVM: Add paravirt kvm_flush_tlb_others
    On Tue, Jul 03, 2012 at 01:49:49PM +0530, Nikunj A Dadhania wrote:
    > On Tue, 3 Jul 2012 04:55:35 -0300, Marcelo Tosatti <mtosatti@redhat.com> wrote:
    > > >
    > > > if (!zero_mask)
    > > > goto again;
    > >
    > > Can you please measure increased vmentry/vmexit overhead? x86/vmexit.c
    > > of git://git.kernel.org/pub/scm/virt/kvm/kvm-unit-tests.git should
    > > help.
    > >
    > Sure will get back with the result.
    >
    > > > + /*
    > > > + * Guest might have seen us offline and would have set
    > > > + * flush_on_enter.
    > > > + */
    > > > + kvm_read_guest_cached(vcpu->kvm, ghc, vs, 2*sizeof(__u32));
    > > > + if (vs->flush_on_enter)
    > > > + kvm_x86_ops->tlb_flush(vcpu);
    > >
    > >
    > > So flush_tlb_page which was an invlpg now flushes the entire TLB. Did
    > > you take that into account?
    > >
    > When the vcpu is sleeping/pre-empted out, multiple request for flush_tlb
    > could have happened. And now when we are here, it is cleaning up all the
    > TLB.

    Yes, cases where there are sufficient exits transforming one TLB entry
    invalidation into full TLB invalidation should go unnoticed.

    > One other approach would be to queue the addresses, that brings us with
    > the question: how many request to queue? This would require us adding
    > more syncronization between guest and host for updating the area where
    > these addresses is shared.

    Sounds unnecessarily complicated.

    > > > +again:
    > > > + for_each_cpu(cpu, to_cpumask(f->flush_cpumask)) {
    > > > + v_state = &per_cpu(vcpu_state, cpu);
    > > > +
    > > > + if (!v_state->state) {
    > >
    > > Should use ACCESS_ONCE to make sure the value is not register cached.
    > > \
    > > > + v_state->flush_on_enter = 1;
    > > > + smp_mb();
    > > > + if (!v_state->state)
    > >
    > > And here.
    > >
    > Sure will add this check for both in my next version.
    >
    > > > + cpumask_clear_cpu(cpu, to_cpumask(f->flush_cpumask));
    > > > + }
    > > > + }
    > > > +
    > > > + if (cpumask_empty(to_cpumask(f->flush_cpumask)))
    > > > + goto out;
    > > > +
    > > > + apic->send_IPI_mask(to_cpumask(f->flush_cpumask),
    > > > + INVALIDATE_TLB_VECTOR_START + sender);
    > > > +
    > > > + loop = 1000;
    > > > + while (!cpumask_empty(to_cpumask(f->flush_cpumask)) && --loop)
    > > > + cpu_relax();
    > > > +
    > > > + if (!cpumask_empty(to_cpumask(f->flush_cpumask)))
    > > > + goto again;
    > >
    > > Is this necessary in addition to the in-guest-mode/out-guest-mode
    > > detection? If so, why?
    > >
    > The "case 3" where we initially saw the vcpu was running, and a flush
    > ipi is send to the vcpu. During this time the vcpu might be pre-empted,
    > so we come out of the loop=1000 with !empty flushmask. We then re-verify
    > the flushmask against the current running vcpu and make sure that the
    > vcpu that was pre-empted is un-marked and we can proceed out of the
    > kvm_flush_tlb_others_ipi without waiting for sleeping/pre-empted vcpus.
    >
    > Regards
    > Nikunj


    \
     
     \ /
      Last update: 2012-07-05 07:01    [W:0.037 / U:215.488 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site