From patchwork Tue Apr 2 21:45:16 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Michael Roth X-Patchwork-Id: 233193 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id A58002C015A for ; Wed, 3 Apr 2013 09:49:37 +1100 (EST) Received: from localhost ([::1]:33200 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UN9A8-0003mg-T5 for incoming@patchwork.ozlabs.org; Tue, 02 Apr 2013 17:54:40 -0400 Received: from eggs.gnu.org ([208.118.235.92]:36628) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UN96c-0007AZ-Ro for qemu-devel@nongnu.org; Tue, 02 Apr 2013 17:51:04 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UN96a-00006C-3q for qemu-devel@nongnu.org; Tue, 02 Apr 2013 17:51:02 -0400 Received: from mail-yh0-x22c.google.com ([2607:f8b0:4002:c01::22c]:50496) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UN96Z-00005u-V0; Tue, 02 Apr 2013 17:51:00 -0400 Received: by mail-yh0-f44.google.com with SMTP id q11so143504yhf.17 for ; Tue, 02 Apr 2013 14:50:59 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=x-received:sender:from:to:cc:subject:date:message-id:x-mailer :in-reply-to:references; bh=jaikkkfXOrhsXCXemtmkV5A6gmxmi5AellW6IILx/1U=; b=xpjZfy8/Cktn88UWf4/+7z0rnZ1jSCb6RkjWU7Wk34l2qcMCGw8+jUdm/3fnrukM5H gCqDnSbCnbKiA5kTgkhO347dB7CTgE7CpVQsDXWc45ecruAVRIcGPlgKIjdchtUkSHC2 tV4SZmLDr54midNCJY346wMAzFmlaED7BEjt3MF05MehnYDdmZ71htQ2KAGbuy62Ahdl /zDvJAME00x8f/f99Kbwg2AZI0sGuGYESOj/1OO55/pQG/D5e9d6jL5loRqq7gRiZEPB B32wU/MoMxPC8SnAvp8NLqIfnsIE0lO5Rt99QBvoBT3ZvmRUBXIHTuXCX4SQBzTWkfGi Epsg== X-Received: by 10.236.134.162 with SMTP id s22mr16514793yhi.172.1364939459494; Tue, 02 Apr 2013 14:50:59 -0700 (PDT) Received: from localhost ([32.97.110.51]) by mx.google.com with ESMTPS id v48sm5667959yhi.26.2013.04.02.14.50.58 (version=TLSv1.2 cipher=RC4-SHA bits=128/128); Tue, 02 Apr 2013 14:50:58 -0700 (PDT) From: Michael Roth To: qemu-devel@nongnu.org Date: Tue, 2 Apr 2013 16:45:16 -0500 Message-Id: <1364939142-30066-12-git-send-email-mdroth@linux.vnet.ibm.com> X-Mailer: git-send-email 1.7.9.5 In-Reply-To: <1364939142-30066-1-git-send-email-mdroth@linux.vnet.ibm.com> References: <1364939142-30066-1-git-send-email-mdroth@linux.vnet.ibm.com> X-detected-operating-system: by eggs.gnu.org: Error: Malformed IPv6 address (bad octet value). X-Received-From: 2607:f8b0:4002:c01::22c Cc: qemu-stable@nongnu.org Subject: [Qemu-devel] [PATCH 11/37] scsi-disk: handle io_canceled uniformly and correctly 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 From: Paolo Bonzini Always check it immediately after calling bdrv_acct_done, and always do a "goto done" in case the "done" label has to free some memory---as is the case for scsi_unmap_complete in the previous patch. This patch could fix problems that happen when a request is split into multiple parts, and one of them is canceled. Then the next part is fired, but the HBA's cancellation callbacks have fired already. Whether this happens or not, depends on how the block/ driver implements AIO cancellation. It it does a simple bdrv_drain_all() or similar, then it will not have a problem. If it only cancels the given AIOCB, this scenario could happen. Cc: qemu-stable@nongnu.org Signed-off-by: Paolo Bonzini (cherry picked from commit 0c92e0e6b64c9061f7365a2712b9055ea35b52f9) Signed-off-by: Michael Roth --- hw/scsi-disk.c | 25 +++++++++++++++++++++---- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c index 28e75bb..6bc739d 100644 --- a/hw/scsi-disk.c +++ b/hw/scsi-disk.c @@ -176,6 +176,9 @@ static void scsi_aio_complete(void *opaque, int ret) assert(r->req.aiocb != NULL); r->req.aiocb = NULL; bdrv_acct_done(s->qdev.conf.bs, &r->acct); + if (r->req.io_canceled) { + goto done; + } if (ret < 0) { if (scsi_handle_rw_error(r, -ret)) { @@ -221,6 +224,10 @@ static void scsi_write_do_fua(SCSIDiskReq *r) { SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, r->req.dev); + if (r->req.io_canceled) { + goto done; + } + if (scsi_is_cmd_fua(&r->req.cmd)) { bdrv_acct_start(s->qdev.conf.bs, &r->acct, 0, BDRV_ACCT_FLUSH); r->req.aiocb = bdrv_aio_flush(s->qdev.conf.bs, scsi_aio_complete, r); @@ -228,6 +235,8 @@ static void scsi_write_do_fua(SCSIDiskReq *r) } scsi_req_complete(&r->req, GOOD); + +done: if (!r->req.io_canceled) { scsi_req_unref(&r->req); } @@ -241,6 +250,9 @@ static void scsi_dma_complete(void *opaque, int ret) assert(r->req.aiocb != NULL); r->req.aiocb = NULL; bdrv_acct_done(s->qdev.conf.bs, &r->acct); + if (r->req.io_canceled) { + goto done; + } if (ret < 0) { if (scsi_handle_rw_error(r, -ret)) { @@ -272,6 +284,9 @@ static void scsi_read_complete(void * opaque, int ret) assert(r->req.aiocb != NULL); r->req.aiocb = NULL; bdrv_acct_done(s->qdev.conf.bs, &r->acct); + if (r->req.io_canceled) { + goto done; + } if (ret < 0) { if (scsi_handle_rw_error(r, -ret)) { @@ -303,6 +318,9 @@ static void scsi_do_read(void *opaque, int ret) r->req.aiocb = NULL; bdrv_acct_done(s->qdev.conf.bs, &r->acct); } + if (r->req.io_canceled) { + goto done; + } if (ret < 0) { if (scsi_handle_rw_error(r, -ret)) { @@ -310,10 +328,6 @@ static void scsi_do_read(void *opaque, int ret) } } - if (r->req.io_canceled) { - return; - } - /* The request is used as the AIO opaque value, so add a ref. */ scsi_req_ref(&r->req); @@ -421,6 +435,9 @@ static void scsi_write_complete(void * opaque, int ret) r->req.aiocb = NULL; bdrv_acct_done(s->qdev.conf.bs, &r->acct); } + if (r->req.io_canceled) { + goto done; + } if (ret < 0) { if (scsi_handle_rw_error(r, -ret)) {