From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:51606) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z8Vkz-0007Fg-9k for qemu-devel@nongnu.org; Fri, 26 Jun 2015 11:41:34 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Z8Vkv-0006KY-6E for qemu-devel@nongnu.org; Fri, 26 Jun 2015 11:41:33 -0400 Received: from greensocs.com ([193.104.36.180]:54079) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z8Vku-0006KE-Si for qemu-devel@nongnu.org; Fri, 26 Jun 2015 11:41:29 -0400 Message-ID: <558D72A3.4060907@greensocs.com> Date: Fri, 26 Jun 2015 17:41:23 +0200 From: Frederic Konrad MIME-Version: 1.0 References: <1435330053-18733-1-git-send-email-fred.konrad@greensocs.com> <1435330053-18733-7-git-send-email-fred.konrad@greensocs.com> <558D699A.2010804@redhat.com> In-Reply-To: <558D699A.2010804@redhat.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [RFC PATCH V6 06/18] tcg: remove tcg_halt_cond global variable. List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini , qemu-devel@nongnu.org, mttcg@greensocs.com Cc: peter.maydell@linaro.org, a.spyridakis@virtualopensystems.com, mark.burton@greensocs.com, agraf@suse.de, alistair.francis@xilinx.com, guillaume.delbergue@greensocs.com, alex.bennee@linaro.org On 26/06/2015 17:02, Paolo Bonzini wrote: > > On 26/06/2015 16:47, fred.konrad@greensocs.com wrote: >> From: KONRAD Frederic >> >> This removes tcg_halt_cond global variable. >> We need one QemuCond per virtual cpu for multithread TCG. >> >> Signed-off-by: KONRAD Frederic >> --- >> cpus.c | 18 +++++++----------- >> 1 file changed, 7 insertions(+), 11 deletions(-) >> >> diff --git a/cpus.c b/cpus.c >> index 2d62a35..79383df 100644 >> --- a/cpus.c >> +++ b/cpus.c >> @@ -813,7 +813,6 @@ static unsigned iothread_requesting_mutex; >> static QemuThread io_thread; >> >> static QemuThread *tcg_cpu_thread; >> -static QemuCond *tcg_halt_cond; >> >> /* cpu creation */ >> static QemuCond qemu_cpu_cond; >> @@ -919,15 +918,13 @@ static void qemu_wait_io_event_common(CPUState *cpu) >> cpu->thread_kicked = false; >> } >> >> -static void qemu_tcg_wait_io_event(void) >> +static void qemu_tcg_wait_io_event(CPUState *cpu) >> { >> - CPUState *cpu; >> - >> while (all_cpu_threads_idle()) { >> /* Start accounting real time to the virtual clock if the CPUs >> are idle. */ >> qemu_clock_warp(QEMU_CLOCK_VIRTUAL); >> - qemu_cond_wait(tcg_halt_cond, &qemu_global_mutex); >> + qemu_cond_wait(cpu->halt_cond, &qemu_global_mutex); >> } >> >> while (iothread_requesting_mutex) { >> @@ -1047,7 +1044,7 @@ static void *qemu_tcg_cpu_thread_fn(void *arg) >> >> /* wait for initial kick-off after machine start */ >> while (first_cpu->stopped) { >> - qemu_cond_wait(tcg_halt_cond, &qemu_global_mutex); >> + qemu_cond_wait(first_cpu->halt_cond, &qemu_global_mutex); >> >> /* process any pending work */ >> CPU_FOREACH(cpu) { >> @@ -1068,7 +1065,7 @@ static void *qemu_tcg_cpu_thread_fn(void *arg) >> qemu_clock_notify(QEMU_CLOCK_VIRTUAL); >> } >> } >> - qemu_tcg_wait_io_event(); >> + qemu_tcg_wait_io_event(QTAILQ_FIRST(&cpus)); > Does this work (for non-multithreaded TCG) if tcg_thread_fn is waiting > on the "wrong" condition variable? For example if all CPUs are idle and > the second CPU wakes up, qemu_tcg_wait_io_event won't be kicked out of > the wait. > > I think you need to have a CPUThread struct like this: > > struct CPUThread { > QemuThread thread; > QemuCond halt_cond; > }; > > and in CPUState have a CPUThread * field instead of the thread and > halt_cond fields. > > Then single-threaded TCG can point all CPUStates to the same instance of > the struct, while multi-threaded TCG can point each CPUState to a > different struct. > > Paolo Hmm probably not, though we didn't pay attention to keep the non MTTCG working. (which is probably not good). > >> } >> >> return NULL; >> @@ -1235,12 +1232,12 @@ static void qemu_tcg_init_vcpu(CPUState *cpu) >> >> tcg_cpu_address_space_init(cpu, cpu->as); >> >> + cpu->halt_cond = g_malloc0(sizeof(QemuCond)); >> + qemu_cond_init(cpu->halt_cond); >> + >> /* share a single thread for all cpus with TCG */ >> if (!tcg_cpu_thread) { >> cpu->thread = g_malloc0(sizeof(QemuThread)); >> - cpu->halt_cond = g_malloc0(sizeof(QemuCond)); >> - qemu_cond_init(cpu->halt_cond); >> - tcg_halt_cond = cpu->halt_cond; >> snprintf(thread_name, VCPU_THREAD_NAME_SIZE, "CPU %d/TCG", >> cpu->cpu_index); >> qemu_thread_create(cpu->thread, thread_name, qemu_tcg_cpu_thread_fn, >> @@ -1254,7 +1251,6 @@ static void qemu_tcg_init_vcpu(CPUState *cpu) >> tcg_cpu_thread = cpu->thread; >> } else { >> cpu->thread = tcg_cpu_thread; >> - cpu->halt_cond = tcg_halt_cond; >> }