From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44023) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1agenN-0005w6-CZ for qemu-devel@nongnu.org; Thu, 17 Mar 2016 16:45:26 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1agenK-0005JD-70 for qemu-devel@nongnu.org; Thu, 17 Mar 2016 16:45:25 -0400 Received: from mail-lb0-x22e.google.com ([2a00:1450:4010:c04::22e]:36684) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1agenJ-0005J9-UM for qemu-devel@nongnu.org; Thu, 17 Mar 2016 16:45:22 -0400 Received: by mail-lb0-x22e.google.com with SMTP id qe11so21852186lbc.3 for ; Thu, 17 Mar 2016 13:45:21 -0700 (PDT) References: <1458222382-6498-1-git-send-email-sergey.fedorov@linaro.org> <1458222382-6498-4-git-send-email-sergey.fedorov@linaro.org> <56EAF001.2050303@twiddle.net> <56EB060A.9080302@redhat.com> From: Sergey Fedorov Message-ID: <56EB175F.804@gmail.com> Date: Thu, 17 Mar 2016 23:45:19 +0300 MIME-Version: 1.0 In-Reply-To: <56EB060A.9080302@redhat.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 3/5] tcg: always keep jump target and tb->jmp_next consistent List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini , Richard Henderson , sergey.fedorov@linaro.org, qemu-devel@nongnu.org Cc: Peter Crosthwaite On 17/03/16 22:31, Paolo Bonzini wrote: > > On 17/03/2016 18:57, Richard Henderson wrote: >>> @@ -951,18 +959,10 @@ static inline void tb_jmp_remove(TranslationBlock *tb, int n) >>> } >>> /* now we can suppress tb(n) from the list */ >>> *ptb = tb->jmp_next[n]; >>> - >>> - tb->jmp_next[n] = NULL; >>> + tb_reset_jump(tb, n); >> What's the motivation here? This implies an extra cache flush. >> Where were we resetting the jump previously? Or is this a bug >> in that we *weren't* resetting the jump previously? > Indeed I think this patch can be removed if it has a performance effect > on machines that require icache invalidation. If it doesn't, it would > be just a small code simplification. In fact, tb_jmp_remove() is only supposed to remove the TB from a list of all TB's jumping to the same TB which is n-th jump destination of the given TB. This function is only called in tb_phys_invalidate() for the TB being invalidated. Thus we don't have to patch that TB anymore. We don't even have to do "tb->jmp_next[n] = NULL" here. Probably it's time to audit the code that handles direct jumping and clean-up/document/rename things to make it more easy to understand? :) Kind regards, Sergey