qemu-devel
[Top][All Lists]
Advanced

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

Re: [Qemu-devel] [PATCH 1/6] qcow2: use one single memory block for the


From: Eric Blake
Subject: Re: [Qemu-devel] [PATCH 1/6] qcow2: use one single memory block for the L2/refcount cache tables
Date: Thu, 30 Apr 2015 09:08:05 -0600
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.6.0

On 04/30/2015 04:11 AM, Alberto Garcia wrote:
> The qcow2 L2/refcount cache contains one separate table for each cache
> entry. Doing one allocation per table adds unnecessary overhead and it
> also requires us to store the address of each table separately.
> 
> Since the size of the cache is constant during its lifetime, it's
> better to have an array that contains all the tables using one single
> allocation.
> 
> In my tests measuring freshly created caches with sizes 128MB (L2) and
> 32MB (refcount) this uses around 10MB of RAM less.
> 
> Signed-off-by: Alberto Garcia <address@hidden>
> ---
>  block/qcow2-cache.c | 48 +++++++++++++++++++++---------------------------
>  1 file changed, 21 insertions(+), 27 deletions(-)
> 

>  
>  typedef struct Qcow2CachedTable {
> -    void*   table;
>      int64_t offset;
>      bool    dirty;
>      int     cache_hits;
> @@ -40,39 +39,34 @@ struct Qcow2Cache {
>      struct Qcow2Cache*      depends;
>      int                     size;
>      bool                    depends_on_flush;
> +    void                   *table_array;
> +    int                     table_size;

Should this be size_t? [1]

>  };
>  
> +static inline void *table_addr(Qcow2Cache *c, int table)
> +{
> +    return c->table_array + table * c->table_size;

Addition on void* is not portable.

> +}
> +
>  Qcow2Cache *qcow2_cache_create(BlockDriverState *bs, int num_tables)
>  {
>      BDRVQcowState *s = bs->opaque;
>      Qcow2Cache *c;
> -    int i;
>  
>      c = g_new0(Qcow2Cache, 1);
>      c->size = num_tables;
> +    c->table_size = s->cluster_size;

[1] Oh, maybe not, since BDRVQcowState declares cluster_size as int.

>      c->entries = g_try_new0(Qcow2CachedTable, num_tables);
> -    if (!c->entries) {
> -        goto fail;
> -    }
> +    c->table_array = qemu_try_blockalign(bs->file, num_tables * 
> c->table_size);

Are we sure this won't overflow?

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


reply via email to

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