qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH] page_cache: Fix memory leak


From: Peter Lieven
Subject: Re: [Qemu-devel] [PATCH] page_cache: Fix memory leak
Date: Mon, 25 Feb 2013 13:08:38 +0100

Am 25.02.2013 um 12:58 schrieb Orit Wasserman <address@hidden>:

> On 02/25/2013 01:42 PM, Peter Lieven wrote:
>> XBZRLE encoded migration introduced a MRU page cache meachnism.
>> Unfortunately, cached items where never freed on a collision.
>> 
>> This lead to out of memory conditions during XBZRLE migration
>> if the page cache was small and there where a lot of collisions.
>> 
>> Signed-off-by: Peter Lieven <address@hidden>
>> ---
>> page_cache.c |    4 +++-
>> 1 file changed, 3 insertions(+), 1 deletion(-)
>> 
>> diff --git a/page_cache.c b/page_cache.c
>> index ba5640b..a6c3a15 100644
>> --- a/page_cache.c
>> +++ b/page_cache.c
>> @@ -152,8 +152,10 @@ void cache_insert(PageCache *cache, uint64_t addr, 
>> uint8_t *pdata)
>>     /* actual update of entry */
>>     it = cache_get_by_addr(cache, addr);
>> 
>> -    if (!it->it_data) {
>> +    if (it->it_data == NULL) {
>>         cache->num_items++;
>> +    } else {
>> +        g_free(it->it_data);
> 
> Why? we don't allocate here but just store the pointer.
> It is the caller responsibility to allocate/free the data,
> for example for migration it is the guest memory page.

It is being dupped when the page is added to the cache.

            cache_insert(XBZRLE.cache, current_addr,
                         g_memdup(current_data, TARGET_PAGE_SIZE));

This is, of course, necessary because the memory might change
and you need the state of the page that has been transferred to the
destination.

It is also obvious that the cache needs to hold his own copy because
it frees the data on finish and on resize.

Peter


> 
> Orit
>>     }
>> 
>>     it->it_data = pdata;
> 




reply via email to

[Prev in Thread] Current Thread [Next in Thread]