Re: RFA: dcache corruption bug

[Andrew Cagney <ac131313 at cygnus dot com>]

> To fix this problem, I changed dcache_alloc() to initialize the
> address and invalidate the cache line before returning a new block.

Ok.  I suspect they were trying to avoid double writes to the data
section.

> Other related issues:
> 
> * dcache makes no attempt to remove the least-recently-used block when
>   a cache line is recycled.  When a new cache-line is allocated it is
>   added to the tail of the valid list.  If there aren't any cache-lines
>   in the free list, the one from the head of the valid list is recycled.
>   Anyone have any reasons why making this a LRU cache is a bad idea?

From memory, that algorithm isn't totally bad, just less efficient on
average than LRU.  To notice this you must be maxing out the data cache.

> * dcache has a free list and a valid list.  Why not just have one list
>   and invalidate unused cache lines.  When we need a new block, we can
>   just grab a line from the pool.  With a LRU cache, the unused blocks
>   will be the ones subject for re-cycling.

This is just implementation.  If the chain is long then you would want
to be careful to avoid searching through its entire length - host
page/cache problems - more carefully ordered.

> * Is there a generic set of list manipulation macros in libiberty?  I
>   thought there was, but couldn't find anything.

GDB's bcache?

> 2000-08-09  J.T. Conklin  <jtc@redback.com>
> 
>         * dcache.c (dcache_alloc): Changed to take address of line as an
>         argument, and to invalidate cache line before returning.
>         (dcache_peek_byte): Updated.
>         (dcache_poke_byte): Updated.

Ok with me.

	Andrew
> 
> Index: dcache.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/dcache.c,v
> retrieving revision 1.5
> diff -c -r1.5 dcache.c
> *** dcache.c    2000/07/30 01:48:25     1.5
> --- dcache.c    2000/08/09 18:17:28
> ***************
> *** 157,163 ****
> 
>   static int dcache_write_line (DCACHE * dcache, struct dcache_block *db);
> 
> ! static struct dcache_block *dcache_alloc (DCACHE * dcache);
> 
>   static int dcache_writeback (DCACHE * dcache);
> 
> --- 157,163 ----
> 
>   static int dcache_write_line (DCACHE * dcache, struct dcache_block *db);
> 
> ! static struct dcache_block *dcache_alloc (DCACHE * dcache, CORE_ADDR addr);
> 
>   static int dcache_writeback (DCACHE * dcache);
> 
> ***************
> *** 267,281 ****
> 
> 
>   /* Get a free cache block, put or keep it on the valid list,
> !    and return its address.  The caller should store into the block
> !    the address and data that it describes, then remque it from the
> !    free list and insert it into the valid list.  This procedure
> !    prevents errors from creeping in if a memory retrieval is
> !    interrupted (which used to put garbage blocks in the valid
> !    list...).  */
> 
>   static struct dcache_block *
> ! dcache_alloc (DCACHE *dcache)
>   {
>     register struct dcache_block *db;
> 
> --- 267,276 ----
> 
> 
>   /* Get a free cache block, put or keep it on the valid list,
> !    and return its address.  */
> 
>   static struct dcache_block *
> ! dcache_alloc (DCACHE *dcache, CORE_ADDR addr)
>   {
>     register struct dcache_block *db;
> 
> ***************
> *** 297,302 ****
> --- 292,302 ----
>         dcache_write_line (dcache, db);
>       }
> 
> +   db->addr = MASK(addr);
> +   db->refs = 0;
> +   db->anydirty = 0;
> +   memset (db->state, ENTRY_BAD, sizeof (db->data));
> +
>     /* append this line to end of valid list */
>     if (!dcache->valid_head)
>       dcache->valid_head = db;
> ***************
> *** 327,335 ****
>           dcache_write_line (dcache, db);
>         }
>         else
> !       db = dcache_alloc (dcache);
>         immediate_quit++;
> -       db->addr = MASK (addr);
>         while (done < LINE_SIZE)
>         {
>           int try =
> --- 327,335 ----
>           dcache_write_line (dcache, db);
>         }
>         else
> !       db = dcache_alloc (dcache, addr);
> !
>         immediate_quit++;
>         while (done < LINE_SIZE)
>         {
>           int try =
> ***************
> *** 379,387 ****
> 
>     if (!db)
>       {
> !       db = dcache_alloc (dcache);
> !       db->addr = MASK (addr);
> !       memset (db->state, ENTRY_BAD, sizeof (db->data));
>       }
> 
>     db->data[XFORM (addr)] = *ptr;
> --- 379,385 ----
> 
>     if (!db)
>       {
> !       db = dcache_alloc (dcache, addr);
>       }
> 
>     db->data[XFORM (addr)] = *ptr;
> 
> --
> J.T. Conklin
> RedBack Networks