perform_rw_mapped() reads md->status and md->wbd without
md_pages_lock, relying on dm-qcow2 sequential nature: L1/L2 pages
become dirty, go to writeback and drop md->wbd from main kwork only.
Expand md_pages_lock to also cover the check, so it can be called
from the per-qio fast path in parallel with main kwork.
The lock alone is not enough. parse_l2() does not delay a write to
a changed index of a DIRTY L2 page, as perform_rw_mapped() is going
to attach the write to md->wbd. Main kwork does parse, submit and the
DIRTY -> WRITEBACK switch in one do_qcow2_work() pass, so the page
is still DIRTY there. The fast path parses and submits under separate
lock holds, and main kwork may switch the page to WRITEBACK in
between:
main kwork fast worker
allocates cluster X,
L2 page DIRTY
parse_l2(): write to X, page DIRTY,
not delayed, unlock
submit_metadata_writeback():
DIRTY -> WRITEBACK
perform_rw_mapped(): no MD_DIRTY,
qio->data = NULL, submit
The write is then acked as soon as its data is written, before the
L2 entry mapping it is on disk. A FLUSH after it finds
QLIST_COMPLETED_WB empty and only calls vfs_fsync(), so it does not
wait for the L2 write in flight, and a crash loses a write that was
acked and flushed. If the L2 write fails instead, the entry is
reverted and X freed while the write already returned success.
So, when the page is under writeback and the index is changed, park
the qio on md->wait_list, as parse_l2() does. It is redispatched to
main kwork when the writeback ends and parsed again. Do not attach it
to the wbd under writeback: after a failed writeback complete_wbd()
reverts the indexes and frees the wbd.
This changes nothing for main kwork, where parse_l2() has already
delayed such writes in the same pass.
https://virtuozzo.atlassian.net/browse/VSTOR-95494
Feature: dm-qcow2: block device over QCOW2 files driver
Signed-off-by: Andrey Zhadchenko <[email protected]>
---
v2: renamed from "check MD_DIRTY and md->wbd under lock in
perform_rw_mapped()"; park a write on md->wait_list if its L2 page
went to writeback after parse_l2(), instead of acking it before its
L2 entry is on disk; commit message describes the race
drivers/md/dm-qcow2-map.c | 24 +++++++++++++++++++-----
1 file changed, 19 insertions(+), 5 deletions(-)
diff --git a/drivers/md/dm-qcow2-map.c b/drivers/md/dm-qcow2-map.c
index 21d1c8975af99..49dc3e8119e76 100644
--- a/drivers/md/dm-qcow2-map.c
+++ b/drivers/md/dm-qcow2-map.c
@@ -2912,14 +2912,28 @@ static void perform_rw_mapped(struct qcow2_map *map,
struct qio *qio)
* wait for md writeback completion.
*
* L1/L2 pages become set and unset dirty from main work only,
- * so lock is not needed for MD_DIRTY/changed_indexes visibility.
+ * but this may be called from per-qio fast path in parallel
+ * with main work submitting the writeback and with its completion
+ * dropping md->wbd.
+ *
+ * So md may have gone DIRTY -> WRITEBACK after parse_l2() decided
+ * the qio need not wait. Then the qio must wait for writeback end
+ * the way parse_l2() makes it. It can't be attached to wbd: after
+ * a failed writeback, complete_wbd() reverts the indexes and frees
+ * the wbd without looking at qios attached in the meantime.
*/
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)) {
+ if (rw == WRITE) {
spin_lock_irqsave(&qcow2->md_pages_lock, flags);
- md->wbd->nr_submitted++;
- qio->data = md->wbd;
+ if (__delay_if_writeback(qcow2, md, index_in_page, &qio,
false)) {
+ spin_unlock_irqrestore(&qcow2->md_pages_lock, flags);
+ return;
+ }
+ if ((md->status & MD_DIRTY) &&
+ test_bit(index_in_page, md->wbd->changed_indexes)) {
+ md->wbd->nr_submitted++;
+ qio->data = md->wbd;
+ }
spin_unlock_irqrestore(&qcow2->md_pages_lock, flags);
}
--
2.43.5
_______________________________________________
Devel mailing list
[email protected]
https://lists.openvz.org/mailman/listinfo/devel
