Re: [Qemu-devel] [PATCH 03/10] replace spinlock by QemuMutex.

[Frederic Konrad]

On 13/08/2015 15:12, Paolo Bonzini wrote:
> On 13/08/2015 14:17, Frederic Konrad wrote:
> > > diff --git a/linux-user/main.c b/linux-user/main.c
> > > index fdee981..fd06ce9 100644
> > > --- a/linux-user/main.c
> > > +++ b/linux-user/main.c
> > > @@ -107,7 +107,7 @@ static int pending_cpus;
> > >    /* Make sure everything is in a consistent state for calling
> > > fork().  */
> > >    void fork_start(void)
> > >    {
> > > -    pthread_mutex_lock(&tcg_ctx.tb_ctx.tb_lock);
> > > +    qemu_mutex_lock(&tcg_ctx.tb_ctx.tb_lock);
> > >        pthread_mutex_lock(&exclusive_lock);
> > >        mmap_fork_start();
> > >    }
> > > @@ -129,11 +129,11 @@ void fork_end(int child)
> > >            pthread_mutex_init(&cpu_list_mutex, NULL);
> > >            pthread_cond_init(&exclusive_cond, NULL);
> > >            pthread_cond_init(&exclusive_resume, NULL);
> > > -        pthread_mutex_init(&tcg_ctx.tb_ctx.tb_lock, NULL);
> > > +        qemu_mutex_init(&tcg_ctx.tb_ctx.tb_lock);
> > >            gdbserver_fork(thread_cpu);
> > >        } else {
> > >            pthread_mutex_unlock(&exclusive_lock);
> > > -        pthread_mutex_unlock(&tcg_ctx.tb_ctx.tb_lock);
> > > +        qemu_mutex_unlock(&tcg_ctx.tb_ctx.tb_lock);
> > We might want to use tb_lock/unlock in user code as well instead of
> > calling directly qemu_mutex_* ?
> You cannot do that because of the recursive locking assertions; the
> child is not using qemu_mutex_unlock, it's using qemu_mutex_init.  So I
> would have to add some kind of tb_lock_reset_after_fork() function which
> is a bit ugly.

True.

Fred
> > > @@ -676,6 +709,7 @@ static inline void code_gen_alloc(size_t tb_size)
> > >                CODE_GEN_AVG_BLOCK_SIZE;
> > >        tcg_ctx.tb_ctx.tbs =
> > >                g_malloc(tcg_ctx.code_gen_max_blocks *
> > > sizeof(TranslationBlock));
> > > +    qemu_mutex_init(&tcg_ctx.tb_ctx.tb_lock);
> > Maybe we can initialize the mutex only for CONFIG_USER_ONLY?
> It's okay, it doesn't consume system resources.
>
> Paolo