From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:32957) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b5Jsp-0006ZR-7x for qemu-devel@nongnu.org; Tue, 24 May 2016 17:29:00 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1b5Jsl-00013R-0f for qemu-devel@nongnu.org; Tue, 24 May 2016 17:28:58 -0400 Received: from mail-lf0-x242.google.com ([2a00:1450:4010:c07::242]:34726) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b5Jsk-00012a-1X for qemu-devel@nongnu.org; Tue, 24 May 2016 17:28:54 -0400 Received: by mail-lf0-x242.google.com with SMTP id 65so1990912lfq.1 for ; Tue, 24 May 2016 14:28:53 -0700 (PDT) References: <1459870344-16773-1-git-send-email-alex.bennee@linaro.org> <1459870344-16773-11-git-send-email-alex.bennee@linaro.org> From: Sergey Fedorov Message-ID: <5744C792.6030908@gmail.com> Date: Wed, 25 May 2016 00:28:50 +0300 MIME-Version: 1.0 In-Reply-To: <1459870344-16773-11-git-send-email-alex.bennee@linaro.org> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [RFC v2 10/11] tcg: drop global lock during TCG code execution List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: =?UTF-8?Q?Alex_Benn=c3=a9e?= , mttcg@listserver.greensocs.com, fred.konrad@greensocs.com, a.rigo@virtualopensystems.com, cota@braap.org Cc: qemu-devel@nongnu.org, mark.burton@greensocs.com, pbonzini@redhat.com, jan.kiszka@siemens.com, rth@twiddle.net, peter.maydell@linaro.org, claudio.fontana@huawei.com, Peter Crosthwaite , Eduardo Habkost , "Michael S. Tsirkin" On 05/04/16 18:32, Alex BennĂ©e wrote: > diff --git a/cpu-exec.c b/cpu-exec.c > index bd50fef..f558508 100644 > --- a/cpu-exec.c > +++ b/cpu-exec.c > @@ -28,6 +28,7 @@ > #include "qemu/rcu.h" > #include "exec/tb-hash.h" > #include "exec/log.h" > +#include "qemu/main-loop.h" > #if defined(TARGET_I386) && !defined(CONFIG_USER_ONLY) > #include "hw/i386/apic.h" > #endif > @@ -452,6 +453,8 @@ int cpu_exec(CPUState *cpu) > for(;;) { > interrupt_request = cpu->interrupt_request; > if (unlikely(interrupt_request)) { > + qemu_mutex_lock_iothread(); > + > if (unlikely(cpu->singlestep_enabled & SSTEP_NOIRQ)) { > /* Mask out external interrupts for this step. */ > interrupt_request &= ~CPU_INTERRUPT_SSTEP_MASK; > @@ -504,6 +507,11 @@ int cpu_exec(CPUState *cpu) > the program flow was changed */ > next_tb = 0; > } > + > + if (qemu_mutex_iothread_locked()) { > + qemu_mutex_unlock_iothread(); > + } > + Why do we need to check for "qemu_mutex_iothread_locked()" here? iothread lock is always held here, isn't it? > } > if (unlikely(cpu->exit_request > || replay_has_interrupt())) { (snip) > diff --git a/cputlb.c b/cputlb.c > index 466663b..1412049 100644 > --- a/cputlb.c > +++ b/cputlb.c > @@ -18,6 +18,7 @@ > */ > > #include "qemu/osdep.h" > +#include "qemu/main-loop.h" No need in this include. > #include "cpu.h" > #include "exec/exec-all.h" > #include "exec/memory.h" > diff --git a/exec.c b/exec.c > index c46c123..9e83673 100644 > --- a/exec.c > +++ b/exec.c (snip) > @@ -2347,8 +2353,14 @@ static void io_mem_init(void) > memory_region_init_io(&io_mem_rom, NULL, &unassigned_mem_ops, NULL, NULL, UINT64_MAX); > memory_region_init_io(&io_mem_unassigned, NULL, &unassigned_mem_ops, NULL, > NULL, UINT64_MAX); > + > + /* io_mem_notdirty calls tb_invalidate_phys_page_fast, > + * which must be called without the iothread mutex. > + */ notdirty_mem_write() operates on page dirty flags. Is it safe to do so out of iothread lock? > memory_region_init_io(&io_mem_notdirty, NULL, ¬dirty_mem_ops, NULL, > NULL, UINT64_MAX); > + memory_region_clear_global_locking(&io_mem_notdirty); > + > memory_region_init_io(&io_mem_watch, NULL, &watch_mem_ops, NULL, > NULL, UINT64_MAX); > } (snip) > diff --git a/translate-all.c b/translate-all.c > index 935d24c..0c377bb 100644 > --- a/translate-all.c > +++ b/translate-all.c > @@ -1318,6 +1318,7 @@ void tb_invalidate_phys_range(tb_page_addr_t start, tb_page_addr_t end) > * this TB. > * > * Called with mmap_lock held for user-mode emulation > + * If called from generated code, iothread mutex must not be held. What does that mean? If iothread mutex is not required by the function then why mention it here at all? Kind regards, Sergey