Re: [patch] x86: kvm: Convert i8254/i8259 locks to raw_spinlocks

Avi Kivity
2010-02-18T10:18:39Z

On 02/18/2010 12:05 PM, Jan Kiszka wrote:
> Avi Kivity wrote:
>
>> On 02/18/2010 11:45 AM, Avi Kivity wrote:
>>
>>> On 02/18/2010 11:40 AM, Jan Kiszka wrote:
>>>
>>>>> Meanwhile, if anyone has any idea how to kill this lock, I'd love to
>>>>> see it.
>>>>>
>>>>>
>>>> What concurrency does it resolve in the end? On first glance, it only
>>>> synchronize the fiddling with pre-VCPU request bits, right? What forces
>>>> us to do this? Wouldn't it suffice to disable preemption (thus
>>>> migration) and the let concurrent requests race for setting the bits? I
>>>> mean if some request bit was already set on entry, we don't include the
>>>> related VCPU in smp_call_function_many anyway.
>>>>
>>> It's more difficult.
>>>
>>> vcpu 0: sets request bit on vcpu 2
>>> vcpu 1: test_and_set request bit on vcpu 2, returns already set
>>> vcpu 1: returns
>>> vcpu 0: sends IPI
>>> vcpu 0: returns
>>>
>>> so vcpu 1 returns before the IPI was performed. If the request was a
>>> tlb flush, for example, vcpu 1 may free a page that is still in vcpu
>>> 2's tlb.
>>>
>> One way out would be to have a KVM_REQ_IN_PROGRESS, set it in
>> make_request, clear it in the IPI function.
>>
>> If a second make_request sees it already set, it can simply busy wait
>> until it is cleared, without sending the IPI. Of course the busy wait
>> means we can't enable preemption (or we may busy wait on an unscheduled
>> task), but at least the requests can proceed in parallel instead of
>> serializing.
>>
> ...or include VCPUs with KVM_REQ_IN_PROGRESS set into the IPI set even
> if they already have the desired request bit set.

But then we're making them take the IPI, which is pointless and
expensive. My approach piggy backs multiple requesters on one IPI.

> Then we should
> serialize in smp_call_function_many.
>

Do you mean rely on s_c_f_m's internal synchronization?

--
error compiling committee.c: too many arguments to function