On 20 November 2012 12:30, 陳韋任 (Wei-Ren Chen) <che...@iis.sinica.edu.tw> wrote:
> When tb_remove was first commited at fd6ce8f6, there were three different
> calls pass different names to offsetof. In current codebase, the other two
> calls are replaced with tb_page_remove. There is no need to have a general
> tb_remove. Omit passing the third parameter and using tb1->phys_hash_next
> directly.
I like this, it makes the function less confusing to remove this
now-unneeded generality.
> Signed-off-by: Chen Wei-Ren <che...@iis.sinica.edu.tw>
> ---
> exec.c | 10 ++++------
> 1 files changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/exec.c b/exec.c
> index 8435de0..e54fce2 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -863,17 +863,16 @@ static void tb_page_check(void)
> #endif
>
> /* invalidate one TB */
This comment is now a bit out of date (and in fact it has been for
some time) and should probably be deleted. (The function that really
needs a comment is the top level tb_phys_invalidate(), rather than
the helpers tb_hash_remove/tb_page_remove/tb_jmp_remove.)
> -static inline void tb_remove(TranslationBlock **ptb, TranslationBlock *tb,
> - int next_offset)
> +static inline void tb_hash_remove(TranslationBlock **ptb, TranslationBlock
> *tb)
> {
> TranslationBlock *tb1;
> for(;;) {
> tb1 = *ptb;
> if (tb1 == tb) {
> - *ptb = *(TranslationBlock **)((char *)tb1 + next_offset);
> + *ptb = tb1->phys_hash_next;
> break;
> }
> - ptb = (TranslationBlock **)((char *)tb1 + next_offset);
> + ptb = &(tb1->phys_hash_next);
You don't need these brackets.
> }
> }
>
> @@ -940,8 +939,7 @@ void tb_phys_invalidate(TranslationBlock *tb,
> tb_page_addr_t page_addr)
> /* remove the TB from the hash list */
> phys_pc = tb->page_addr[0] + (tb->pc & ~TARGET_PAGE_MASK);
> h = tb_phys_hash_func(phys_pc);
> - tb_remove(&tb_phys_hash[h], tb,
> - offsetof(TranslationBlock, phys_hash_next));
> + tb_hash_remove(&tb_phys_hash[h], tb);
>
> /* remove the TB from the page list */
> if (tb->page_addr[0] != page_addr) {
> --
> 1.7.3.4
-- PMM
