Re: [Qemu-devel] [PATCH v3 36/39] qcow2: Allow configuring the L2 slice size

[Max Reitz]

On 2018-01-26 16:00, Alberto Garcia wrote:
> Now that the code is ready to handle L2 slices we can finally add an
> option to allow configuring their size.
> 
> An L2 slice is the portion of an L2 table that is read by the qcow2
> cache. Until now the cache was always reading full L2 tables, and
> since the L2 table size is equal to the cluster size this was not very
> efficient with large clusters. Here's a more detailed explanation of
> why it makes sense to have smaller cache entries in order to load L2
> data:
> 
>https://lists.gnu.org/archive/html/qemu-block/2017-09/msg00635.html
> 
> This patch introduces a new command-line option to the qcow2 driver
> named l2-cache-entry-size (cf. l2-cache-size). The cache entry size
> has the same restrictions as the cluster size: it must be a power of
> two and it has the same range of allowed values, with the additional
> requirement that it must not be larger than the cluster size.
> 
> The L2 cache entry size (L2 slice size) remains equal to the cluster
> size for now by default, so this feature must be explicitly enabled.
> Although my tests show that 4KB slices consistently improve
> performance and give the best results, let's wait and make more tests
> with different cluster sizes before deciding on an optimal default.
> 
> Now that the cache entry size is not necessarily equal to the cluster
> size we need to reflect that in the MIN_L2_CACHE_SIZE documentation.
> That minimum value is a requirement of the COW algorithm: we need to
> read two L2 slices (and not two L2 tables) in order to do COW, see
> l2_allocate() for the actual code.
> 
> Signed-off-by: Alberto Garcia 
> ---
>  block/qcow2-cache.c  | 10 --
>  block/qcow2.c| 34 +++---
>  block/qcow2.h|  6 --
>  qapi/block-core.json |  6 ++
>  4 files changed, 45 insertions(+), 11 deletions(-)

Reviewed-by: Max Reitz 



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v3 36/39] qcow2: Allow configuring the L2 slice size

[Eric Blake]

On 01/26/2018 09:00 AM, Alberto Garcia wrote:
> Now that the code is ready to handle L2 slices we can finally add an
> option to allow configuring their size.
> 
> An L2 slice is the portion of an L2 table that is read by the qcow2
> cache. Until now the cache was always reading full L2 tables, and
> since the L2 table size is equal to the cluster size this was not very
> efficient with large clusters. Here's a more detailed explanation of
> why it makes sense to have smaller cache entries in order to load L2
> data:
> 
>https://lists.gnu.org/archive/html/qemu-block/2017-09/msg00635.html
> 
> This patch introduces a new command-line option to the qcow2 driver
> named l2-cache-entry-size (cf. l2-cache-size). The cache entry size
> has the same restrictions as the cluster size: it must be a power of
> two and it has the same range of allowed values, with the additional
> requirement that it must not be larger than the cluster size.
> 
> The L2 cache entry size (L2 slice size) remains equal to the cluster
> size for now by default, so this feature must be explicitly enabled.
> Although my tests show that 4KB slices consistently improve
> performance and give the best results, let's wait and make more tests
> with different cluster sizes before deciding on an optimal default.
> 
> Now that the cache entry size is not necessarily equal to the cluster
> size we need to reflect that in the MIN_L2_CACHE_SIZE documentation.
> That minimum value is a requirement of the COW algorithm: we need to
> read two L2 slices (and not two L2 tables) in order to do COW, see
> l2_allocate() for the actual code.
> 
> Signed-off-by: Alberto Garcia 
> ---
Reviewed-by: Eric Blake 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org



signature.asc
Description: OpenPGP digital signature

[Qemu-devel] [PATCH v3 36/39] qcow2: Allow configuring the L2 slice size

[Alberto Garcia]

Now that the code is ready to handle L2 slices we can finally add an
option to allow configuring their size.

An L2 slice is the portion of an L2 table that is read by the qcow2
cache. Until now the cache was always reading full L2 tables, and
since the L2 table size is equal to the cluster size this was not very
efficient with large clusters. Here's a more detailed explanation of
why it makes sense to have smaller cache entries in order to load L2
data:

   https://lists.gnu.org/archive/html/qemu-block/2017-09/msg00635.html

This patch introduces a new command-line option to the qcow2 driver
named l2-cache-entry-size (cf. l2-cache-size). The cache entry size
has the same restrictions as the cluster size: it must be a power of
two and it has the same range of allowed values, with the additional
requirement that it must not be larger than the cluster size.

