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

[Sergey Fedorov <serge.fdrv@gmail.com>]

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