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

[Frederic Konrad]

On 07/07/2015 14:33, Alex Bennée wrote:
> 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).

Yes exactly.
> > 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)