|
From: | Sergey Fedorov |
Subject: | Re: [Qemu-devel] [PATCH 3/5] tcg: always keep jump target and tb->jmp_next consistent |
Date: | Thu, 17 Mar 2016 23:45:19 +0300 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.6.0 |
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
[Prev in Thread] | Current Thread | [Next in Thread] |