On 01/04/2012 01:46 PM, Stefan Hajnoczi wrote:
> On Tue, Jan 3, 2012 at 3:34 PM, Orit Wasserman <owass...@redhat.com> wrote:
>> +static unsigned long cache_get_cache_pos(ram_addr_t address)
>> +{
>> + unsigned long pos;
>> +
>> + assert(cache_num_buckets);
>> + pos = (address/TARGET_PAGE_SIZE) & (cache_num_buckets - 1);
>
> Where do we guarantee that cache_num_buckets is a power of 2?
We should do it to when setting cache size , I will add the check.
>
>> +static void cache_insert(unsigned long addr, uint8_t *pdata)
>> +{
>> + unsigned long pos;
>> + int slot = -1;
>> + CacheBucket *bucket;
>> +
>> + pos = cache_get_cache_pos(addr);
>> + assert(page_cache);
>> + bucket = &page_cache[pos];
>> + slot = cache_get_oldest(bucket); /* evict LRU */
>> +
>> + /* actual update of entry */
>> + CacheItem *it = cache_item_get(pos, slot);
>> + if (!it->it_data) {
>> + cache_num_items++;
>> + }
>> + g_free(it->it_data);
>> +
>> + it->it_data = g_malloc0(TARGET_PAGE_SIZE);
>> + memcpy(it->it_data, pdata, TARGET_PAGE_SIZE);
>
> If we're evicting an entry:
> 1. Free existing data.
> 2. Allocate new data.
> 3. Zero new data.
> 4. Memcpy pdata into new data.
>
> That does a bunch of extra work. How about:
> 1. Memcpy pdata over old data.
I noticed it too , i will fix it in next version.
>
> So:
> if (!it->it_data) {
> cache_num_items++;
> it->it_data = g_malloc(TARGET_PAGE_SIZE);
> }
> memcpy(it->it_data, pdata, TARGET_PAGE_SIZE);
