Re: [Qemu-devel] [PATCH 3/5] tcg: always keep jump target and tb->jmp_next consistent

[Sergey Fedorov]

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