public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: Fiona Ebner <f.ebner@proxmox.com>
To: pve-devel@lists.proxmox.com
Subject: [pve-devel] [PATCH qemu 1/2] fixup patch "ide: avoid potential deadlock when draining during trim"
Date: Thu,  9 Mar 2023 14:37:34 +0100	[thread overview]
Message-ID: <20230309133735.124859-1-f.ebner@proxmox.com> (raw)

The patch was incomplete and (re-)introduced an issue with a potential
failing assertion upon cancelation of the DMA request.

There is a patch on qemu-devel now[0], and it's the same as this one
code-wise (except for comments). But the discussion is still ongoing.
While there shouldn't be a real issue with the patch, there might be
better approaches. The plan is to use this as a stop-gap for now and
pick up the proper solution once it's ready.

[0]: https://lists.nongnu.org/archive/html/qemu-devel/2023-03/msg03325.html

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 ...ial-deadlock-when-draining-during-tr.patch | 28 +++++++++++++++++--
 1 file changed, 25 insertions(+), 3 deletions(-)

diff --git a/debian/patches/extra/0011-ide-avoid-potential-deadlock-when-draining-during-tr.patch b/debian/patches/extra/0011-ide-avoid-potential-deadlock-when-draining-during-tr.patch
index 77d7eee..8ce9c79 100644
--- a/debian/patches/extra/0011-ide-avoid-potential-deadlock-when-draining-during-tr.patch
+++ b/debian/patches/extra/0011-ide-avoid-potential-deadlock-when-draining-during-tr.patch
@@ -37,15 +37,25 @@ 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.
 
+However, the issue 7e5cdb345f fixed for canceling resurfaces, because
+ide_cancel_dma_sync assumes that it just needs to drain once. But now
+the in_flight count is not consistently > 0 during the trim operation.
+So, change it to drain until !s->bus->dma->aiocb, which means that the
+operation finished (s->bus->dma->aiocb is cleared by ide_set_inactive
+via the ide_dma_cb when the end of the transfer is reached).
+
+Discussion here:
+https://lists.nongnu.org/archive/html/qemu-devel/2023-03/msg02506.html
+
 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(-)
+ hw/ide/core.c | 12 ++++++------
+ 1 file changed, 6 insertions(+), 6 deletions(-)
 
 diff --git a/hw/ide/core.c b/hw/ide/core.c
-index 39afdc0006..6474522bc9 100644
+index 39afdc0006..b67c1885a8 100644
 --- a/hw/ide/core.c
 +++ b/hw/ide/core.c
 @@ -443,7 +443,7 @@ static void ide_trim_bh_cb(void *opaque)
@@ -76,3 +86,15 @@ index 39afdc0006..6474522bc9 100644
      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);
+@@ -739,8 +738,9 @@ void ide_cancel_dma_sync(IDEState *s)
+      */
+     if (s->bus->dma->aiocb) {
+         trace_ide_cancel_dma_sync_remaining();
+-        blk_drain(s->blk);
+-        assert(s->bus->dma->aiocb == NULL);
++        while (s->bus->dma->aiocb) {
++            blk_drain(s->blk);
++        }
+     }
+ }
+ 
-- 
2.30.2





             reply	other threads:[~2023-03-09 13:37 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-03-09 13:37 Fiona Ebner [this message]
2023-03-09 13:37 ` [pve-devel] [PATCH qemu 2/2] add more stable fixes Fiona Ebner
2023-03-13 16:53 ` [pve-devel] applied-series: [PATCH qemu 1/2] fixup patch "ide: avoid potential deadlock when draining during trim" Thomas Lamprecht

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230309133735.124859-1-f.ebner@proxmox.com \
    --to=f.ebner@proxmox.com \
    --cc=pve-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal