Re: [RFA] dcache invalidate fix

[Michael Snyder <msnyder at vmware dot com>]

Doug Evans wrote:
> On Tue, Nov 10, 2009 at 1:33 PM, Michael Snyder <msnyder@vmware.com> wrote:
> > Doug, I'm vague about this, but it seems right and it fixes the
> > bug that I'm running into.
> >
> > It seems like dcache_invalidate_line needs to remove the block
> > from the in use list at the same time as adding it to the freed
> > list.
> >
> > The problem that bit me was getting the two lists cross-linked,
> > which eventually led to an infinite loop behavior in dcache_invalidate.
> >
> >
> > 2009-11-10  Michael Snyder  <msnyder@vmware.com>
> >
> >        * dcache.c (dcache_invalidate_line): Remove block from used list
> >        when adding it to freed list.
> >
> > Index: dcache.c
> > ===================================================================
> > RCS file: /cvs/src/src/gdb/dcache.c,v
> > retrieving revision 1.37
> > diff -u -p -r1.37 dcache.c
> > --- dcache.c    10 Nov 2009 18:36:50 -0000      1.37
> > +++ dcache.c    10 Nov 2009 21:31:03 -0000
> > @@ -167,10 +167,18 @@ dcache_invalidate_line (DCACHE *dcache,
> >
> >   if (db)
> >     {
> > +      struct dcache_block *db2;
> >       splay_tree_remove (dcache->tree, (splay_tree_key) db->addr);
> >       db->newer = dcache->freelist;
> >       dcache->freelist = db;
> >       --dcache->size;
> > +      /* Remove db from dcache in-use chain.  */
> > +      for (db2 = dcache->oldest; db2; db2 = db2->newer)
> > +       if (db2->newer == db)
> > +         {
> > +           dcache->newest = db2;
> > +           db2->newer = NULL;
> > +         }
> >     }
> >  }
>
> Blech.  Thanks for catching this.
>
> The list will contain 4096 elements at this point so I think we need
> to do something different.
> The following comes to mind.
>
> I'll check it in in a few days if there are no objections.

FYI, this fixes the problem that caused me to look into it.
Thanks,
Michael

> 2009-11-11  Doug Evans  <dje@google.com>
>
>         * dcache.c (dcache_block): Replace member newer with next,prev.
>         (dcache_struct): Delete member newest.
> 	(block_func): New typedef.
>         (append_block, remove_block, for_each_block): New functions.
>         (invalidate_block, free_block): New functions.
>         (dcache_invalidate): Update
>         (dcache_invalidate_line, dcache_alloc): Update to use new list
>         accessors.
>         (dcache_free): Ditto.  Fix memory leak.