From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:39870) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RGVyB-0005kz-E8 for qemu-devel@nongnu.org; Wed, 19 Oct 2011 09:14:09 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1RGVy5-0004bh-Gt for qemu-devel@nongnu.org; Wed, 19 Oct 2011 09:14:06 -0400 Received: from mail-pz0-f43.google.com ([209.85.210.43]:58506) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RGVy4-0004bT-Tx for qemu-devel@nongnu.org; Wed, 19 Oct 2011 09:14:01 -0400 Received: by pzk33 with SMTP id 33so4285036pzk.2 for ; Wed, 19 Oct 2011 06:13:59 -0700 (PDT) Sender: Paolo Bonzini Message-ID: <4E9ECD11.7060105@redhat.com> Date: Wed, 19 Oct 2011 15:13:53 +0200 From: Paolo Bonzini MIME-Version: 1.0 References: <1318503845-11473-22-git-send-email-pbonzini@redhat.com> <1318865867-2785-1-git-send-email-pbonzini@redhat.com> In-Reply-To: <1318865867-2785-1-git-send-email-pbonzini@redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v2 21/35] scsi-disk: bump SCSIRequest reference count until aio completion runs List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: kwolf@redhat.com, qemu-devel@nongnu.org On 10/17/2011 05:37 PM, Paolo Bonzini wrote: > In some cases a request may be canceled before the completion > callback runs. Keep a reference to the request between starting > an AIO operation, and let scsi_*_complete remove it. > > Since scsi_handle_rw_error returns whether something else has to > be done for the request by the caller, it makes sense to transfer > ownership of the ref to scsi_handle_rw_error when it returns 1; > scsi_dma_restart_bh will then free the reference after restarting > the operation. > > Signed-off-by: Paolo Bonzini > --- > v1->v2: Add "return" after calling scsi_write_complete with > ENOMEDIUM. Bump refcount before testing data direction. > > hw/scsi-disk.c | 16 ++++++++++++++++ > 1 files changed, 16 insertions(+), 0 deletions(-) > > diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c > index e28c39d..702e6ca 100644 > --- a/hw/scsi-disk.c > +++ b/hw/scsi-disk.c > @@ -139,6 +139,7 @@ static void scsi_read_complete(void * opaque, int ret) > > if (ret) { > if (scsi_handle_rw_error(r, -ret, SCSI_REQ_STATUS_RETRY_READ)) { > + /* Leave in ref for scsi_dma_restart_bh. */ > return; > } > } > @@ -149,6 +150,7 @@ static void scsi_read_complete(void * opaque, int ret) > r->sector += n; > r->sector_count -= n; > scsi_req_data(&r->req, r->qiov.size); > + scsi_req_unref(&r->req); > } > > static void scsi_flush_complete(void * opaque, int ret) > @@ -163,11 +165,13 @@ static void scsi_flush_complete(void * opaque, int ret) > > if (ret< 0) { > if (scsi_handle_rw_error(r, -ret, SCSI_REQ_STATUS_RETRY_FLUSH)) { > + /* Leave in ref for scsi_dma_restart_bh. */ > return; > } > } > > scsi_req_complete(&r->req, GOOD); > + scsi_req_unref(&r->req); > } > > /* Read more data from scsi device into buffer. */ > @@ -193,6 +197,8 @@ static void scsi_read_data(SCSIRequest *req) > /* No data transfer may already be in progress */ > assert(r->req.aiocb == NULL); > > + /* Save a ref for scsi_read_complete, in case r is canceled. */ > + scsi_req_ref(&r->req); > if (r->req.cmd.mode == SCSI_XFER_TO_DEV) { > DPRINTF("Data transfer direction invalid\n"); > scsi_read_complete(r, -EINVAL); > @@ -201,7 +207,9 @@ static void scsi_read_data(SCSIRequest *req) > > if (s->tray_open) { > scsi_read_complete(r, -ENOMEDIUM); > + return; > } > + > n = scsi_init_iovec(r); > bdrv_acct_start(s->bs,&r->acct, n * BDRV_SECTOR_SIZE, BDRV_ACCT_READ); > r->req.aiocb = bdrv_aio_readv(s->bs, r->sector,&r->qiov, n, > @@ -279,6 +287,7 @@ static void scsi_write_complete(void * opaque, int ret) > DPRINTF("Write complete tag=0x%x more=%d\n", r->req.tag, r->qiov.size); > scsi_req_data(&r->req, r->qiov.size); > } > + scsi_req_unref(&r->req); > } > > static void scsi_write_data(SCSIRequest *req) > @@ -290,6 +299,8 @@ static void scsi_write_data(SCSIRequest *req) > /* No data transfer may already be in progress */ > assert(r->req.aiocb == NULL); > > + /* Save a ref for scsi_write_complete, in case r is canceled. */ > + scsi_req_ref(&r->req); > if (r->req.cmd.mode != SCSI_XFER_TO_DEV) { > DPRINTF("Data transfer direction invalid\n"); > scsi_write_complete(r, -EINVAL); > @@ -300,6 +311,7 @@ static void scsi_write_data(SCSIRequest *req) > if (n) { > if (s->tray_open) { > scsi_write_complete(r, -ENOMEDIUM); > + return; > } > bdrv_acct_start(s->bs,&r->acct, n * BDRV_SECTOR_SIZE, BDRV_ACCT_WRITE); > r->req.aiocb = bdrv_aio_writev(s->bs, r->sector,&r->qiov, n, > @@ -344,6 +356,8 @@ static void scsi_dma_restart_bh(void *opaque) > scsi_req_complete(&r->req, GOOD); > } > } > + /* This reference was left in by scsi_handle_rw_error. */ > + scsi_req_unref(&r->req); > } > } > } > @@ -1345,6 +1359,8 @@ static int32_t scsi_send_command(SCSIRequest *req, uint8_t *buf) > r->iov.iov_len = rc; > break; > case SYNCHRONIZE_CACHE: > + /* Save a ref for scsi_flush_complete, in case r is canceled. */ > + scsi_req_ref(&r->req); > bdrv_acct_start(s->bs,&r->acct, 0, BDRV_ACCT_FLUSH); > r->req.aiocb = bdrv_aio_flush(s->bs, scsi_flush_complete, r); > if (r->req.aiocb == NULL) { This needs to be redone after dropping 20/35. Paolo