perform_rw_mapped() reads md->status and md->wbd without md_pages_lock and counts the write in md writers, relying on dm-qcow2 sequential nature: main kwork parses a qio and submits it in one pass. Outside main kwork an L1/L2 page changes only when its writeback ends, and parse_l2() already delays writes to the changed indexes of a page under writeback.
This does not hold for the per-qio fast path, which parses L2 and submits in parallel with main kwork. The fast path takes only writes to fully allocated clusters mapped exactly once, whose indexes are neither locked nor changed in a dirty or under writeback L2 page. In between, main kwork may still discard or zero the same range, which is a valid race with the write, or allocate other subclusters of the cluster, which does not affect the write. But a discard unmapping the cluster lets it be reused, and a write landing after that corrupts the new owner. The discard waits for writes counted in md writers, but the counter is valid only if it is increased from main kwork or under md_pages_lock. So count a fast path write in parse_metadata() under the same md_pages_lock hold, which checks the L2 entry. A discard switches the writer generation after it clears the entry under the lock, so either the fast path sees the entry cleared, or the discard waits for the write. Drop the count, if the qio goes to main kwork afterwards. A partial discard on the fast path punches a hole in the cluster, so it is counted the same way and drops the count after the punch. A discard of a shared cluster punches nothing and is not counted. Add @fast parameter to perform_rw_mapped(). With it, skip the md->wbd attach, which the fast path never needs, and the writers increase. https://virtuozzo.atlassian.net/browse/VSTOR-95494 Feature: dm-qcow2: block device over QCOW2 files driver Signed-off-by: Andrey Zhadchenko <[email protected]> --- v3: new patch, replaces "wait for L2 writeback in perform_rw_mapped()": a fast path write or partial discard is counted in md writers under the lock which checks its L2 entry, so a discard unmapping the cluster waits for it before the cluster is freed drivers/md/dm-qcow2-map.c | 37 ++++++++++++++++++++++++------------- 1 file changed, 24 insertions(+), 13 deletions(-) diff --git a/drivers/md/dm-qcow2-map.c b/drivers/md/dm-qcow2-map.c index 5e254c11ae7b8..b9daa258285e6 100644 --- a/drivers/md/dm-qcow2-map.c +++ b/drivers/md/dm-qcow2-map.c @@ -2160,9 +2160,14 @@ 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) + if (ret > 0 && write && avoid_metadata_changes) { l2_in_flight = dirty_or_writeback(qcow2, md, map->l2.index_in_page + !!(qcow2->ext_l2)); + /* A discard is counted only if it is going to punch a hole */ + if (!l2_in_flight && + (!op_is_discard((*qio)->bi_op) || !map->clu_is_cow)) + inc_writers(md, *qio); + } unlock: spin_unlock_irq(&qcow2->md_pages_lock); if (ret <= 0) /* Err, delayed, DATA is not allocated, or zero read */ @@ -3089,7 +3094,7 @@ static void data_rw_complete(struct qio *qio) } } -static void perform_rw_mapped(struct qcow2_map *map, struct qio *qio) +static void perform_rw_mapped(struct qcow2_map *map, struct qio *qio, bool fast) { struct qcow2 *qcow2 = map->qcow2; struct md_page *md = map->l2.md; @@ -3112,15 +3117,16 @@ static void perform_rw_mapped(struct qcow2_map *map, struct qio *qio) * so lock is not needed for MD_DIRTY/changed_indexes visibility. */ index_in_page = map->l2.index_in_page + !!(qcow2->ext_l2); - if (rw == WRITE && (md->status & MD_DIRTY) && - test_bit(index_in_page, md->wbd->changed_indexes)) { - spin_lock_irqsave(&qcow2->md_pages_lock, flags); - md->wbd->nr_submitted++; - qio->data = md->wbd; - spin_unlock_irqrestore(&qcow2->md_pages_lock, flags); - } - if (rw == WRITE) + if (rw == WRITE && !fast) { + if ((md->status & MD_DIRTY) && + test_bit(index_in_page, md->wbd->changed_indexes)) { + spin_lock_irqsave(&qcow2->md_pages_lock, flags); + md->wbd->nr_submitted++; + qio->data = md->wbd; + spin_unlock_irqrestore(&qcow2->md_pages_lock, flags); + } inc_writers(md, qio); + } map_and_submit_rw(qcow2, map->data_clu_pos, qio); } @@ -3608,6 +3614,8 @@ static void issue_discard(struct qcow2_map *map, struct qio *qio) if (!map->clu_is_cow) { pos = bio_sector_to_file_pos(qcow2, qio, map); ret = qcow2_punch_hole(qcow2->file, pos, qio->bi_iter.bi_size); + if (qio->writer_md) + dec_writers(qio); if (ret) { qio->bi_status = errno_to_blk_status(ret); @@ -3878,7 +3886,7 @@ static void process_read_qio(struct qcow2 *qcow2, struct qio *qio, qcow2_dispatch_qios(qio->qcow2, qio, NULL); } else { /* Mapped */ - perform_rw_mapped(map, qio); + perform_rw_mapped(map, qio, false); } qio = split; @@ -3899,8 +3907,11 @@ static int process_one_qio(struct qcow2 *qcow2, struct qio *qio, bool fast) ret = handle_metadata(qcow2, &qio, &map, fast); if (!ret) return 0; - if (ret == QIO_NEEDS_MAIN_KWORK) + if (ret == QIO_NEEDS_MAIN_KWORK) { + if (qio->writer_md) + dec_writers(qio); return 1; + } /* * Merge machinery makes out of order merges for present @@ -3937,7 +3948,7 @@ static int process_one_qio(struct qcow2 *qcow2, struct qio *qio, bool fast) else if (unlikely(map.clu_is_cow || map.backing_file_cow)) submit_read_sliced_cow_clu(&map, qio); else - perform_rw_mapped(&map, qio); + perform_rw_mapped(&map, qio, fast); } return 0; -- 2.43.5 _______________________________________________ Devel mailing list [email protected] https://lists.openvz.org/mailman/listinfo/devel
