Re: [Qemu-devel] [PATCH] page_cache: dup memory on insert

[Orit Wasserman]

Hi Peter,
Now I get the previous patch, it should have been a patch series :).
The reason we allocate from outside of the page cache is because of cache_resize
that also uses cache_insert but doesn't duplicate the buffer.
There is no memory leak because if the page is cached we don't call cache_insert
but do memcpy instead.

Regards,
Orit
On 02/25/2013 01:52 PM, Peter Lieven wrote:
> The page cache frees all data on finish, on resize and
> if there is collision on insert. So it should be the caches
> responsibility to dup the data that is stored in the cache.
> 
> Signed-off-by: Peter Lieven <address@hidden>
> ---
>  arch_init.c                    |    3 +--
>  include/migration/page_cache.h |    3 ++-
>  page_cache.c                   |    2 +-
>  3 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/arch_init.c b/arch_init.c
> index 8da868b..97bcc29 100644
> --- a/arch_init.c
> +++ b/arch_init.c
> @@ -293,8 +293,7 @@ static int save_xbzrle_page(QEMUFile *f, uint8_t 
> *current_data,
> 
>      if (!cache_is_cached(XBZRLE.cache, current_addr)) {
>          if (!last_stage) {
> -            cache_insert(XBZRLE.cache, current_addr,
> -                         g_memdup(current_data, TARGET_PAGE_SIZE));
> +            cache_insert(XBZRLE.cache, current_addr, current_data);
>          }
>          acct_info.xbzrle_cache_miss++;
>          return -1;
> diff --git a/include/migration/page_cache.h b/include/migration/page_cache.h
> index 3839ac7..87894fe 100644
> --- a/include/migration/page_cache.h
> +++ b/include/migration/page_cache.h
> @@ -57,7 +57,8 @@ bool cache_is_cached(const PageCache *cache, uint64_t addr);
>  uint8_t *get_cached_data(const PageCache *cache, uint64_t addr);
> 
>  /**
> - * cache_insert: insert the page into the cache. the previous value will be 
> overwritten
> + * cache_insert: insert the page into the cache. the page cache
> + * will dup the data on insert. the previous value will be overwritten
>   *
>   * @cache pointer to the PageCache struct
>   * @addr: page address
> diff --git a/page_cache.c b/page_cache.c
> index a6c3a15..e670d91 100644
> --- a/page_cache.c
> +++ b/page_cache.c
> @@ -158,7 +158,7 @@ void cache_insert(PageCache *cache, uint64_t addr, 
> uint8_t *pdata)
>          g_free(it->it_data);
>      }
> 
> -    it->it_data = pdata;
> +    it->it_data = g_memdup(pdata, cache->page_size);
>      it->it_age = ++cache->max_item_age;
>      it->it_addr = addr;
>  }