The L2 cache entry size (L2 slice size) remains equal to the cluster
size for now by default, so this feature must be explicitly enabled.
Although my tests show that 4KB slices consistently improve
performance and give the best results, let's wait and make more tests
with different cluster sizes before deciding on an optimal default.

Now that the cache entry size is not necessarily equal to the cluster
size we need to reflect that in the MIN_L2_CACHE_SIZE documentation.
That minimum value is a requirement of the COW algorithm: we need to
read two L2 slices (and not two L2 tables) in order to do COW, see
l2_allocate() for the actual code.

Signed-off-by: Alberto Garcia 
---
 block/qcow2-cache.c  | 10 --
 block/qcow2.c| 34 +++---
 block/qcow2.h|  6 --
 qapi/block-core.json |  6 ++
 4 files changed, 45 insertions(+), 11 deletions(-)

diff --git a/block/qcow2-cache.c b/block/qcow2-cache.c
index b1aa42477e..d9dafa31e5 100644
--- a/block/qcow2-cache.c
+++ b/block/qcow2-cache.c
@@ -120,14 +120,20 @@ void qcow2_cache_clean_unused(Qcow2Cache *c)
 c->cache_clean_lru_counter = c->lru_counter;
 }
 
-Qcow2Cache *qcow2_cache_create(BlockDriverState *bs, int num_tables)
+Qcow2Cache *qcow2_cache_create(BlockDriverState *bs, int num_tables,
+   unsigned table_size)
 {
 BDRVQcow2State *s = bs->opaque;
 Qcow2Cache *c;
 
+assert(num_tables > 0);
+assert(is_power_of_2(table_size));
+assert(table_size >= (1 << MIN_CLUSTER_BITS));
+assert(table_size <= s->cluster_size);
+
 c = g_new0(Qcow2Cache, 1);
 c->size = num_tables;
-c->table_size = s->cluster_size;
+c->table_size = table_size;
 c->entries = g_try_new0(Qcow2CachedTable, num_tables);
 c->table_array = qemu_try_blockalign(bs->file->bs,
  (size_t) num_tables * c->table_size);
diff --git a/block/qcow2.c b/block/qcow2.c
index 529becfa30..98762433a0 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -674,6 +674,11 @@ static QemuOptsList qcow2_runtime_opts = {
 .help = "Maximum L2 table cache size",
 },
 {
+.name = QCOW2_OPT_L2_CACHE_ENTRY_SIZE,
+.type = QEMU_OPT_SIZE,
+.help = "Size of each entry in the L2 cache",
+},
+{
 .name = QCOW2_OPT_REFCOUNT_CACHE_SIZE,
 .type = QEMU_OPT_SIZE,
 .help = "Maximum refcount block cache size",
@@ -745,6 +750,7 @@ static void qcow2_attach_aio_context(BlockDriverState *bs,
 
 static void read_cache_sizes(BlockDriverState *bs, QemuOpts *opts,
  uint64_t *l2_cache_size,
+ uint64_t *l2_cache_entry_size,
  uint64_t *refcount_cache_size, Error **errp)
 {
 BDRVQcow2State *s = bs->opaque;
@@ -760,6 +766,9 @@ static void read_cache_sizes(BlockDriverState *bs, QemuOpts 
*opts,
 *refcount_cache_size = qemu_opt_get_size(opts,
  QCOW2_OPT_REFCOUNT_CACHE_SIZE, 0);
 
+*l2_cache_entry_size = qemu_opt_get_size(
+opts, QCOW2_OPT_L2_CACHE_ENTRY_SIZE, s->cluster_size);
+
 if (combined_cache_size_set) {
 if (l2_cache_size_set && refcount_cache_size_set) {
 error_setg(errp, QCOW2_OPT_CACHE_SIZE ", " QCOW2_OPT_L2_CACHE_SIZE
@@ -800,6 +809,15 @@ static void read_cache_sizes(BlockDriverState *bs, 
QemuOpts *opts,
  / DEFAULT_L2_REFCOUNT_SIZE_RATIO;
 }
 }
+
+if (*l2_cache_entry_size < (1 << MIN_CLUSTER_BITS) ||
+*l2_cache_entry_size > s->cluster_size ||
+!is_power_of_2(*l2_cache_entry_size)) {
+error_setg(errp, "L2 cache entry size must be a power of two "
+   "between %d and the cluster size (%d)",
+   1 << MIN_CLUSTER_BITS, s->cluster_size);
+return;
+}
 }
 
 typedef struct Qcow2ReopenState {
@@ -822,7 +840,7 @@ static int qcow2_update_options_prepare(BlockDriverState 
*bs,
 QemuOpts *opts = NULL;
 const char