From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34306) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bK2K2-00079F-F3 for qemu-devel@nongnu.org; Mon, 04 Jul 2016 07:45:55 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bK2Jy-0004cn-9E for qemu-devel@nongnu.org; Mon, 04 Jul 2016 07:45:53 -0400 Received: from mail-wm0-x233.google.com ([2a00:1450:400c:c09::233]:38076) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bK2Jx-0004ce-HK for qemu-devel@nongnu.org; Mon, 04 Jul 2016 07:45:50 -0400 Received: by mail-wm0-x233.google.com with SMTP id r201so112037236wme.1 for ; Mon, 04 Jul 2016 04:45:49 -0700 (PDT) References: <1467389770-9738-1-git-send-email-alex.bennee@linaro.org> <1467389770-9738-3-git-send-email-alex.bennee@linaro.org> <20160702003942.GB2295@flamenco> From: Alex =?utf-8?Q?Benn=C3=A9e?= In-reply-to: <20160702003942.GB2295@flamenco> Date: Mon, 04 Jul 2016 12:45:52 +0100 Message-ID: <87lh1hmxsf.fsf@linaro.org> MIME-Version: 1.0 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: "Emilio G. Cota" Cc: mttcg@listserver.greensocs.com, qemu-devel@nongnu.org, fred.konrad@greensocs.com, a.rigo@virtualopensystems.com, serge.fdrv@gmail.com, bobby.prani@gmail.com, rth@twiddle.net, mark.burton@greensocs.com, pbonzini@redhat.com, jan.kiszka@siemens.com, peter.maydell@linaro.org, claudio.fontana@huawei.com, Peter Crosthwaite Emilio G. Cota writes: > On Fri, Jul 01, 2016 at 17:16:10 +0100, Alex Bennée wrote: >> Lock contention in the hot path of moving between existing patched >> TranslationBlocks is the main drag in multithreaded performance. This >> patch pushes the tb_lock() usage down to the two places that really need >> it: >> >> - code generation (tb_gen_code) >> - jump patching (tb_add_jump) >> >> The rest of the code doesn't really need to hold a lock as it is either >> using per-CPU structures, atomically updated or designed to be used in >> concurrent read situations (qht_lookup). >> >> To keep things simple I removed the #ifdef CONFIG_USER_ONLY stuff as the >> locks become NOPs anyway until the MTTCG work is completed. > > From a scalability point of view it would be better to have a single > critical section. You mean merge the critical region for patching and code-generation? > > From a correctness point of view, we're reading tb->page_addr[1] > without holding a lock. This field is set after qht_insert(tb), > so we might read a yet-uninitialized value. > > I propose to just extend the critical section, like we used to > do with tcg_lock_reset. > > Emilio -- Alex Bennée