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