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

[Frederic Konrad]

On 10/08/2015 18:15, Paolo Bonzini wrote:
> On 10/08/2015 17:27, address@hidden 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