Re: [Qemu-devel] [RFC PATCH V6 07/18] Drop global lock during TCG code execution

[Alex Bennée]

Frederic Konrad <address@hidden> writes:

> On 26/06/2015 16:56, Jan Kiszka wrote:
>> On 2015-06-26 16:47, address@hidden wrote:
>>> From: Jan Kiszka <address@hidden>
>>>
>>> This finally allows TCG to benefit from the iothread introduction: Drop
>>> the global mutex while running pure TCG CPU code. Reacquire the lock
>>> when entering MMIO or PIO emulation, or when leaving the TCG loop.
<snip>
>>> diff --git a/translate-all.c b/translate-all.c
>>> index c25b79b..ade2269 100644
>>> --- a/translate-all.c
>>> +++ b/translate-all.c
>>> @@ -1222,6 +1222,7 @@ void tb_invalidate_phys_page_range(tb_page_addr_t 
>>> start, tb_page_addr_t end,
>>>   #endif
>>>   #ifdef TARGET_HAS_PRECISE_SMC
>>>       if (current_tb_modified) {
>>> +        qemu_mutex_unlock_iothread();
>>>           /* we generate a block containing just the instruction
>>>              modifying the memory. It will ensure that it cannot modify
>>>              itself */
>>> @@ -1326,6 +1327,7 @@ static void tb_invalidate_phys_page(tb_page_addr_t 
>>> addr,
>>>       p->first_tb = NULL;
>>>   #ifdef TARGET_HAS_PRECISE_SMC
>>>       if (current_tb_modified) {
>>> +        qemu_mutex_unlock_iothread();
>>>           /* we generate a block containing just the instruction
>>>              modifying the memory. It will ensure that it cannot modify
>>>              itself */
>>> diff --git a/vl.c b/vl.c
>>> index 69ad90c..2983d44 100644
>>> --- a/vl.c
>>> +++ b/vl.c
>>> @@ -1698,10 +1698,16 @@ void qemu_devices_reset(void)
>>>   {
>>>       QEMUResetEntry *re, *nre;
>>>   
>>> +    /*
>>> +     * Some device's reset needs to grab the global_mutex. So just release 
>>> it
>>> +     * here.
>> That's a property newly introduced by the patch, or how does this
>> happen? In turn, are all reset handlers now fine to be called outside of
>> BQL? This looks suspicious, but it's been quite a while since I last
>> starred at this.
>>
>> Jan
> Hi Jan,
>
> Sorry for that, it's a dirty hack :).
> Some reset handler probably load stuff in the memory hence a double lock.
> It will probably disappear with:
>
> http://thread.gmane.org/gmane.comp.emulators.qemu/345258

So I guess this patch will shrink a lot once we re-base ontop of Paolo's
patches (which should be real soon now).

>
> Thanks,
> Fred
>
>>> +     */
>>> +    qemu_mutex_unlock_iothread();
>>>       /* reset all devices */
>>>       QTAILQ_FOREACH_SAFE(re, &reset_handlers, entry, nre) {
>>>           re->func(re->opaque);
>>>       }
>>> +    qemu_mutex_lock_iothread();
>>>   }
>>>   
>>>   void qemu_system_reset(bool report)
>>>

-- 
Alex Bennée