diff mbox

[3/3] qcow2: Add DEFAULT_L2_CACHE_CLUSTERS

Message ID 1433174959-7176-4-git-send-email-mreitz@redhat.com
State New
Headers show

Commit Message

Max Reitz June 1, 2015, 4:09 p.m. UTC 
If a relatively large cluster size is chosen, the default of 1 MB L2
cache is not really appropriate. In this case, unless overridden by the
user, the default cache size should not be determined by its size in
bytes but by the number of L2 tables (clusters) it is supposed to
contain.

Note that without this patch, MIN_L2_CACHE_SIZE will effectively take
over the same role. However, providing space for just two L2 tables is
not enough to be the default.

Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block/qcow2.c | 11 ++++++++---
 block/qcow2.h |  2 ++
 2 files changed, 10 insertions(+), 3 deletions(-)

Comments

Alberto Garcia June 1, 2015, 5:16 p.m. UTC | #1 
On Mon 01 Jun 2015 06:09:19 PM CEST, Max Reitz wrote:
> If a relatively large cluster size is chosen, the default of 1 MB L2
> cache is not really appropriate. In this case, unless overridden by the
> user, the default cache size should not be determined by its size in
> bytes but by the number of L2 tables (clusters) it is supposed to
> contain.
>
> Note that without this patch, MIN_L2_CACHE_SIZE will effectively take
> over the same role. However, providing space for just two L2 tables is
> not enough to be the default.
>
> Signed-off-by: Max Reitz <mreitz@redhat.com>

Reviewed-by: Alberto Garcia <berto@igalia.com>

Berto
diff mbox

Patch

diff --git a/block/qcow2.c b/block/qcow2.c
index f7b4cc6..c4f6938 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -483,9 +483,11 @@  static const char *overlap_bool_option_names[QCOW2_OL_MAX_BITNR] = {
     [QCOW2_OL_INACTIVE_L2_BITNR]    = QCOW2_OPT_OVERLAP_INACTIVE_L2,
 };
 
-static void read_cache_sizes(QemuOpts *opts, uint64_t *l2_cache_size,
+static void read_cache_sizes(BlockDriverState *bs, QemuOpts *opts,
+                             uint64_t *l2_cache_size,
                              uint64_t *refcount_cache_size, Error **errp)
 {
+    BDRVQcowState *s = bs->opaque;
     uint64_t combined_cache_size;
     bool l2_cache_size_set, refcount_cache_size_set, combined_cache_size_set;
 
@@ -525,7 +527,9 @@  static void read_cache_sizes(QemuOpts *opts, uint64_t *l2_cache_size,
         }
     } else {
         if (!l2_cache_size_set && !refcount_cache_size_set) {
-            *l2_cache_size = DEFAULT_L2_CACHE_BYTE_SIZE;
+            *l2_cache_size = MAX(DEFAULT_L2_CACHE_BYTE_SIZE,
+                                 (uint64_t)DEFAULT_L2_CACHE_CLUSTERS
+                                 * s->cluster_size);
             *refcount_cache_size = *l2_cache_size
                                  / DEFAULT_L2_REFCOUNT_SIZE_RATIO;
         } else if (!l2_cache_size_set) {
@@ -803,7 +807,8 @@  static int qcow2_open(BlockDriverState *bs, QDict *options, int flags,
         goto fail;
     }
 
-    read_cache_sizes(opts, &l2_cache_size, &refcount_cache_size, &local_err);
+    read_cache_sizes(bs, opts, &l2_cache_size, &refcount_cache_size,
+                     &local_err);
     if (local_err) {
         error_propagate(errp, local_err);
         ret = -EINVAL;
diff --git a/block/qcow2.h b/block/qcow2.h
index aa20022..5936d29 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -68,6 +68,8 @@ 
 /* Must be at least 4 to cover all cases of refcount table growth */
 #define MIN_REFCOUNT_CACHE_SIZE 4 /* clusters */
 
+/* Whichever is more */
+#define DEFAULT_L2_CACHE_CLUSTERS 8 /* clusters */
 #define DEFAULT_L2_CACHE_BYTE_SIZE 1048576 /* bytes */
 
 /* The refblock cache needs only a fourth of the L2 cache size to cover as many