add patch to avoid potential deadlock with trim for IDE/SATA and draining
In particular, the deadlock can occur, together with unlucky timing between the QEMU threads, when the guest is issuing trim requests during the start of a backup operation. Signed-off-by: Fiona Ebner <f.ebner@proxmox.com> [ T: resolve trivial merge conflict in series file ] Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
This commit is contained in:
parent
10691e04e9
commit
58659169de
78
debian/patches/extra/0011-ide-avoid-potential-deadlock-when-draining-during-tr.patch
vendored
Normal file
78
debian/patches/extra/0011-ide-avoid-potential-deadlock-when-draining-during-tr.patch
vendored
Normal file
@ -0,0 +1,78 @@
|
|||||||
|
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
|
||||||
|
From: Fiona Ebner <f.ebner@proxmox.com>
|
||||||
|
Date: Tue, 7 Mar 2023 15:03:02 +0100
|
||||||
|
Subject: [PATCH] ide: avoid potential deadlock when draining during trim
|
||||||
|
MIME-Version: 1.0
|
||||||
|
Content-Type: text/plain; charset=UTF-8
|
||||||
|
Content-Transfer-Encoding: 8bit
|
||||||
|
|
||||||
|
The deadlock can happen as follows:
|
||||||
|
1. ide_issue_trim is called, and increments the in_flight counter.
|
||||||
|
2. ide_issue_trim_cb calls blk_aio_pdiscard.
|
||||||
|
3. Somebody else starts draining (e.g. backup to insert the cbw node).
|
||||||
|
4. ide_issue_trim_cb is called as the completion callback for
|
||||||
|
blk_aio_pdiscard.
|
||||||
|
5. ide_issue_trim_cb issues yet another blk_aio_pdiscard request.
|
||||||
|
6. The request is added to the wait queue via blk_wait_while_drained,
|
||||||
|
because draining has been started.
|
||||||
|
7. Nobody ever decrements the in_flight counter and draining can't
|
||||||
|
finish. This would be done by ide_trim_bh_cb, which is called after
|
||||||
|
ide_issue_trim_cb has issued its last request, but
|
||||||
|
ide_issue_trim_cb is not called anymore, because it's the
|
||||||
|
completion callback of blk_aio_pdiscard, which waits on draining.
|
||||||
|
|
||||||
|
Quoting Hanna Czenczek:
|
||||||
|
> The point of 7e5cdb345f was that we need any in-flight count to
|
||||||
|
> accompany a set s->bus->dma->aiocb. While blk_aio_pdiscard() is
|
||||||
|
> happening, we don’t necessarily need another count. But we do need
|
||||||
|
> it while there is no blk_aio_pdiscard().
|
||||||
|
> ide_issue_trim_cb() returns in two cases (and, recursively through
|
||||||
|
> its callers, leaves s->bus->dma->aiocb set):
|
||||||
|
> 1. After calling blk_aio_pdiscard(), which will keep an in-flight
|
||||||
|
> count,
|
||||||
|
> 2. After calling replay_bh_schedule_event() (i.e.
|
||||||
|
> qemu_bh_schedule()), which does not keep an in-flight count.
|
||||||
|
|
||||||
|
Thus, even after moving the blk_inc_in_flight to above the
|
||||||
|
replay_bh_schedule_event call, the invariant "ide_issue_trim_cb
|
||||||
|
returns with an accompanying in-flight count" is still satisfied.
|
||||||
|
|
||||||
|
Fixes: 7e5cdb345f ("ide: Increment BB in-flight counter for TRIM BH")
|
||||||
|
Suggested-by: Hanna Czenczek <hreitz@redhat.com>
|
||||||
|
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
|
||||||
|
---
|
||||||
|
hw/ide/core.c | 7 +++----
|
||||||
|
1 file changed, 3 insertions(+), 4 deletions(-)
|
||||||
|
|
||||||
|
diff --git a/hw/ide/core.c b/hw/ide/core.c
|
||||||
|
index 39afdc0006..6474522bc9 100644
|
||||||
|
--- a/hw/ide/core.c
|
||||||
|
+++ b/hw/ide/core.c
|
||||||
|
@@ -443,7 +443,7 @@ static void ide_trim_bh_cb(void *opaque)
|
||||||
|
iocb->bh = NULL;
|
||||||
|
qemu_aio_unref(iocb);
|
||||||
|
|
||||||
|
- /* Paired with an increment in ide_issue_trim() */
|
||||||
|
+ /* Paired with an increment in ide_issue_trim_cb() */
|
||||||
|
blk_dec_in_flight(blk);
|
||||||
|
}
|
||||||
|
|
||||||
|
@@ -503,6 +503,8 @@ static void ide_issue_trim_cb(void *opaque, int ret)
|
||||||
|
done:
|
||||||
|
iocb->aiocb = NULL;
|
||||||
|
if (iocb->bh) {
|
||||||
|
+ /* Paired with a decrement in ide_trim_bh_cb() */
|
||||||
|
+ blk_inc_in_flight(s->blk);
|
||||||
|
replay_bh_schedule_event(iocb->bh);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
@@ -514,9 +516,6 @@ BlockAIOCB *ide_issue_trim(
|
||||||
|
IDEState *s = opaque;
|
||||||
|
TrimAIOCB *iocb;
|
||||||
|
|
||||||
|
- /* Paired with a decrement in ide_trim_bh_cb() */
|
||||||
|
- blk_inc_in_flight(s->blk);
|
||||||
|
-
|
||||||
|
iocb = blk_aio_get(&trim_aiocb_info, s->blk, cb, cb_opaque);
|
||||||
|
iocb->s = s;
|
||||||
|
iocb->bh = qemu_bh_new(ide_trim_bh_cb, iocb);
|
1
debian/patches/series
vendored
1
debian/patches/series
vendored
@ -8,6 +8,7 @@ extra/0007-block-fix-detect-zeroes-with-BDRV_REQ_REGISTERED_BUF.patch
|
|||||||
extra/0008-memory-prevent-dma-reentracy-issues.patch
|
extra/0008-memory-prevent-dma-reentracy-issues.patch
|
||||||
extra/0009-block-iscsi-fix-double-free-on-BUSY-or-similar-statu.patch
|
extra/0009-block-iscsi-fix-double-free-on-BUSY-or-similar-statu.patch
|
||||||
extra/0010-scsi-megasas-Internal-cdbs-have-16-byte-length.patch
|
extra/0010-scsi-megasas-Internal-cdbs-have-16-byte-length.patch
|
||||||
|
extra/0011-ide-avoid-potential-deadlock-when-draining-during-tr.patch
|
||||||
bitmap-mirror/0001-drive-mirror-add-support-for-sync-bitmap-mode-never.patch
|
bitmap-mirror/0001-drive-mirror-add-support-for-sync-bitmap-mode-never.patch
|
||||||
bitmap-mirror/0002-drive-mirror-add-support-for-conditional-and-always-.patch
|
bitmap-mirror/0002-drive-mirror-add-support-for-conditional-and-always-.patch
|
||||||
bitmap-mirror/0003-mirror-add-check-for-bitmap-mode-without-bitmap.patch
|
bitmap-mirror/0003-mirror-add-check-for-bitmap-mode-without-bitmap.patch
|
||||||
|
Loading…
Reference in New Issue
Block a user