dm-qcow2 heavily relies on main kwork do_qcow2_work() being sequential. All metadata change happens there and cannot run in parallel. Unfortunately that caps dm-qcow2 performance. But if the request does not require metadata changes we should be able to immediately dispatch it. process_one_qio() and handle_metadata() already know what to do: just add a fast flag to process_one_qio(), passed as avoid_metadata_changes to handle_metadata(), and return codes. While at it, just return in handle_metadata() instead of setting ret to 0 and jumping to if (ret < 0) label.
Also pass the flag to parse_metadata() and return to the slow path before it looks up R1/R2 pages. They are needed only by COW writes and cluster unmapping discards, which go to the slow path anyway. Besides the useless lookup, R1 is unstable outside main kwork: an R1 page read in parallel with relocate_refcount_table() may stay cached under a stale page id. Return to the slow path from parse_metadata() also for a write to an index changed in a dirty or under writeback L2 page, i.e. to a cluster whose allocation is not on disk yet. Such a write must be attached to md->wbd, or wait for the writeback end, so it is not acked before its L2 entry is written. Leave this to main kwork, so the fast path never refers to md->wbd. Also exclude compressed clusters as there are some complications with wpc_readers. https://virtuozzo.atlassian.net/browse/VSTOR-95494 Feature: dm-qcow2: block device over QCOW2 files driver Signed-off-by: Andrey Zhadchenko <[email protected]> --- v2: return to the slow path before R1/R2 lookup in parse_metadata(); QIO_NEEDS_MAIN_KWORK instead of magic 2; fixed the "avoid_metadata_changes() flag" typo v3: a write to an index changed in a dirty or under writeback L2 page goes to main kwork from parse_metadata(); process_one_qio() parameter renamed to fast; dropped redundant else drivers/md/dm-qcow2-map.c | 63 +++++++++++++++++++++++++++++++-------- 1 file changed, 50 insertions(+), 13 deletions(-) diff --git a/drivers/md/dm-qcow2-map.c b/drivers/md/dm-qcow2-map.c index 58b95d7a955cf..5e254c11ae7b8 100644 --- a/drivers/md/dm-qcow2-map.c +++ b/drivers/md/dm-qcow2-map.c @@ -2094,6 +2094,12 @@ static int qcow2_handle_r1r2_maps(struct qcow2 *qcow2, loff_t pos, struct qio ** return ret; } +/* + * Returned by parse_metadata() and handle_metadata() with + * @avoid_metadata_changes, when @qio must be processed by main kwork. + */ +#define QIO_NEEDS_MAIN_KWORK 2 + /* * This caches pages of allocated on disk md levels, which are * required for submission @qio, and checks they are stable. @@ -2101,11 +2107,17 @@ static int qcow2_handle_r1r2_maps(struct qcow2 *qcow2, loff_t pos, struct qio ** * Returns: negative in case of error, or 0 if success. * Special case: return 0 with zeroed @qio means @qio was deferred * till some event: reading of md page, end of writeback, etc. + * With @avoid_metadata_changes, return QIO_NEEDS_MAIN_KWORK if @qio + * needs R1/R2: outside main kwork R1 is unstable, see + * __handle_r1r2_maps(). The same for a write to an index changed in + * a dirty or under writeback L2 page: it must be attached to md->wbd + * or wait for the writeback, which is left to main kwork. */ static int parse_metadata(struct qcow2 *qcow2, struct qio **qio, - struct qcow2_map *map) + struct qcow2_map *map, bool avoid_metadata_changes) { bool write = op_is_write((*qio)->bi_op); + bool l2_in_flight = false; struct md_page *md; u64 pos; s64 ret; @@ -2148,10 +2160,15 @@ static int parse_metadata(struct qcow2 *qcow2, struct qio **qio, /* Find DATA cluster (from L2 page) */ pos = ret = parse_l2(qcow2, map, qio, write); + if (ret > 0 && write && avoid_metadata_changes) + l2_in_flight = dirty_or_writeback(qcow2, md, + map->l2.index_in_page + !!(qcow2->ext_l2)); unlock: spin_unlock_irq(&qcow2->md_pages_lock); if (ret <= 0) /* Err, delayed, DATA is not allocated, or zero read */ return ret; + if (l2_in_flight) + return QIO_NEEDS_MAIN_KWORK; map->data_clu_pos = pos; if (!write) @@ -2167,6 +2184,9 @@ static int parse_metadata(struct qcow2 *qcow2, struct qio **qio, if (!qio_may_modify_image(qcow2, *qio)) return 0; + if (avoid_metadata_changes) + return QIO_NEEDS_MAIN_KWORK; + /* Now refcounters table/block */ ret = qcow2_handle_r1r2_maps(qcow2, pos, qio, &map->r1, &map->r2, map->compressed); @@ -3716,24 +3736,31 @@ static int prepare_cluster_discard(struct qcow2 *qcow2, struct qio **qio, } static int handle_metadata(struct qcow2 *qcow2, struct qio **qio, - struct qcow2_map *map) + struct qcow2_map *map, bool avoid_metadata_changes) { bool write = op_is_write((*qio)->bi_op); int ret; - ret = parse_metadata(qcow2, qio, map); + ret = parse_metadata(qcow2, qio, map, avoid_metadata_changes); if (ret < 0 || !*qio) /* Error or postponed */ goto check_err; + if (ret == QIO_NEEDS_MAIN_KWORK) + return QIO_NEEDS_MAIN_KWORK; + + if (avoid_metadata_changes && map->compressed) + return QIO_NEEDS_MAIN_KWORK; ret = 1; if (unlikely(qcow2->backward_merge_in_process)) { + if (avoid_metadata_changes) + return QIO_NEEDS_MAIN_KWORK; /* Keep in mind the below may replace *qio */ ret = prepare_backward_merge(qcow2, qio, map, write); } else if (unlikely(fake_l1cow_qio(*qio)) && (!map->clu_is_cow || (map->level & L2_LEVEL))) { /* Nothing to COW or L1 is mapped exactly once */ qio_endio(*qio); - ret = 0; + return 0; } else if (unlikely(op_is_discard((*qio)->bi_op))) { /* * Discard carries no data and is advisory. If it does not @@ -3741,13 +3768,17 @@ static int handle_metadata(struct qcow2 *qcow2, struct qio **qio, * is not present, end early. */ if (qio_discard_updates_metadata(qcow2, *qio, map)) { + if (avoid_metadata_changes) + return QIO_NEEDS_MAIN_KWORK; ret = prepare_cluster_discard(qcow2, qio, map); } else if (!map->data_clu_alloced || map->clu_is_cow) { qio_endio(*qio); - ret = 0; + return 0; } } else if (write && (!qio_is_fully_alloced(qcow2, *qio, map) || map->clu_is_cow)) { + if (avoid_metadata_changes) + return QIO_NEEDS_MAIN_KWORK; if (map->clu_is_cow) { /* COW to compressed or shared with snapshot cluster */ ret = prepare_l1l2_replace(qcow2, *qio, map); @@ -3859,13 +3890,17 @@ static void process_read_qio(struct qcow2 *qcow2, struct qio *qio, qio_endio(qio); } -static void process_one_qio(struct qcow2 *qcow2, struct qio *qio) +static int process_one_qio(struct qcow2 *qcow2, struct qio *qio, bool fast) { struct qcow2_map map = { .qcow2 = qcow2, }; bool write; + int ret; - if (!handle_metadata(qcow2, &qio, &map)) - return; + ret = handle_metadata(qcow2, &qio, &map, fast); + if (!ret) + return 0; + if (ret == QIO_NEEDS_MAIN_KWORK) + return 1; /* * Merge machinery makes out of order merges for present @@ -3877,7 +3912,7 @@ static void process_one_qio(struct qcow2 *qcow2, struct qio *qio) if (unlikely(qcow2->backward_merge_in_process) && (fake_merge_qio(qio) || qcow2_file_is_writable(qcow2))) { submit_top_delta_read(&map, qio); - return; + return 0; } write = op_is_write(qio->bi_op); @@ -3885,13 +3920,13 @@ static void process_one_qio(struct qcow2 *qcow2, struct qio *qio) /* Process discard early. Don't do COW just to discard */ if (unlikely(op_is_discard(qio->bi_op))) { issue_discard(&map, qio); - return; + return 0; } if (unlikely(map.compressed)) { /* Compressed qio never uses sub-clus */ submit_read_compressed(&map, qio, write); - return; + return 0; } if (!write) { @@ -3904,6 +3939,8 @@ static void process_one_qio(struct qcow2 *qcow2, struct qio *qio) else perform_rw_mapped(&map, qio); } + + return 0; } static struct bio_vec *create_bvec_from_rq(struct request *rq) @@ -4006,7 +4043,7 @@ static void process_deferred_qios(struct qcow2 *qcow2, struct list_head *qios) continue; } - process_one_qio(qcow2, qio); + process_one_qio(qcow2, qio, false); } } @@ -4946,7 +4983,7 @@ static int qcow2_llseek_hole_qio(struct qio *qio, int whence, loff_t *result) map.qcow2 = qio->qcow2; qptr = qio; - ret = parse_metadata(qio->qcow2, &qptr, &map); + ret = parse_metadata(qio->qcow2, &qptr, &map, false); if (ret < 0) break; if (qptr == NULL) { -- 2.43.5 _______________________________________________ Devel mailing list [email protected] https://lists.openvz.org/mailman/listinfo/devel
