1be5c7
From eaade87072e903cf550dfdb8ed1480dddc6bb0e3 Mon Sep 17 00:00:00 2001
1be5c7
From: Hanna Reitz <hreitz@redhat.com>
1be5c7
Date: Thu, 20 Jan 2022 15:22:59 +0100
1be5c7
Subject: [PATCH 21/24] ide: Increment BB in-flight counter for TRIM BH
1be5c7
MIME-Version: 1.0
1be5c7
Content-Type: text/plain; charset=UTF-8
1be5c7
Content-Transfer-Encoding: 8bit
1be5c7
1be5c7
RH-Author: Hanna Reitz <hreitz@redhat.com>
1be5c7
RH-MergeRequest: 188: ide: Increment BB in-flight counter for TRIM BH
1be5c7
RH-Commit: [1/1] 1e702e735ff63f2b8b69c20cac1b309dd085cd62
1be5c7
RH-Bugzilla: 2029980
1be5c7
RH-Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
1be5c7
RH-Acked-by: Kevin Wolf <kwolf@redhat.com>
1be5c7
RH-Acked-by: Paolo Bonzini <pbonzini@redhat.com>
1be5c7
1be5c7
When we still have an AIOCB registered for DMA operations, we try to
1be5c7
settle the respective operation by draining the BlockBackend associated
1be5c7
with the IDE device.
1be5c7
1be5c7
However, this assumes that every DMA operation is associated with an
1be5c7
increment of the BlockBackend’s in-flight counter (e.g. through some
1be5c7
ongoing I/O operation), so that draining the BB until its in-flight
1be5c7
counter reaches 0 will settle all DMA operations.  That is not the case:
1be5c7
For TRIM, the guest can issue a zero-length operation that will not
1be5c7
result in any I/O operation forwarded to the BlockBackend, and also not
1be5c7
increment the in-flight counter in any other way.  In such a case,
1be5c7
blk_drain() will be a no-op if no other operations are in flight.
1be5c7
1be5c7
It is clear that if blk_drain() is a no-op, the value of
1be5c7
s->bus->dma->aiocb will not change between checking it in the `if`
1be5c7
condition and asserting that it is NULL after blk_drain().
1be5c7
1be5c7
The particular problem is that ide_issue_trim() creates a BH
1be5c7
(ide_trim_bh_cb()) to settle the TRIM request: iocb->common.cb() is
1be5c7
ide_dma_cb(), which will either create a new request, or find the
1be5c7
transfer to be done and call ide_set_inactive(), which clears
1be5c7
s->bus->dma->aiocb.  Therefore, the blk_drain() must wait for
1be5c7
ide_trim_bh_cb() to run, which currently it will not always do.
1be5c7
1be5c7
To fix this issue, we increment the BlockBackend's in-flight counter
1be5c7
when the TRIM operation begins (in ide_issue_trim(), when the
1be5c7
ide_trim_bh_cb() BH is created) and decrement it when ide_trim_bh_cb()
1be5c7
is done.
1be5c7
1be5c7
Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=2029980
1be5c7
Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
1be5c7
Signed-off-by: Hanna Reitz <hreitz@redhat.com>
1be5c7
Message-Id: <20220120142259.120189-1-hreitz@redhat.com>
1be5c7
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
1be5c7
Reviewed-by: John Snow <jsnow@redhat.com>
1be5c7
Tested-by: John Snow <jsnow@redhat.com>
1be5c7
(cherry picked from commit 7e5cdb345f77d76cb4877fe6230c4e17a7d0d0ca)
1be5c7
Signed-off-by: Hanna Reitz <hreitz@redhat.com>
1be5c7
---
1be5c7
 hw/ide/core.c | 7 +++++++
1be5c7
 1 file changed, 7 insertions(+)
1be5c7
1be5c7
diff --git a/hw/ide/core.c b/hw/ide/core.c
1be5c7
index e28f8aad61..15138225be 100644
1be5c7
--- a/hw/ide/core.c
1be5c7
+++ b/hw/ide/core.c
1be5c7
@@ -433,12 +433,16 @@ static const AIOCBInfo trim_aiocb_info = {
1be5c7
 static void ide_trim_bh_cb(void *opaque)
1be5c7
 {
1be5c7
     TrimAIOCB *iocb = opaque;
1be5c7
+    BlockBackend *blk = iocb->s->blk;
1be5c7
 
1be5c7
     iocb->common.cb(iocb->common.opaque, iocb->ret);
1be5c7
 
1be5c7
     qemu_bh_delete(iocb->bh);
1be5c7
     iocb->bh = NULL;
1be5c7
     qemu_aio_unref(iocb);
1be5c7
+
1be5c7
+    /* Paired with an increment in ide_issue_trim() */
1be5c7
+    blk_dec_in_flight(blk);
1be5c7
 }
1be5c7
 
1be5c7
 static void ide_issue_trim_cb(void *opaque, int ret)
1be5c7
@@ -508,6 +512,9 @@ BlockAIOCB *ide_issue_trim(
1be5c7
     IDEState *s = opaque;
1be5c7
     TrimAIOCB *iocb;
1be5c7
 
1be5c7
+    /* Paired with a decrement in ide_trim_bh_cb() */
1be5c7
+    blk_inc_in_flight(s->blk);
1be5c7
+
1be5c7
     iocb = blk_aio_get(&trim_aiocb_info, s->blk, cb, cb_opaque);
1be5c7
     iocb->s = s;
1be5c7
     iocb->bh = qemu_bh_new(ide_trim_bh_cb, iocb);
1be5c7
-- 
1be5c7
2.35.3
1be5c7