From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57586) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bKQcB-0004jU-Iu for qemu-devel@nongnu.org; Tue, 05 Jul 2016 09:42:16 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bKQc5-0001Gt-II for qemu-devel@nongnu.org; Tue, 05 Jul 2016 09:42:14 -0400 Received: from mail-wm0-x241.google.com ([2a00:1450:400c:c09::241]:34321) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bKQc5-0001Gl-BO for qemu-devel@nongnu.org; Tue, 05 Jul 2016 09:42:09 -0400 Received: by mail-wm0-x241.google.com with SMTP id 187so28874506wmz.1 for ; Tue, 05 Jul 2016 06:42:09 -0700 (PDT) Sender: Paolo Bonzini References: <1467389770-9738-1-git-send-email-alex.bennee@linaro.org> <1467389770-9738-3-git-send-email-alex.bennee@linaro.org> <20160702003942.GB2295@flamenco> <87lh1hmxsf.fsf@linaro.org> <20160704223001.GA29624@flamenco> <87inwkmj5d.fsf@linaro.org> <38496c08-3c5a-a8a7-dfb2-a81db29c4592@redhat.com> <87h9c4mdqu.fsf@linaro.org> From: Paolo Bonzini Message-ID: Date: Tue, 5 Jul 2016 15:42:06 +0200 MIME-Version: 1.0 In-Reply-To: <87h9c4mdqu.fsf@linaro.org> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [PATCH 2/2] cpu-exec: remove tb_lock from the hot-path List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: =?UTF-8?Q?Alex_Benn=c3=a9e?= Cc: mttcg@listserver.greensocs.com, peter.maydell@linaro.org, Peter Crosthwaite , jan.kiszka@siemens.com, claudio.fontana@huawei.com, a.rigo@virtualopensystems.com, qemu-devel@nongnu.org, "Emilio G. Cota" , mark.burton@greensocs.com, serge.fdrv@gmail.com, bobby.prani@gmail.com, fred.konrad@greensocs.com, rth@twiddle.net On 05/07/2016 15:11, Alex Bennée wrote: > > Paolo Bonzini writes: > >> On 05/07/2016 13:14, Alex Bennée wrote: >>> /* >>> * Patch the last TB with a jump to the current TB. >>> * >>> * Modification of the TB has to be protected with tb_lock which can >>> * either be already held or taken here. >>> */ >>> static inline void maybe_patch_last_tb(CPUState *cpu, >>> TranslationBlock *tb, >>> TranslationBlock **last_tb, >>> int tb_exit, >>> bool locked) >>> { >>> if (cpu->tb_flushed) { >>> /* Ensure that no TB jump will be modified as the >>> * translation buffer has been flushed. >>> */ >>> *last_tb = NULL; >>> cpu->tb_flushed = false; >>> } >>> #ifndef CONFIG_USER_ONLY >>> /* We don't take care of direct jumps when address mapping changes in >>> * system emulation. So it's not safe to make a direct jump to a TB >>> * spanning two pages because the mapping for the second page can change. >>> */ >>> if (tb->page_addr[1] != -1) { >>> *last_tb = NULL; >>> } >>> #endif >>> /* See if we can patch the calling TB. */ >>> if (*last_tb && !qemu_loglevel_mask(CPU_LOG_TB_NOCHAIN)) { >>> if (!locked) { >>> tb_lock(); >>> } >>> tb_add_jump(*last_tb, tb_exit, tb); >>> if (!locked) { >>> tb_unlock(); >>> } >>> } >>> } >> >> Why not add tb_lock_recursive() and tb_lock_reset()? > > I thought we didn't like having recursive locking? I agree it would make > things a little neater though. I didn't like having recursive mutexes (because recursive mutexes encourage you to be sloppy). Explicitly tagging some tb_lock()s as recursive is fine, though. The explicit tag tells you to be careful. Paolo