From patchwork Fri Sep 16 14:40:02 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Paolo Bonzini X-Patchwork-Id: 114954 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [140.186.70.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id 02B08B70BF for ; Sat, 17 Sep 2011 01:34:46 +1000 (EST) Received: from localhost ([::1]:58329 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1R4Zat-0003DJ-PQ for incoming@patchwork.ozlabs.org; Fri, 16 Sep 2011 10:40:43 -0400 Received: from eggs.gnu.org ([140.186.70.92]:37435) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1R4ZaZ-0002EN-DG for qemu-devel@nongnu.org; Fri, 16 Sep 2011 10:40:29 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1R4ZaP-0000F1-CP for qemu-devel@nongnu.org; Fri, 16 Sep 2011 10:40:17 -0400 Received: from mail-ww0-f53.google.com ([74.125.82.53]:35476) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1R4ZaP-0000EC-7w for qemu-devel@nongnu.org; Fri, 16 Sep 2011 10:40:13 -0400 Received: by mail-ww0-f53.google.com with SMTP id 14so4650252wwg.10 for ; Fri, 16 Sep 2011 07:40:12 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=sender:from:to:subject:date:message-id:x-mailer:in-reply-to :references; bh=fRNOCcdAylutsBCGHW+ws/KfwIxk13/UHwTxjZg5xhY=; b=vrCvjr6uIQCoOYS2NstkQLYJisRs4RQxJALSXtGCw5mlvnBPSB7ndg4bjDxrqcTQxT U6Tv0KM6M5cG6TuEBcQfW6EIsBf3TTlpOUzUVyuyJAyBur9tsY5vT54G+LdesTYnxn3x Sv3lumqJCbYJwJ06ZZt1MF1+8XRX5Y+sXYnys= Received: by 10.227.7.141 with SMTP id d13mr779270wbd.78.1316184012736; Fri, 16 Sep 2011 07:40:12 -0700 (PDT) Received: from localhost.localdomain (93-34-199-31.ip51.fastwebnet.it. [93.34.199.31]) by mx.google.com with ESMTPS id i11sm12750379wbn.25.2011.09.16.07.40.11 (version=TLSv1/SSLv3 cipher=OTHER); Fri, 16 Sep 2011 07:40:12 -0700 (PDT) From: Paolo Bonzini To: qemu-devel@nongnu.org Date: Fri, 16 Sep 2011 16:40:02 +0200 Message-Id: <1316184004-8260-4-git-send-email-pbonzini@redhat.com> X-Mailer: git-send-email 1.7.6 In-Reply-To: <1316184004-8260-1-git-send-email-pbonzini@redhat.com> References: <1316184004-8260-1-git-send-email-pbonzini@redhat.com> X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6 (newer, 2) X-Received-From: 74.125.82.53 Subject: [Qemu-devel] [PATCH v2 3/5] dma-helpers: rewrite completion/cancellation X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org This fixes various problems with completion/cancellation: * if the io_func fails to get an AIOCB, the callback wasn't called * If DMA encounters a bounce buffer conflict, and the DMA operation is canceled before the bottom half fires, bad things happen. * memory is not unmapped after cancellation, again causing problems when doing DMA to I/O areas * cancellation could leak the iovec * the callback was missed if the I/O operation failed without returning an AIOCB and probably more that I've missed. The patch fixes them by sharing the cleanup code between completion and cancellation. The dma_bdrv_cb now returns a boolean completed/not completed flag, and the wrapper dma_continue takes care of tasks to do upon completion. Most of these are basically impossible in practice, but it is better to be tidy... Signed-off-by: Paolo Bonzini --- v1->v2: call the callback when the io_func fails. Add in_cancel. dma-helpers.c | 44 +++++++++++++++++++++++++++++++------------- 1 files changed, 31 insertions(+), 13 deletions(-) diff --git a/dma-helpers.c b/dma-helpers.c index 717e384..86d2d0a 100644 --- a/dma-helpers.c +++ b/dma-helpers.c @@ -43,6 +43,7 @@ typedef struct { QEMUSGList *sg; uint64_t sector_num; bool to_dev; + bool in_cancel; int sg_cur_index; target_phys_addr_t sg_cur_byte; QEMUIOVector iov; @@ -58,7 +59,7 @@ static void reschedule_dma(void *opaque) qemu_bh_delete(dbs->bh); dbs->bh = NULL; - dma_bdrv_cb(opaque, 0); + dma_bdrv_cb(dbs, 0); } static void continue_after_map_failure(void *opaque) @@ -78,6 +79,26 @@ static void dma_bdrv_unmap(DMAAIOCB *dbs) dbs->iov.iov[i].iov_len, !dbs->to_dev, dbs->iov.iov[i].iov_len); } + qemu_iovec_reset(&dbs->iov); +} + +static void dma_complete(DMAAIOCB *dbs, int ret) +{ + dma_bdrv_unmap(dbs); + if (dbs->common.cb) { + dbs->common.cb(dbs->common.opaque, ret); + } + qemu_iovec_destroy(&dbs->iov); + if (dbs->bh) { + qemu_bh_delete(dbs->bh); + dbs->bh = NULL; + } + if (!dbs->in_cancel) { + /* Requests may complete while dma_aio_cancel is in progress. In + * this case, the AIOCB should not be released because it is still + * referenced by dma_aio_cancel. */ + qemu_aio_release(dbs); + } } static void dma_bdrv_cb(void *opaque, int ret) @@ -89,12 +110,9 @@ static void dma_bdrv_cb(void *opaque, int ret) dbs->acb = NULL; dbs->sector_num += dbs->iov.size / 512; dma_bdrv_unmap(dbs); - qemu_iovec_reset(&dbs->iov); if (dbs->sg_cur_index == dbs->sg->nsg || ret < 0) { - dbs->common.cb(dbs->common.opaque, ret); - qemu_iovec_destroy(&dbs->iov); - qemu_aio_release(dbs); + dma_complete(dbs, ret); return; } @@ -120,9 +138,7 @@ static void dma_bdrv_cb(void *opaque, int ret) dbs->acb = dbs->io_func(dbs->bs, dbs->sector_num, &dbs->iov, dbs->iov.size / 512, dma_bdrv_cb, dbs); if (!dbs->acb) { - dma_bdrv_unmap(dbs); - qemu_iovec_destroy(&dbs->iov); - return; + dma_complete(dbs, -EIO); } } @@ -131,8 +147,14 @@ static void dma_aio_cancel(BlockDriverAIOCB *acb) DMAAIOCB *dbs = container_of(acb, DMAAIOCB, common); if (dbs->acb) { - bdrv_aio_cancel(dbs->acb); + BlockDriverAIOCB *acb = dbs->acb; + dbs->acb = NULL; + dbs->in_cancel = true; + bdrv_aio_cancel(acb); + dbs->in_cancel = false; } + dbs->common.cb = NULL; + dma_complete(dbs, 0); } static AIOPool dma_aio_pool = { @@ -158,10 +180,6 @@ BlockDriverAIOCB *dma_bdrv_io( dbs->bh = NULL; qemu_iovec_init(&dbs->iov, sg->nsg); dma_bdrv_cb(dbs, 0); - if (!dbs->acb) { - qemu_aio_release(dbs); - return NULL; - } return &dbs->common; }