Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Low] dm-qcow2: commit message omits sleeping steps run serially in fast-path worker --
Commit 6cf4c03ea626fb7fe01c889ccd2361694ab98cf0 Author: Andrey Zhadchenko <[email protected]> dm-qcow2: process fast path qios in per-cpu batches on a workqueue This replaces the per-qio work item on the fast path with a per-CPU llist and a single per-CPU work item. The worker drains one batch per run and requeues itself if more qios arrived meanwhile. > The fast path is now serial per submitting CPU: qios queued from one > CPU are processed one by one by its work, while qios from different > CPUs are still processed in parallel. This is fine, since a fast path > qio is only mapped and submitted as direct AIO: O_DIRECT is enforced > for every delta. The exception is a partial discard, which punches > the hole synchronously in the worker, so the qios queued after it on > that CPU wait. [Severity: Low] This isn't a bug in the code, but the commit message looks incomplete. It says a fast path qio is "only mapped and submitted as direct AIO" and names only a partial discard as an exception. Is that accurate? qcow2_fast_work() runs the whole batch serially: > + node = llist_del_all(&fc->qios); > + node = llist_reverse_order(node); > + llist_for_each_entry_safe(qio, next, node, fast_link) > + do_qio_work(qio); For QIO_STATE_START, do_qio_work() calls prepare_one_embedded_qio(). For multi-bio requests this reaches create_bvec_from_rq(), which does: bvec = kvmalloc_array(nr_bvec, sizeof(struct bio_vec), GFP_NOIO); The qio split path also allocates through qcow2_alloc_qio(), which calls mempool_alloc(GFP_NOIO). Both can sleep under memory pressure, and while they do, every qio queued behind on that CPU is delayed. The direct AIO submission via qcow2_call_rw_iter() can also block on the inode lock. The partial discard case reaches issue_discard(), which calls qcow2_punch_hole() synchronously, and the commit message already mentions this. Could the commit message list these other sleeping steps, or state that only the discard case is the notable one and explain why the others are acceptable? -- Sashiko AI review ยท https://sashiko.vzint.dev/#/patchset/[email protected]?part=6 _______________________________________________ Devel mailing list [email protected] https://lists.openvz.org/mailman/listinfo/devel
