Re: [Qemu-devel] [RFC PATCH V7 09/19] Drop global lock during TCG code execution

[Frederic Konrad <fred.konrad@greensocs.com>]

On 10/08/2015 18:15, Paolo Bonzini wrote:
>
> On 10/08/2015 17:27, fred.konrad@greensocs.com wrote:
>>   void qemu_mutex_lock_iothread(void)
>>   {
>> -    atomic_inc(&iothread_requesting_mutex);
>> -    /* In the simple case there is no need to bump the VCPU thread out of
>> -     * TCG code execution.
>> -     */
>> -    if (!tcg_enabled() || qemu_in_vcpu_thread() ||
>> -        !first_cpu || !first_cpu->thread) {
>> -        qemu_mutex_lock(&qemu_global_mutex);
>> -        atomic_dec(&iothread_requesting_mutex);
>> -    } else {
>> -        if (qemu_mutex_trylock(&qemu_global_mutex)) {
>> -            qemu_cpu_kick_thread(first_cpu);
>> -            qemu_mutex_lock(&qemu_global_mutex);
>> -        }
>> -        atomic_dec(&iothread_requesting_mutex);
>> -        qemu_cond_broadcast(&qemu_io_proceeded_cond);
>> -    }
>> -    iothread_locked = true;
> "iothread_locked = true" must be kept.  Otherwise... yay! :)

oops :).
>
>> @@ -125,8 +128,10 @@ void tlb_flush_page(CPUState *cpu, target_ulong addr)
>>      can be detected */
>>   void tlb_protect_code(ram_addr_t ram_addr)
>>   {
>> +    qemu_mutex_lock_iothread();
>>       cpu_physical_memory_test_and_clear_dirty(ram_addr, TARGET_PAGE_SIZE,
>>                                                DIRTY_MEMORY_CODE);
>> +    qemu_mutex_unlock_iothread();
>>   }
>>   
> Not needed anymore.
>
>> diff --git a/target-i386/misc_helper.c b/target-i386/misc_helper.c
>> index 52c5d65..55f63bf 100644
>> --- a/target-i386/misc_helper.c
>> +++ b/target-i386/misc_helper.c
> None of this is needed anymore either! :)
>
>> +    /*
>> +     * Some device's reset needs to grab the global_mutex. So just release it
>> +     * here.
>> +     */
>> +    qemu_mutex_unlock_iothread();
>>       /* reset all devices */
>>       QTAILQ_FOREACH_SAFE(re, &reset_handlers, entry, nre) {
>>           re->func(re->opaque);
>>       }
>> +    qemu_mutex_lock_iothread();
> Should never have been true?  (And, I think, it was pointed out in a
> previous version too).
I had a double lock with the reset handler from vexpress-a15. I don't really
remember why. But I hacked that. It's fixed now :)

Thanks,
Fred

> Paolo
>