[Devel] [PATCH VZ10 v2 04/10] drivers/md/dm-qcow2: never trigger COW or allocation on discard

[Andrey Zhadchenko]

In handle_metadata() the partial cluster discard fallback is checked
only after the COW branches. As a result a discard, whose range
touches a shared (snapshot or compressed) cluster or crosses an
unmapped unit border over a backing file, is routed into the data COW
machinery. COW then tries to submit the discard payload as a data
write, but discard qios carry no bvec array: the iov_iter is built
from garbage, which triggers the WARN_ON() in __submit_rw_mapped()
and writes random kernel memory into the COW'ed cluster.

Check for discard before the COW branches: discard is advisory and
carries no data, so it must never COW or allocate. End discard
early while handling metadata if possible. Otherwise do discard
operations before COWs.

Feature: dm-qcow2: block device over QCOW2 files driver
https://virtuozzo.atlassian.net/browse/VSTOR-139406
Signed-off-by: Andrey Zhadchenko <andrey.zhadchenko at virtuozzo.com>
---
 drivers/md/dm-qcow2-map.c | 28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/drivers/md/dm-qcow2-map.c b/drivers/md/dm-qcow2-map.c
index 8d2c4e78a4c55..dcc65735881d4 100644
--- a/drivers/md/dm-qcow2-map.c
+++ b/drivers/md/dm-qcow2-map.c
@@ -3431,9 +3431,18 @@ static int handle_metadata(struct qcow2 *qcow2, struct qio **qio,
 		/* Nothing to COW or L1 is mapped exactly once */
 		qio_endio(*qio);
 		ret = 0;
-	} else if (unlikely(op_is_discard((*qio)->bi_op)) &&
-		   qio_discard_updates_metadata(qcow2, *qio, map)) {
-		ret = prepare_cluster_discard(qcow2, qio, map);
+	} else if (unlikely(op_is_discard((*qio)->bi_op))) {
+		/*
+		 * Discard carries no data and advisory. If it does not
+		 * trigger metadata changes or to-be-discarded cluster
+		 * is not present, end early.
+		 */
+		if (qio_discard_updates_metadata(qcow2, *qio, map)) {
+			ret = prepare_cluster_discard(qcow2, qio, map);
+		} else if (!map->data_clu_alloced || map->clu_is_cow) {
+			qio_endio(*qio);
+			ret = 0;
+		}
 	} else if (write &&
 		   (!qio_is_fully_alloced(qcow2, *qio, map) || map->clu_is_cow)) {
 		if (map->clu_is_cow) {
@@ -3449,13 +3458,6 @@ static int handle_metadata(struct qcow2 *qcow2, struct qio **qio,
 			 */
 			map->backing_file_cow = true;
 			ret = prepare_l1l2_replace(qcow2, *qio, map);
-		} else if (unlikely(op_is_discard((*qio)->bi_op) &&
-				    (map->level & L2_LEVEL))) {
-			if (!map->data_clu_alloced) {
-				qio_endio(*qio);
-				ret = 0;
-			}
-			/* Otherwise issue_discard(). */
 		} else {
 			/* Wants L1 or L2 entry allocation */
 			ret = prepare_l1l2_allocation(qcow2, *qio, map);
@@ -3569,8 +3571,8 @@ static void process_one_qio(struct qcow2 *qcow2, struct qio *qio)
 
 	write = op_is_write(qio->bi_op);
 
-	/* Discard with prepared metadata update, see prepare_cluster_discard() */
-	if (unlikely(qio->flags & QIO_IS_DISCARD_FL)) {
+	/* Process discard early. Don't do COW just to discard */
+	if (unlikely(op_is_discard(qio->bi_op))) {
 		issue_discard(&map, qio);
 		return;
 	}
@@ -3588,8 +3590,6 @@ static void process_one_qio(struct qcow2 *qcow2, struct qio *qio)
 			submit_read_whole_cow_clu(&map, qio);
 		else if (unlikely(map.clu_is_cow || map.backing_file_cow))
 			submit_read_sliced_cow_clu(&map, qio);
-		else if (unlikely(op_is_discard(qio->bi_op)))
-			issue_discard(&map, qio);
 		else
 			perform_rw_mapped(&map, qio);
 	}
-- 
2.43.5