From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id 6DA6C90B76 for ; Thu, 9 Mar 2023 14:37:42 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 4A709B6DE for ; Thu, 9 Mar 2023 14:37:42 +0100 (CET) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS for ; Thu, 9 Mar 2023 14:37:40 +0100 (CET) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 30A5345DEF for ; Thu, 9 Mar 2023 14:37:40 +0100 (CET) From: Fiona Ebner To: pve-devel@lists.proxmox.com Date: Thu, 9 Mar 2023 14:37:34 +0100 Message-Id: <20230309133735.124859-1-f.ebner@proxmox.com> X-Mailer: git-send-email 2.30.2 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 AWL -0.003 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: [pve-devel] [PATCH qemu 1/2] fixup patch "ide: avoid potential deadlock when draining during trim" X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 09 Mar 2023 13:37:42 -0000 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 --- ...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 Signed-off-by: Fiona Ebner --- - 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