From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:52340)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <fred.konrad@greensocs.com>) id 1ZPsRY-0002Be-V2
	for qemu-devel@nongnu.org; Thu, 13 Aug 2015 09:21:17 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <fred.konrad@greensocs.com>) id 1ZPsRU-0004Rr-V6
	for qemu-devel@nongnu.org; Thu, 13 Aug 2015 09:21:16 -0400
Received: from greensocs.com ([193.104.36.180]:42491)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <fred.konrad@greensocs.com>) id 1ZPsRU-0004RF-KQ
	for qemu-devel@nongnu.org; Thu, 13 Aug 2015 09:21:12 -0400
Message-ID: <55CC99C5.9070904@greensocs.com>
Date: Thu, 13 Aug 2015 15:21:09 +0200
From: Frederic Konrad <fred.konrad@greensocs.com>
MIME-Version: 1.0
References: <1439397664-70734-1-git-send-email-pbonzini@redhat.com>
	<1439397664-70734-4-git-send-email-pbonzini@redhat.com>
	<55CC8ADF.6040002@greensocs.com> <55CC97BF.9030408@redhat.com>
In-Reply-To: <55CC97BF.9030408@redhat.com>
Content-Type: text/plain; charset=windows-1252; format=flowed
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [PATCH 03/10] replace spinlock by QemuMutex.
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Paolo Bonzini <pbonzini@redhat.com>, qemu-devel@nongnu.org
Cc: mttcg@greensocs.com

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