Re: [RFA] dcache invalidate fix

[Michael Snyder <msnyder@vmware.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.