Re: [RFA] dcache invalidate fix

Michael Snyder
2009-11-12T00:06:45Z

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.