qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH] exec.c: Use tb1->phys_hash_next directly in tb_


From: Wei-Ren Chen
Subject: Re: [Qemu-devel] [PATCH] exec.c: Use tb1->phys_hash_next directly in tb_remove
Date: Sun, 25 Nov 2012 15:50:42 +0800
User-agent: Mutt/1.5.21 (2010-09-15)

  ping?

On Tue, Nov 20, 2012 at 12:41:03PM +0000, Peter Maydell wrote:
> On 20 November 2012 12:30, 陳韋任 (Wei-Ren Chen) <address@hidden> 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 <address@hidden>
> > ---
> >  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

-- 
Wei-Ren Chen (陳韋任)
Computer Systems Lab, Institute of Information Science,
Academia Sinica, Taiwan (R.O.C.)
Tel:886-2-2788-3799 #1667
Homepage: http://people.cs.nctu.edu.tw/~chenwj



reply via email to

[Prev in Thread] Current Thread [Next in Thread]