Linux Virtualization - Feb 2014 - [PATCH RFC v5 7/8] pvqspinlock, x86: Add qspinlock para-virtualization support

On 27/02/14 13:11, Paolo Bonzini wrote:
> Il 27/02/2014 13:11, David Vrabel ha scritto:
>>> > This patch adds para-virtualization support to the queue
spinlock code
>>> > by enabling the queue head to kick the lock holder CPU, if
known,
>>> > in when the lock isn't released for a certain amount of
time. It
>>> > also enables the mutual monitoring of the queue head CPU and
the
>>> > following node CPU in the queue to make sure that their CPUs
will
>>> > stay scheduled in.
>> I'm not really understanding how this is supposed to work.  There
>> appears to be an assumption that a guest can keep one of its VCPUs
>> running by repeatedly kicking it?  This is not possible under Xen and I
>> doubt it's possible under KVM or any other hypervisor.
> 
> KVM allows any VCPU to wake up a currently halted VCPU of its choice,
> see Documentation/virtual/kvm/hypercalls.txt.
But neither of the VCPUs being kicked here are halted -- they're either
running or runnable (descheduled by the hypervisor).

David

Il 27/02/2014 15:18, David Vrabel ha scritto:
> On 27/02/14 13:11, Paolo Bonzini wrote:
>> Il 27/02/2014 13:11, David Vrabel ha scritto:
>>>>> This patch adds para-virtualization support to the queue
spinlock code
>>>>> by enabling the queue head to kick the lock holder CPU, if
known,
>>>>> in when the lock isn't released for a certain amount of
time. It
>>>>> also enables the mutual monitoring of the queue head CPU
and the
>>>>> following node CPU in the queue to make sure that their
CPUs will
>>>>> stay scheduled in.
>>> I'm not really understanding how this is supposed to work. 
There
>>> appears to be an assumption that a guest can keep one of its VCPUs
>>> running by repeatedly kicking it?  This is not possible under Xen
and I
>>> doubt it's possible under KVM or any other hypervisor.
>>
>> KVM allows any VCPU to wake up a currently halted VCPU of its choice,
>> see Documentation/virtual/kvm/hypercalls.txt.
>
> But neither of the VCPUs being kicked here are halted -- they're either
> running or runnable (descheduled by the hypervisor).
/me actually looks at Waiman's code...

Right, this is really different from pvticketlocks, where the *unlock* 
primitive wakes up a sleeping VCPU.  It is more similar to PLE 
(pause-loop exiting).

Paolo

On 02/27/2014 08:15 PM, Paolo Bonzini wrote:
[...]
>> But neither of the VCPUs being kicked here are halted -- they're
either
>> running or runnable (descheduled by the hypervisor).
>
> /me actually looks at Waiman's code...
>
> Right, this is really different from pvticketlocks, where the *unlock*
> primitive wakes up a sleeping VCPU.  It is more similar to PLE
> (pause-loop exiting).
Adding to the discussion, I see there are two possibilities here,
considering that in undercommit cases we should not exceed
HEAD_SPIN_THRESHOLD,

1. the looping vcpu in pv_head_spin_check() should do halt()
considering that we have done enough spinning (more than typical
lock-hold time), and hence we are in potential overcommit.

2. multiplex kick_cpu to do directed yield in qspinlock case.
But this may result in some ping ponging?

On 02/27/2014 09:45 AM, Paolo Bonzini wrote:
> Il 27/02/2014 15:18, David Vrabel ha scritto:
>> On 27/02/14 13:11, Paolo Bonzini wrote:
>>> Il 27/02/2014 13:11, David Vrabel ha scritto:
>>>>>> This patch adds para-virtualization support to the
queue spinlock
>>>>>> code
>>>>>> by enabling the queue head to kick the lock holder CPU,
if known,
>>>>>> in when the lock isn't released for a certain
amount of time. It
>>>>>> also enables the mutual monitoring of the queue head
CPU and the
>>>>>> following node CPU in the queue to make sure that their
CPUs will
>>>>>> stay scheduled in.
>>>> I'm not really understanding how this is supposed to work. 
There
>>>> appears to be an assumption that a guest can keep one of its
VCPUs
>>>> running by repeatedly kicking it?  This is not possible under
Xen
>>>> and I
>>>> doubt it's possible under KVM or any other hypervisor.
>>>
>>> KVM allows any VCPU to wake up a currently halted VCPU of its
choice,
>>> see Documentation/virtual/kvm/hypercalls.txt.
>>
>> But neither of the VCPUs being kicked here are halted -- they're
either
>> running or runnable (descheduled by the hypervisor).
>
> /me actually looks at Waiman's code...
>
> Right, this is really different from pvticketlocks, where the *unlock* 
> primitive wakes up a sleeping VCPU.  It is more similar to PLE 
> (pause-loop exiting).
>
> Paolo
Yes, it is mostly to deal with vCPU that are not running because of PLE.

-Longman