Re: [PATCH v2 5/7] KVM: Introduce PV kick in flush tlb

[Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>]

On Wed, 4 Jul 2012 23:37:46 -0300, Marcelo Tosatti <mtosatti@redhat.com> wrote:
> On Tue, Jul 03, 2012 at 01:55:02PM +0530, Nikunj A Dadhania wrote:
> > On Tue, 3 Jul 2012 05:07:13 -0300, Marcelo Tosatti <mtosatti@redhat.com> wrote:
> > > On Mon, Jun 04, 2012 at 10:38:17AM +0530, Nikunj A. Dadhania wrote:
> > > > In place of looping continuously introduce a halt if we do not succeed
> > > > after some time.
> > > > 
> > > > For vcpus that were running an IPI is sent.  In case, it went to sleep
> > > > between this, we will be doing flush_on_enter(harmless). But as a
> > > > flush IPI was already sent, that will be processed in ipi handler,
> > > > this might result into something undesireable, i.e. It might clear the
> > > > flush_mask of a new request.
> > > > 
> > > > So after sending an IPI and waiting for a while, do a halt and wait
> > > > for a kick from the last vcpu.
> > > > 
> > > > Signed-off-by: Srivatsa Vaddagiri <vatsa@linux.vnet.ibm.com>
> > > > Signed-off-by: Nikunj A. Dadhania <nikunj@linux.vnet.ibm.com>
> > > 
> > > Again, was it determined that this is necessary from data of 
> > > benchmarking on the in-guest-mode/out-guest-mode patch?
> > > 
> > No, this is more of a fix wrt algo.
> 
> Please have numbers for the improvement relative to the previous
> patch.
> 
I would consider this more of a correctness fix, rather than an
improvement. In this scenario, suppose vcpu1 was pre-empted out before
the delivery of the IPI. After the loop count, we find that vcpu1 did
not respond and is found pre-empted. We set the flush_on_enter flag for
the vcpu1 and proceed. During vcpu1's guest_enter we would do a
flush_on_enter. Now the vcpu1 will also receive an ipi interrupt in guest
mode, in which it will try to clear the flush_mask and acknowledge to
the interrupt. This processing of ipi would not be correct. So with this
patch, we execute a halt and wait for vcpu1 to clear the flush_mask
through the ipi interrupt.

> It introduces a dependency, these (pvtlbflush and pvspinlocks) are
> separate features. It is useful to switch them on/off individually.
> 
Agree, we can also get the pv kick feature separated which can be useful
to both of the approaches, so they can become independent.

Although, tests suggests that for best results both these features
should be enabled.

Nikunj