From nobody Sun Oct 5 19:22:57 2025 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) client-ip=208.118.235.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org; dmarc=fail(p=none dis=none) header.from=redhat.com Return-Path: Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) by mx.zohomail.com with SMTPS id 1519165612546942.6571706923141; Tue, 20 Feb 2018 14:26:52 -0800 (PST) Received: from localhost ([::1]:58066 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1eoGN9-0004yQ-Ot for importer@patchew.org; Tue, 20 Feb 2018 17:26:51 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:56295) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1eoGLe-00046S-Ek for qemu-devel@nongnu.org; Tue, 20 Feb 2018 17:25:19 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1eoGLd-0005Gd-BO for qemu-devel@nongnu.org; Tue, 20 Feb 2018 17:25:18 -0500 Received: from mx3-rdu2.redhat.com ([66.187.233.73]:52268 helo=mx1.redhat.com) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1eoGLa-0005Fj-8C; Tue, 20 Feb 2018 17:25:14 -0500 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.rdu2.redhat.com [10.11.54.4]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id D14774023112; Tue, 20 Feb 2018 22:25:13 +0000 (UTC) Received: from red.redhat.com (ovpn-122-122.rdu2.redhat.com [10.10.122.122]) by smtp.corp.redhat.com (Postfix) with ESMTP id 595E12024CA7; Tue, 20 Feb 2018 22:25:13 +0000 (UTC) From: Eric Blake To: qemu-devel@nongnu.org Date: Tue, 20 Feb 2018 16:24:59 -0600 Message-Id: <20180220222459.8461-3-eblake@redhat.com> In-Reply-To: <20180220222459.8461-1-eblake@redhat.com> References: <20180220222459.8461-1-eblake@redhat.com> X-Scanned-By: MIMEDefang 2.78 on 10.11.54.4 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.6]); Tue, 20 Feb 2018 22:25:13 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.6]); Tue, 20 Feb 2018 22:25:13 +0000 (UTC) for IP:'10.11.54.4' DOMAIN:'int-mx04.intmail.prod.int.rdu2.redhat.com' HELO:'smtp.corp.redhat.com' FROM:'eblake@redhat.com' RCPT:'' X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 66.187.233.73 Subject: [Qemu-devel] [PATCH 2/2] qcow2: Avoid memory over-allocation on compressed images X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Kevin Wolf , berto@igalia.com, qemu-block@nongnu.org, mreitz@redhat.com Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail: RSF_0 Z_629925259 SPT_0 Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" When reading a compressed image, we were allocating s->cluster_data to 32*cluster_size + 512 (possibly over 64 megabytes, for an image with 2M clusters). Let's check out the history: Back when qcow2 was first written, we used s->cluster_data for everything, including copy_sectors() and encryption, where we want to operate on more than one cluster at once. Obviously, at that point, the buffer had to be aligned for other users, even though compression itself doesn't require any alignment. But commit 1b9f1491 (v1.1!) changed things to allocate parallel buffers on demand rather than sharing a single buffer, for encryption and COW, leaving compression as the final client of s->cluster_data. That use was still preserved, because if a single compressed cluster is read more than once, we reuse the cache instead of decompressing it a second time (I'm not sure how often this optimization actually fires, or if it penalizes us from being able to decompress multiple clusters in parallel even though we can now decrypt clusters in parallel; the XXX comment in qcow2_co_preadv for QCOW2_CLUSTER_COMPRESSED is telling). Much later, in commit de82815d (v2.2), we noticed that a 64M allocation is prone to failure, so we switched over to a graceful memory allocation error message. But note that elsewhere in the code, we do g_malloc(2 * cluster_size) without ever checking for failure. Then even later, in 3e4c7052 (2.11), we realized that allocating a large buffer up front for every qcow2 image is expensive, and switched to lazy allocation only for images that actually had compressed clusters. But in the process, we never even bothered to check whether what we were allocating still made sense in its new context! So, it's time to cut back on the waste. A compressed cluster will NEVER occupy more than an uncompressed cluster (okay, gzip DOES document that because the compression stream adds metadata, and because of the pigeonhole principle, there are worst case scenarios where attempts to compress will actually inflate an image - but in those cases, we would just write the cluster uncompressed instead of inflating it). And as that is a smaller amount of memory, we can get by with the simpler g_malloc. Signed-off-by: Eric Blake --- block/qcow2-cluster.c | 12 +++--------- block/qcow2.c | 2 +- 2 files changed, 4 insertions(+), 10 deletions(-) diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c index 85be7d5e340..8c4b26ceaf2 100644 --- a/block/qcow2-cluster.c +++ b/block/qcow2-cluster.c @@ -1603,15 +1603,9 @@ int qcow2_decompress_cluster(BlockDriverState *bs, u= int64_t cluster_offset) * are freed in .bdrv_close(). */ if (!s->cluster_data) { - /* one more sector for decompressed data alignment */ - s->cluster_data =3D 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 =3D g_malloc(s->cluster_size); + assert(!s->cluster_cache); + s->cluster_data =3D g_try_malloc(s->cluster_size); + s->cluster_cache =3D g_try_malloc(s->cluster_size); } BLKDBG_EVENT(bs->file, BLKDBG_READ_COMPRESSED); diff --git a/block/qcow2.c b/block/qcow2.c index 288b5299d80..6ad3436e0e5 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -2103,7 +2103,7 @@ static void qcow2_close(BlockDriverState *bs) g_free(s->image_backing_format); g_free(s->cluster_cache); - qemu_vfree(s->cluster_data); + g_free(s->cluster_data); qcow2_refcount_close(bs); qcow2_free_snapshots(bs); } --=20 2.14.3