From patchwork Mon Jun 21 20:25:30 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jan Kiszka X-Patchwork-Id: 56344 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [199.232.76.165]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id A8B48B6F15 for ; Tue, 22 Jun 2010 06:28:01 +1000 (EST) Received: from localhost ([127.0.0.1]:37905 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1OQnb3-0004wH-Ea for incoming@patchwork.ozlabs.org; Mon, 21 Jun 2010 16:27:57 -0400 Received: from [140.186.70.92] (port=42476 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1OQnYs-0004KI-5k for qemu-devel@nongnu.org; Mon, 21 Jun 2010 16:25:43 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69) (envelope-from ) id 1OQnYo-0000X9-MC for qemu-devel@nongnu.org; Mon, 21 Jun 2010 16:25:40 -0400 Received: from fmmailgate02.web.de ([217.72.192.227]:52614) by eggs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1OQnYo-0000WY-4I for qemu-devel@nongnu.org; Mon, 21 Jun 2010 16:25:38 -0400 Received: from smtp06.web.de ( [172.20.5.172]) by fmmailgate02.web.de (Postfix) with ESMTP id F1B2C16870E98; Mon, 21 Jun 2010 22:25:35 +0200 (CEST) Received: from [92.75.137.1] (helo=[192.168.1.10]) by smtp06.web.de with asmtp (TLSv1:AES256-SHA:256) (WEB.DE 4.110 #4) id 1OQnYl-000232-00; Mon, 21 Jun 2010 22:25:35 +0200 Message-ID: <4C1FCABA.3060207@web.de> Date: Mon, 21 Jun 2010 22:25:30 +0200 From: Jan Kiszka User-Agent: Mozilla/5.0 (X11; U; Linux i686 (x86_64); de; rv:1.8.1.12) Gecko/20080226 SUSE/2.0.0.12-1.1 Thunderbird/2.0.0.12 Mnenhy/0.7.5.666 MIME-Version: 1.0 To: Marcelo Tosatti References: <4C1BA0B4.2010803@siemens.com> <4C1BCEB0.6050601@codemonkey.ws> <20100621193123.GA14083@amt.cnet> In-Reply-To: <20100621193123.GA14083@amt.cnet> X-Enigmail-Version: 0.95.7 X-Sender: jan.kiszka@web.de X-Provags-ID: V01U2FsdGVkX1/PfR3FfkAtNbHnQdsVldl0s7IHiRf0XRZOS4lM cqFFCeT+WN8Fj0/KbGAFie8Ft9KKmjomAjkXt/P3kM6N/c9PJt AuK/STjDw= X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.4-2.6 Cc: Glauber Costa , qemu-devel Subject: [Qemu-devel] Re: [PATCH] fix smp with tcg mode and --enable-io-thread X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Marcelo Tosatti wrote: > Clear exit_request when iothread grabs the global lock. > > Signed-off-by: Marcelo Tosatti > > diff --git a/cpu-exec.c b/cpu-exec.c > index 026980a..74cb973 100644 > --- a/cpu-exec.c > +++ b/cpu-exec.c > @@ -236,10 +236,8 @@ int cpu_exec(CPUState *env1) > asm(""); > env = env1; > > - if (exit_request) { > + if (exit_request) > env->exit_request = 1; > - exit_request = 0; > - } Coding style... > > #if defined(TARGET_I386) > if (!kvm_enabled()) { > diff --git a/cpus.c b/cpus.c > index fcd0f09..ef1ab22 100644 > --- a/cpus.c > +++ b/cpus.c > @@ -598,6 +598,7 @@ void qemu_mutex_lock_iothread(void) > } > qemu_mutex_unlock(&qemu_fair_mutex); > } > + exit_request = 0; > } > > void qemu_mutex_unlock_iothread(void) > > I looked into this a bit as well, and that's what I also have in my queue. But things are still widely broken: pause_all_vcpus and run_on_cpu as there is no guarantee that all VCPUs regularly call into qemu_wait_io_event. Also breakpoints don't work, not only in SMP mode. The former issue is mostly fine with this patch: I haven't looked into the breakpoint bug yet as performance (likely VCPU scheduling) in iothread mode is still suffering compared to classic mode, specifically when you go up with the number of VCPUs (try -smp 8). And there is some race that cause a lock up in qemu_mutex_lock_iothread after a while (the cpu_unlink_tb seems to race with the linking - just a guess so far). Anyway, all this is getting terribly complex, hard to grasp, and maybe therefore fragile as well. Specifically the SMP round-robin scheduling looks like it requires a redesign for iothread mode (my feeling is it only works by chance ATM). Part of the issue may not be new, but could have been shadowed by the frequently interrupting I/O load when using a single thread. The rest seems to lack a bit a user base... Jan diff --git a/cpu-exec.c b/cpu-exec.c index 026980a..9f4191d 100644 --- a/cpu-exec.c +++ b/cpu-exec.c @@ -236,9 +236,8 @@ int cpu_exec(CPUState *env1) asm(""); env = env1; - if (exit_request) { + if (unlikely(exit_request)) { env->exit_request = 1; - exit_request = 0; } #if defined(TARGET_I386) diff --git a/cpus.c b/cpus.c index fcd0f09..2ce839d 100644 --- a/cpus.c +++ b/cpus.c @@ -39,7 +39,6 @@ #define SIG_IPI SIGUSR1 #endif -static CPUState *cur_cpu; static CPUState *next_cpu; /***********************************************************/ @@ -331,6 +330,7 @@ int qemu_init_main_loop(void) return ret; qemu_cond_init(&qemu_pause_cond); + qemu_cond_init(&qemu_system_cond); qemu_mutex_init(&qemu_fair_mutex); qemu_mutex_init(&qemu_global_mutex); qemu_mutex_lock(&qemu_global_mutex); @@ -401,10 +401,12 @@ static void qemu_wait_io_event_common(CPUState *env) flush_queued_work(env); } -static void qemu_wait_io_event(CPUState *env) +static void qemu_tcg_wait_io_event(void) { + CPUState *env; + while (!tcg_has_work()) - qemu_cond_timedwait(env->halt_cond, &qemu_global_mutex, 1000); + qemu_cond_timedwait(tcg_halt_cond, &qemu_global_mutex, 1000); qemu_mutex_unlock(&qemu_global_mutex); @@ -417,7 +419,10 @@ static void qemu_wait_io_event(CPUState *env) qemu_mutex_unlock(&qemu_fair_mutex); qemu_mutex_lock(&qemu_global_mutex); - qemu_wait_io_event_common(env); + + for (env = first_cpu; env != NULL; env = env->next_cpu) { + qemu_wait_io_event_common(env); + } } static void qemu_kvm_eat_signal(CPUState *env, int timeout) @@ -502,7 +507,7 @@ static void *tcg_cpu_thread_fn(void *arg) while (1) { tcg_cpu_exec(); - qemu_wait_io_event(cur_cpu); + qemu_tcg_wait_io_event(); } return NULL; @@ -768,11 +773,11 @@ bool tcg_cpu_exec(void) if (next_cpu == NULL) next_cpu = first_cpu; - for (; next_cpu != NULL; next_cpu = next_cpu->next_cpu) { - CPUState *env = cur_cpu = next_cpu; + for (; next_cpu != NULL && !exit_request; next_cpu = next_cpu->next_cpu) { + CPUState *env = next_cpu; qemu_clock_enable(vm_clock, - (cur_cpu->singlestep_enabled & SSTEP_NOTIMER) == 0); + (env->singlestep_enabled & SSTEP_NOTIMER) == 0); if (qemu_alarm_pending()) break; @@ -787,6 +792,7 @@ bool tcg_cpu_exec(void) break; } } + exit_request = 0; return tcg_has_work(); }