On 11/9/26 16:42, Konstantin Khorenko wrote:
I won't apply this version, please send v12 with the changes below.
On 11/9/26 15:01, Alexander Ivanov wrote:
spin_unlock_irqrestore(&qcow2->md_pages_lock, flags);
+ list_for_each_entry(qio, &end_list, link)
+ qio->ext->cleanup_mask &= ~FREE_ALLOCATED_CLU;
qcow2_dispatch_qios(qcow2, NULL, &wait_list);
Item 1 of the cover letter reply: queue the L1-allocation qio on
l1.md->wbd->dependent_list instead of l1.md->wait_list.
Why: that qio does not reach end_list. It waits on l1.md->wait_list,
and on a failed L1 writeback clear_writeback_status() sends it back to
work. The reverted L1 entry is 0 again, so it allocates another
L2-table cluster, and the L1 page is written again in the same worker
pass. Nothing slows this down: with a persistent L1 write error every
round leaks one more cluster and grows the file. Before this patch the
same loop freed the cluster and reused it.
On success a dependent qio goes back to work the same way as a waiter,
so moving it costs nothing there. Today only the qios added in
process_indexes_write() sit on dependent_list, and they all have ext.
A plain write that allocates an L2 table may have none, so check
qio->ext in the new loop.
The move to dependent_list is already done in the v12 version,
"dm-qcow2: fail the L2 table allocation request on L1 writeback
error". Added the qio->ext check to this patch.
Keep the refcount of these clusters instead. If L1/L2 on disk points to
such a cluster, the image stays consistent; otherwise the cluster is
leaked, which check repairs by freeing it. So the image needs no check
and resume needs no refusal.
Item 2 of the cover letter reply: say what is leaked, one cluster per
failed L1/L2 write of a request.
Why: your v10 answer bounded the leak by "refcounts reach the disk
before L1/L2". In this driver it is the other way round: R1/R2 pages
wait on slow_wb_batch_list for the timer (see the comment above
mark_cluster_used()), while L1/L2 pages are written in the same worker
pass. And an L1 update has no data write that could fail first.
Agreed, the v10 bound was wrong: R1/R2 go out after L1/L2 here.
Reworded item 2 of the cover letter and the commit message of
patch 9: one cluster leaks per failed L1/L2 write of a request, the
L2 table for an L1 write and the data cluster for an L2 write.
_______________________________________________
Devel mailing list
[email protected]
https://lists.openvz.org/mailman/listinfo/devel
