All of lore.kernel.org
 help / color / mirror / Atom feed
From: Stefan Hajnoczi <stefanha@redhat.com>
To: qemu-devel@nongnu.org
Cc: Peter Maydell <peter.maydell@linaro.org>,
	Stefan Hajnoczi <stefanha@redhat.com>,
	Kevin Wolf <kwolf@redhat.com>
Subject: [Qemu-devel] [PULL for-2.10 15/15] qcow2: allocate cluster_cache/cluster_data on demand
Date: Thu, 31 Aug 2017 09:22:10 +0100	[thread overview]
Message-ID: <20170831082210.8362-16-stefanha@redhat.com> (raw)
In-Reply-To: <20170831082210.8362-1-stefanha@redhat.com>

Most qcow2 files are uncompressed so it is wasteful to allocate (32 + 1)
* cluster_size + 512 bytes upfront.  Allocate s->cluster_cache and
s->cluster_data when the first read operation is performance on a
compressed cluster.

The buffers are freed in .bdrv_close().  .bdrv_open() no longer has any
code paths that can allocate these buffers, so remove the free functions
in the error code path.

This patch can result in significant memory savings when many qcow2
disks are attached or backing file chains are long:

Before 12.81% (1,023,193,088B)
After   5.36% (393,893,888B)

Reported-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Tested-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-id: 20170821135530.32344-1-stefanha@redhat.com
Cc: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/qcow2-cluster.c | 17 +++++++++++++++++
 block/qcow2.c         | 12 ------------
 2 files changed, 17 insertions(+), 12 deletions(-)

diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index f06c08f64c..8538533102 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -1516,6 +1516,23 @@ int qcow2_decompress_cluster(BlockDriverState *bs, uint64_t cluster_offset)
         nb_csectors = ((cluster_offset >> s->csize_shift) & s->csize_mask) + 1;
         sector_offset = coffset & 511;
         csize = nb_csectors * 512 - sector_offset;
+
+        /* Allocate buffers on first decompress operation, most images are
+         * uncompressed and the memory overhead can be avoided.  The buffers
+         * are freed in .bdrv_close().
+         */
+        if (!s->cluster_data) {
+            /* one more sector for decompressed data alignment */
+            s->cluster_data = qemu_try_blockalign(bs->file->bs,
+                    QCOW_MAX_CRYPT_CLUSTERS * s->cluster_size + 512);
+            if (!s->cluster_data) {
+                return -ENOMEM;
+            }
+        }
+        if (!s->cluster_cache) {
+            s->cluster_cache = g_malloc(s->cluster_size);
+        }
+
         BLKDBG_EVENT(bs->file, BLKDBG_READ_COMPRESSED);
         ret = bdrv_read(bs->file, coffset >> 9, s->cluster_data,
                         nb_csectors);
diff --git a/block/qcow2.c b/block/qcow2.c
index fbfffadc76..a3679c69e8 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1360,16 +1360,6 @@ static int qcow2_do_open(BlockDriverState *bs, QDict *options, int flags,
         goto fail;
     }
 
-    s->cluster_cache = g_malloc(s->cluster_size);
-    /* one more sector for decompressed data alignment */
-    s->cluster_data = qemu_try_blockalign(bs->file->bs, QCOW_MAX_CRYPT_CLUSTERS
-                                                    * s->cluster_size + 512);
-    if (s->cluster_data == NULL) {
-        error_setg(errp, "Could not allocate temporary cluster buffer");
-        ret = -ENOMEM;
-        goto fail;
-    }
-
     s->cluster_cache_offset = -1;
     s->flags = flags;
 
@@ -1507,8 +1497,6 @@ static int qcow2_do_open(BlockDriverState *bs, QDict *options, int flags,
     if (s->refcount_block_cache) {
         qcow2_cache_destroy(bs, s->refcount_block_cache);
     }
-    g_free(s->cluster_cache);
-    qemu_vfree(s->cluster_data);
     qcrypto_block_free(s->crypto);
     qapi_free_QCryptoBlockOpenOptions(s->crypto_opts);
     return ret;
-- 
2.13.5

  parent reply	other threads:[~2017-08-31  8:23 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-08-31  8:21 [Qemu-devel] [PULL for-2.10 00/15] Block patches Stefan Hajnoczi
2017-08-31  8:21 ` [Qemu-devel] [PULL for-2.10 01/15] nvme: Fix get/set number of queues feature, again Stefan Hajnoczi
2017-08-31  8:21 ` [Qemu-devel] [PULL for-2.10 02/15] throttle: Fix wrong variable name in the header documentation Stefan Hajnoczi
2017-08-31  8:21 ` [Qemu-devel] [PULL for-2.10 03/15] throttle: Update the throttle_fix_bucket() documentation Stefan Hajnoczi
2017-08-31  8:21 ` [Qemu-devel] [PULL for-2.10 04/15] throttle: Make throttle_is_valid() a bit less verbose Stefan Hajnoczi
2017-08-31  8:22 ` [Qemu-devel] [PULL for-2.10 05/15] throttle: Remove throttle_fix_bucket() / throttle_unfix_bucket() Stefan Hajnoczi
2017-09-12 17:37   ` Peter Maydell
2017-08-31  8:22 ` [Qemu-devel] [PULL for-2.10 06/15] throttle: Make LeakyBucket.avg and LeakyBucket.max integer types Stefan Hajnoczi
2017-08-31  8:22 ` [Qemu-devel] [PULL for-2.10 07/15] throttle: Make burst_length 64bit and add range checks Stefan Hajnoczi
2017-08-31  8:22 ` [Qemu-devel] [PULL for-2.10 08/15] throttle: Test the valid range of config values Stefan Hajnoczi
2017-08-31  8:22 ` [Qemu-devel] [PULL for-2.10 09/15] oslib-posix: Print errors before aborting on qemu_alloc_stack() Stefan Hajnoczi
2017-08-31  8:22 ` [Qemu-devel] [PULL for-2.10 10/15] misc: Remove unused Error variables Stefan Hajnoczi
2017-08-31  8:22 ` [Qemu-devel] [PULL for-2.10 11/15] scripts: add argparse module for Python 2.6 compatibility Stefan Hajnoczi
2017-08-31  8:22 ` [Qemu-devel] [PULL for-2.10 12/15] docker.py: Python 2.6 argparse compatibility Stefan Hajnoczi
2017-08-31  8:22 ` [Qemu-devel] [PULL for-2.10 13/15] tests: migration/guestperf " Stefan Hajnoczi
2017-08-31  8:22 ` [Qemu-devel] [PULL for-2.10 14/15] qemu-doc: Add UUID support in initiator name Stefan Hajnoczi
2017-08-31  8:22 ` Stefan Hajnoczi [this message]
2017-08-31  8:37 ` [Qemu-devel] [PULL for-2.10 00/15] Block patches no-reply
2017-08-31 13:47 ` Eric Blake
2017-08-31 14:51 ` Peter Maydell

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170831082210.8362-16-stefanha@redhat.com \
    --to=stefanha@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.