From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58418) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1agrhd-0007M7-WF for qemu-devel@nongnu.org; Fri, 18 Mar 2016 06:32:25 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1agrha-0000cW-Q6 for qemu-devel@nongnu.org; Fri, 18 Mar 2016 06:32:21 -0400 Received: from mail-lb0-x231.google.com ([2a00:1450:4010:c04::231]:34955) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1agrha-0000cM-D7 for qemu-devel@nongnu.org; Fri, 18 Mar 2016 06:32:18 -0400 Received: by mail-lb0-x231.google.com with SMTP id bc4so85633523lbc.2 for ; Fri, 18 Mar 2016 03:32:18 -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> <56EB175F.804@gmail.com> From: Sergey Fedorov Message-ID: <56EBD92B.1000800@gmail.com> Date: Fri, 18 Mar 2016 13:32:11 +0300 MIME-Version: 1.0 In-Reply-To: <56EB175F.804@gmail.com> Content-Type: multipart/alternative; boundary="------------060902040203070409060405" 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 This is a multi-part message in MIME format. --------------060902040203070409060405 Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit On 17/03/16 23:45, Sergey Fedorov wrote: > 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. I'll drop the patch from the series in v2 then. Kind regards, Sergey --------------060902040203070409060405 Content-Type: text/html; charset=windows-1252 Content-Transfer-Encoding: 8bit

On 17/03/16 23:45, Sergey Fedorov wrote:

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.

I'll drop the patch from the series in v2 then. Kind regards, Sergey --------------060902040203070409060405--