Re: [Qemu-devel] [RFC PATCH 13/26] cpus: only take BQL for sleeping threads

[Paolo Bonzini]

On 13/11/2017 11:14, Alex Bennée wrote:
> 
> Pavel Dovgalyuk <address@hidden> writes:
> 
>>> From: Paolo Bonzini [mailto:address@hidden
>>>> From: "David Hildenbrand" <address@hidden>
>>>> On 02.11.2017 12:08, Paolo Bonzini wrote:
>>>>> On 31/10/2017 12:26, Pavel Dovgalyuk wrote:
>>>>>> From: Alex Bennée <address@hidden>
>>>>>>
>>>>>> Now the only real need to hold the BQL is for when we sleep on the
>>>>>> cpu->halt conditional. The lock is actually dropped while the thread
>>>>>> sleeps so the actual window for contention is pretty small. This also
>>>>>> means we can remove the special case hack for exclusive work and
>>>>>> simply declare that work no longer has an implicit BQL held. This
>>>>>> isn't a major problem async work is generally only changing things in
>>>>>> the context of its own vCPU. If it needs to work across vCPUs it
>>>>>> should be using the exclusive mechanism or possibly taking the lock
>>>>>> itself.
>>>>>>
>>>>>> Signed-off-by: Alex Bennée <address@hidden>
>>>>>> Tested-by: Pavel Dovgalyuk <address@hidden>
>>>>>
>>>>> At least cpu_throttle_thread would fail with this patch.
>>>>>
>>>>> Also I am not sure if the s390 SIGP handlers are ready for this.
>>>>>
>>>>
>>>> We have a global lock to the SIGP "facility". However we need the BQL in
>>>> order to inject interrupts into CPUs (otherwise it would trigger an
>>>> assert when injecting).
>>>>
>>>> We inject Restart and Stop interrupts from run_on_cpu. This requires the
>>>> BQL. So Paolo should be right, this change would break s390x.
>>>
>>> I had some patches to access interrupt_request with the atomic builtins.  If
>>> Pavel can first extract the other changes to the icount mechanism, I can
>>> update them.
>>
>> What changes do you mean here?
>> I'm not sure that I understand clearly how threads interact with BQL.
>> These patches were authored by Alex and we'll have to get him into the
>> discussion.
> 
> Do you want me to re-spin my sub-set of the patches as a new base?

I think the first part to be merged is changes to cpu-exec.c and
friends.  These might even go into 2.11.

Paolo