[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-block] [PULL 52/58] qcow2: Optimize zero_single_l2() to minimize L
From: |
Kevin Wolf |
Subject: |
[Qemu-block] [PULL 52/58] qcow2: Optimize zero_single_l2() to minimize L2 churn |
Date: |
Thu, 11 May 2017 16:32:55 +0200 |
From: Eric Blake <address@hidden>
Similar to discard_single_l2(), we should try to avoid dirtying
the L2 cache when the cluster we are changing already has the
right characteristics.
Note that by the time we get to zero_single_l2(), BDRV_REQ_MAY_UNMAP
is a requirement to unallocate a cluster (this is because the block
layer clears that flag if discard.* flags during open requested that
we never punch holes - see the conversation around commit 170f4b2e,
https://lists.gnu.org/archive/html/qemu-devel/2016-09/msg07306.html).
Therefore, this patch can only reuse a zero cluster as-is if either
unmapping is not requested, or if the zero cluster was not associated
with an allocation.
Technically, there are some cases where an unallocated cluster
already reads as all zeroes (namely, when there is no backing file
[easy: check bs->backing], or when the backing file also reads as
zeroes [harder: we can't check bdrv_get_block_status since we are
already holding the lock]), where the guest would not immediately see
a difference if we left that cluster unallocated. But if the user
did not request unmapping, leaving an unallocated cluster is wrong;
and even if the user DID request unmapping, keeping a cluster
unallocated risks a subtle semantic change of guest-visible contents
if a backing file is later added, and it is not worth auditing
whether all internal uses such as mirror properly avoid an unmap
request. Thus, this patch is intentionally limited to just clusters
that are already marked as zero.
Signed-off-by: Eric Blake <address@hidden>
Reviewed-by: Max Reitz <address@hidden>
Message-id: address@hidden
Signed-off-by: Max Reitz <address@hidden>
---
block/qcow2-cluster.c | 15 +++++++++++++--
1 file changed, 13 insertions(+), 2 deletions(-)
diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index 558c239..e2c5759 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -1601,6 +1601,7 @@ static int zero_single_l2(BlockDriverState *bs, uint64_t
offset,
int l2_index;
int ret;
int i;
+ bool unmap = !!(flags & BDRV_REQ_MAY_UNMAP);
ret = get_cluster_table(bs, offset, &l2_table, &l2_index);
if (ret < 0) {
@@ -1613,12 +1614,22 @@ static int zero_single_l2(BlockDriverState *bs,
uint64_t offset,
for (i = 0; i < nb_clusters; i++) {
uint64_t old_offset;
+ QCow2ClusterType cluster_type;
old_offset = be64_to_cpu(l2_table[l2_index + i]);
- /* Update L2 entries */
+ /*
+ * Minimize L2 changes if the cluster already reads back as
+ * zeroes with correct allocation.
+ */
+ cluster_type = qcow2_get_cluster_type(old_offset);
+ if (cluster_type == QCOW2_CLUSTER_ZERO_PLAIN ||
+ (cluster_type == QCOW2_CLUSTER_ZERO_ALLOC && !unmap)) {
+ continue;
+ }
+
qcow2_cache_entry_mark_dirty(bs, s->l2_table_cache, l2_table);
- if (old_offset & QCOW_OFLAG_COMPRESSED || flags & BDRV_REQ_MAY_UNMAP) {
+ if (cluster_type == QCOW2_CLUSTER_COMPRESSED || unmap) {
l2_table[l2_index + i] = cpu_to_be64(QCOW_OFLAG_ZERO);
qcow2_free_any_clusters(bs, old_offset, 1, QCOW2_DISCARD_REQUEST);
} else {
--
1.8.3.1
- [Qemu-block] [PULL 42/58] blkdebug: Add pass-through write_zero and discard support, (continued)
- [Qemu-block] [PULL 42/58] blkdebug: Add pass-through write_zero and discard support, Kevin Wolf, 2017/05/11
- [Qemu-block] [PULL 44/58] blkdebug: Add ability to override unmap geometries, Kevin Wolf, 2017/05/11
- [Qemu-block] [PULL 43/58] blkdebug: Simplify override logic, Kevin Wolf, 2017/05/11
- [Qemu-block] [PULL 47/58] qcow2: Use consistent switch indentation, Kevin Wolf, 2017/05/11
- [Qemu-block] [PULL 46/58] qcow2: Nicer variable names in qcow2_update_snapshot_refcount(), Kevin Wolf, 2017/05/11
- [Qemu-block] [PULL 45/58] tests: Add coverage for recent block geometry fixes, Kevin Wolf, 2017/05/11
- [Qemu-block] [PULL 48/58] block: Update comments on BDRV_BLOCK_* meanings, Kevin Wolf, 2017/05/11
- [Qemu-block] [PULL 49/58] qcow2: Correctly report status of preallocated zero clusters, Kevin Wolf, 2017/05/11
- [Qemu-block] [PULL 50/58] qcow2: Name typedef for cluster type, Kevin Wolf, 2017/05/11
- [Qemu-block] [PULL 51/58] qcow2: Make distinction between zero cluster types obvious, Kevin Wolf, 2017/05/11
- [Qemu-block] [PULL 52/58] qcow2: Optimize zero_single_l2() to minimize L2 churn,
Kevin Wolf <=
- [Qemu-block] [PULL 53/58] iotests: Improve _filter_qemu_img_map, Kevin Wolf, 2017/05/11
- [Qemu-block] [PULL 54/58] iotests: Add test 179 to cover write zeroes with unmap, Kevin Wolf, 2017/05/11
- [Qemu-block] [PULL 56/58] qcow2: Assert that cluster operations are aligned, Kevin Wolf, 2017/05/11
- [Qemu-block] [PULL 57/58] qcow2: Discard/zero clusters by byte count, Kevin Wolf, 2017/05/11
- [Qemu-block] [PULL 55/58] qcow2: Optimize write zero of unaligned tail cluster, Kevin Wolf, 2017/05/11
- [Qemu-block] [PULL 58/58] MAINTAINERS: Add qemu-progress to the block layer, Kevin Wolf, 2017/05/11
- Re: [Qemu-block] [PULL 00/58] Block layer patches, Stefan Hajnoczi, 2017/05/12