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