lkml.org 
[lkml]   [2014]   [Dec]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    SubjectRE: [v3 24/26] KVM: Update Posted-Interrupts Descriptor when vCPU is blocked
    Date


    > -----Original Message-----
    > From: linux-kernel-owner@vger.kernel.org
    > [mailto:linux-kernel-owner@vger.kernel.org] On Behalf Of Paolo Bonzini
    > Sent: Thursday, December 18, 2014 4:37 PM
    > To: linux-kernel@vger.kernel.org
    > Cc: iommu@lists.linux-foundation.org; kvm@vger.kernel.org;
    > linux-kernel@vger.kernel.org; kvm@vger.kernel.org
    > Subject: Re: [v3 24/26] KVM: Update Posted-Interrupts Descriptor when vCPU
    > is blocked
    >
    >
    >
    > On 18/12/2014 04:16, Wu, Feng wrote:
    > >>> pre-block:
    > >>> - Add the vCPU to the blocked per-CPU list
    > >>> - Clear 'SN'
    > >>
    > >> Should SN be already clear (and NV set to POSTED_INTR_VECTOR)?
    > >
    > > I think the SN bit should be clear here, Adding it here is just to make sure
    > > SN is clear when vCPU is blocked, so it can receive wakeup notification event
    > later.
    >
    > Then, please, WARN if the SN bit is set inside the if (vcpu->blocked).
    > Inside that if you can just add the vCPU to the blocked list on vcpu_put.
    >
    > >> Can it
    > >> happen that you go from sched-out to blocked without doing a sched-in
    > first?
    > >>
    > >
    > > I cannot imagine this scenario, can you please be more specific? Thanks a lot!
    >
    > I cannot either. :) But it would be the case where SN is not cleared.
    > So we agree that it cannot happen.
    >
    > >> In fact, if this is possible, what happens if vcpu->preempted &&
    > >> vcpu->blocked?
    > >
    > > In fact, vcpu->preempted && vcpu->blocked happens sometimes, but I think
    > there is
    > > no issues. Please refer to the following case:
    >
    > I agree that there should be no issues. But if it can happen, it's better:
    >
    > 1) to separate the handling of preemption and blocking: preemption
    > handles SN/NV/NDST, blocking handles the wakeup list.
    >
    Sorry, I don't quite understand this.

    I think handling of preemption and blocking is separated in vmx_vcpu_put().
    For vmx_vcpu_load(), the handling of SN/NV/NDST is common for preemption
    and blocking.

    Thanks,
    Feng

    > 2) to change this
    >
    > + } else if (vcpu->blocked) {
    > + /*
    > + * The vcpu is blocked on the wait queue.
    > + * Store the blocked vCPU on the list of the
    > + * vcpu->wakeup_cpu, which is the destination
    > + * of the wake-up notification event.
    >
    > to just
    >
    > }
    > if (vcpu->blocked) {
    > ...
    > }
    > > kvm_vcpu_block()
    > > -> vcpu->blocked = true;
    > > -> prepare_to_wait(&vcpu->wq, &wait, TASK_INTERRUPTIBLE);
    > >
    > > before schedule() is called, this vcpu is woken up by another guy, so
    > > the state of the vcpu associated thread is changed to TASK_RUNNING,
    > > then preemption happens after interrupts or the following schedule() is
    > > hit, this will call kvm_sched_out(), in which current->state ==
    > TASK_RUNNING
    > > and vcpu->preempted is set to true. So now vcpu->preempted and
    > vcpu->blocked
    > > are both true. In vmx_vcpu_put(), we will check vcpu->preempted first, so
    > > the vCPU will not be blocked, and the vcpu->blocked will be set the false in
    > > vmx_vcpu_load().
    > >
    > > But maybe I need do a little change to the vmx_vcpu_load() like below:
    > >
    > > /*
    > > * Delete the vCPU from the related wakeup queue
    > > * if we are resuming from blocked state
    > > */
    > > if (vcpu->blocked) {
    > > vcpu->blocked = false;
    > > + /* if wakeup_cpu == -1, the vcpu is currently not
    > blocked on any
    > > + pCPU, don't need dequeue here */
    > > + if (vcpu->wakeup_cpu != -1) {
    > >
    > spin_lock_irqsave(&per_cpu(blocked_vcpu_on_cpu_lock,
    > > vcpu->wakeup_cpu), flags);
    > > list_del(&vcpu->blocked_vcpu_list);
    > >
    > spin_unlock_irqrestore(&per_cpu(blocked_vcpu_on_cpu_lock,
    > > vcpu->wakeup_cpu), flags);
    > > vcpu->wakeup_cpu = -1;
    > > + }
    > > }
    >
    > Good idea.
    >
    > Paolo
    >
    > > Any ideas about this? Thanks a lot!
    > >
    > > Thanks,
    > > Feng
    > >
    > >
    > > -> schedule();
    > >
    > >
    > >>
    > >>> - Set 'NV' to POSTED_INTR_WAKEUP_VECTOR
    > >>>
    > >>> post-block:
    > >>> - Remove the vCPU from the per-CPU list
    > >>
    > >> Paolo
    > >>
    > >>> Signed-off-by: Feng Wu <feng.wu@intel.com>
    > >> --
    > >> To unsubscribe from this list: send the line "unsubscribe kvm" in
    > >> the body of a message to majordomo@vger.kernel.org
    > >> More majordomo info at http://vger.kernel.org/majordomo-info.html
    >
    > --
    > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    > the body of a message to majordomo@vger.kernel.org
    > More majordomo info at http://vger.kernel.org/majordomo-info.html
    > Please read the FAQ at http://www.tux.org/lkml/


    \
     
     \ /
      Last update: 2014-12-19 04:21    [W:2.988 / U:0.932 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site