From patchwork Mon May 23 16:08:50 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Paolo Bonzini X-Patchwork-Id: 96995 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 BFDE4B6FB6 for ; Tue, 24 May 2011 02:13:58 +1000 (EST) Received: from localhost ([::1]:44468 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QOXlU-0007rX-2C for incoming@patchwork.ozlabs.org; Mon, 23 May 2011 12:13:56 -0400 Received: from eggs.gnu.org ([140.186.70.92]:57375) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QOXhP-0001Gv-Ix for qemu-devel@nongnu.org; Mon, 23 May 2011 12:09:45 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QOXhO-0001FN-4s for qemu-devel@nongnu.org; Mon, 23 May 2011 12:09:43 -0400 Received: from mail-pz0-f45.google.com ([209.85.210.45]:49894) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QOXhN-0001EL-Nj for qemu-devel@nongnu.org; Mon, 23 May 2011 12:09:41 -0400 Received: by mail-pz0-f45.google.com with SMTP id 30so3212143pzk.4 for ; Mon, 23 May 2011 09:09:41 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:sender:from:to:cc:subject:date:message-id :x-mailer:in-reply-to:references; bh=mtdnTnTC+/cWK76Q15s+SXyxkgAXyA4KC/lCW7tOF/M=; b=DD+fIFUOmA86ycDNPQJw4MJ49F9I/m4CKEciMD7LEkqF3uweHtFBaVOkoxK058yjaj edpIg9rBQxQ3kXoG4K1lsIj1gJD4Y4mMStJnbRE4moy4dRsZ4puMnw2HcEFYe0kDDTER TqjdPcarWveYAkEWjxZgfWEUinjFi13MfYA7E= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=sender:from:to:cc:subject:date:message-id:x-mailer:in-reply-to :references; b=nNySb83jdJPmaBzMwFPp7wYd7MqxLfcMUIxShEVwswuvt1tIq6MLapD3AFgpKOhSCP 2hqsZ6a+zGbXeG6/JgvGqKTbdaat43+glW5ZnW2S47mxUT49MfPQvvuTt5FC0PLhAavy SJboRWC835TDZCdy2se7Bi12d/UVWfJxMxrbE= Received: by 10.142.149.26 with SMTP id w26mr834310wfd.402.1306166981335; Mon, 23 May 2011 09:09:41 -0700 (PDT) Received: from localhost.localdomain (93-34-184-88.ip51.fastwebnet.it [93.34.184.88]) by mx.google.com with ESMTPS id x12sm5946194wfd.6.2011.05.23.09.09.38 (version=TLSv1/SSLv3 cipher=OTHER); Mon, 23 May 2011 09:09:40 -0700 (PDT) From: Paolo Bonzini To: qemu-devel@nongnu.org Date: Mon, 23 May 2011 18:08:50 +0200 Message-Id: <1306166949-19698-6-git-send-email-pbonzini@redhat.com> X-Mailer: git-send-email 1.7.4.4 In-Reply-To: <1306166949-19698-1-git-send-email-pbonzini@redhat.com> References: <1306166949-19698-1-git-send-email-pbonzini@redhat.com> X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6 (newer, 2) X-Received-From: 209.85.210.45 Cc: hch@lst.de Subject: [Qemu-devel] [PATCH v4 05/24] scsi: reference-count requests 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 With the next patch, a device may hold SCSIRequest for an indefinite time. Split a rather big patch, and protect against access errors, by reference counting them. There is some ugliness in scsi_send_command implementation due to the need to unref the request when it fails. This will go away with the next patches, which move the unref'ing to the devices. Signed-off-by: Paolo Bonzini Cc: Christoph Hellwig --- hw/scsi-bus.c | 29 ++++++++++++++++++++++------- hw/scsi-disk.c | 21 +++++++++++++-------- hw/scsi-generic.c | 24 ++++++++++++++++-------- hw/scsi.h | 5 +++++ 4 files changed, 56 insertions(+), 23 deletions(-) diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c index f21704f..74b9a74 100644 --- a/hw/scsi-bus.c +++ b/hw/scsi-bus.c @@ -136,6 +136,8 @@ SCSIRequest *scsi_req_alloc(size_t size, SCSIDevice *d, uint32_t tag, uint32_t l SCSIRequest *req; req = qemu_mallocz(size); + /* Two references: one is passed back to the HBA, one is in d->requests. */ + req->refcount = 2; req->bus = scsi_bus_from_device(d); req->dev = d; req->tag = tag; @@ -159,21 +161,16 @@ SCSIRequest *scsi_req_find(SCSIDevice *d, uint32_t tag) return NULL; } -static void scsi_req_dequeue(SCSIRequest *req) +void scsi_req_dequeue(SCSIRequest *req) { trace_scsi_req_dequeue(req->dev->id, req->lun, req->tag); if (req->enqueued) { QTAILQ_REMOVE(&req->dev->requests, req, next); req->enqueued = false; + scsi_req_unref(req); } } -void scsi_req_free(SCSIRequest *req) -{ - scsi_req_dequeue(req); - qemu_free(req); -} - static int scsi_req_length(SCSIRequest *req, uint8_t *cmd) { switch (cmd[0] >> 5) { @@ -495,6 +492,22 @@ static const char *scsi_command_name(uint8_t cmd) return names[cmd]; } +SCSIRequest *scsi_req_ref(SCSIRequest *req) +{ + req->refcount++; + return req; +} + +void scsi_req_unref(SCSIRequest *req) +{ + if (--req->refcount == 0) { + if (req->dev->info->free_req) { + req->dev->info->free_req(req); + } + qemu_free(req); + } +} + /* Called by the devices when data is ready for the HBA. The HBA should start a DMA operation to read or fill the device's data buffer. Once it completes, calling one of req->dev->info->read_data or @@ -537,10 +550,12 @@ void scsi_req_print(SCSIRequest *req) void scsi_req_complete(SCSIRequest *req) { assert(req->status != -1); + scsi_req_ref(req); scsi_req_dequeue(req); req->bus->ops->complete(req->bus, SCSI_REASON_DONE, req->tag, req->status); + scsi_req_unref(req); } static char *scsibus_get_fw_dev_path(DeviceState *dev) diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c index 2b5dc2a..d17f43e 100644 --- a/hw/scsi-disk.c +++ b/hw/scsi-disk.c @@ -95,10 +95,11 @@ static SCSIDiskReq *scsi_new_request(SCSIDiskState *s, uint32_t tag, return r; } -static void scsi_remove_request(SCSIDiskReq *r) +static void scsi_free_request(SCSIRequest *req) { + SCSIDiskReq *r = DO_UPCAST(SCSIDiskReq, req, req); + qemu_vfree(r->iov.iov_base); - scsi_req_free(&r->req); } static SCSIDiskReq *scsi_find_request(SCSIDiskState *s, uint32_t tag) @@ -131,7 +132,6 @@ static void scsi_command_complete(SCSIDiskReq *r, int status, int sense) r->req.tag, status, sense); scsi_req_set_status(r, status, sense); scsi_req_complete(&r->req); - scsi_remove_request(r); } /* Cancel a pending data transfer. */ @@ -145,7 +145,7 @@ static void scsi_cancel_io(SCSIDevice *d, uint32_t tag) if (r->req.aiocb) bdrv_aio_cancel(r->req.aiocb); r->req.aiocb = NULL; - scsi_remove_request(r); + scsi_req_dequeue(&r->req); } } @@ -1030,7 +1030,7 @@ static int32_t scsi_send_command(SCSIDevice *d, uint32_t tag, uint8_t *buf, int lun) { SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, d); - uint32_t len; + int32_t len; int is_write; uint8_t command; uint8_t *outbuf; @@ -1092,6 +1092,7 @@ static int32_t scsi_send_command(SCSIDevice *d, uint32_t tag, case REZERO_UNIT: rc = scsi_disk_emulate_command(r, outbuf); if (rc < 0) { + scsi_req_unref(&r->req); return 0; } @@ -1178,9 +1179,11 @@ static int32_t scsi_send_command(SCSIDevice *d, uint32_t tag, DPRINTF("Unknown SCSI command (%2.2x)\n", buf[0]); fail: scsi_command_complete(r, CHECK_CONDITION, ILLEGAL_REQUEST); + scsi_req_unref(&r->req); return 0; illegal_lba: scsi_command_complete(r, CHECK_CONDITION, HARDWARE_ERROR); + scsi_req_unref(&r->req); return 0; } if (r->sector_count == 0 && r->iov.iov_len == 0) { @@ -1188,12 +1191,13 @@ static int32_t scsi_send_command(SCSIDevice *d, uint32_t tag, } len = r->sector_count * 512 + r->iov.iov_len; if (is_write) { - return -len; + len = -len; } else { if (!r->sector_count) r->sector_count = -1; - return len; } + scsi_req_unref(&r->req); + return len; } static void scsi_disk_purge_requests(SCSIDiskState *s) @@ -1205,7 +1209,7 @@ static void scsi_disk_purge_requests(SCSIDiskState *s) if (r->req.aiocb) { bdrv_aio_cancel(r->req.aiocb); } - scsi_remove_request(r); + scsi_req_dequeue(&r->req); } } @@ -1288,6 +1292,7 @@ static SCSIDeviceInfo scsi_disk_info = { .qdev.reset = scsi_disk_reset, .init = scsi_disk_initfn, .destroy = scsi_destroy, + .free_req = scsi_free_request, .send_command = scsi_send_command, .read_data = scsi_read_data, .write_data = scsi_write_data, diff --git a/hw/scsi-generic.c b/hw/scsi-generic.c index f09458b..7e4831c 100644 --- a/hw/scsi-generic.c +++ b/hw/scsi-generic.c @@ -74,10 +74,11 @@ static SCSIGenericReq *scsi_new_request(SCSIDevice *d, uint32_t tag, uint32_t lu return DO_UPCAST(SCSIGenericReq, req, req); } -static void scsi_remove_request(SCSIGenericReq *r) +static void scsi_free_request(SCSIRequest *req) { + SCSIGenericReq *r = DO_UPCAST(SCSIGenericReq, req, req); + qemu_free(r->buf); - scsi_req_free(&r->req); } static SCSIGenericReq *scsi_find_request(SCSIGenericState *s, uint32_t tag) @@ -112,7 +113,6 @@ static void scsi_command_complete(void *opaque, int ret) r, r->req.tag, r->req.status); scsi_req_complete(&r->req); - scsi_remove_request(r); } /* Cancel a pending data transfer. */ @@ -127,7 +127,7 @@ static void scsi_cancel_io(SCSIDevice *d, uint32_t tag) if (r->req.aiocb) bdrv_aio_cancel(r->req.aiocb); r->req.aiocb = NULL; - scsi_remove_request(r); + scsi_req_dequeue(&r->req); } } @@ -320,6 +320,7 @@ static int32_t scsi_send_command(SCSIDevice *d, uint32_t tag, SCSIGenericReq *r; SCSIBus *bus; int ret; + int32_t len; if (cmd[0] != REQUEST_SENSE && (lun != s->lun || (cmd[1] >> 5) != s->lun)) { @@ -348,7 +349,8 @@ static int32_t scsi_send_command(SCSIDevice *d, uint32_t tag, if (-1 == scsi_req_parse(&r->req, cmd)) { BADF("Unsupported command length, command %x\n", cmd[0]); - scsi_remove_request(r); + scsi_req_dequeue(&r->req); + scsi_req_unref(&r->req); return 0; } scsi_req_fixup(&r->req); @@ -374,8 +376,10 @@ static int32_t scsi_send_command(SCSIDevice *d, uint32_t tag, ret = execute_command(s->bs, r, SG_DXFER_NONE, scsi_command_complete); if (ret == -1) { scsi_command_complete(r, -EINVAL); + scsi_req_unref(&r->req); return 0; } + scsi_req_unref(&r->req); return 0; } @@ -390,10 +394,13 @@ static int32_t scsi_send_command(SCSIDevice *d, uint32_t tag, r->len = r->req.cmd.xfer; if (r->req.cmd.mode == SCSI_XFER_TO_DEV) { r->len = 0; - return -r->req.cmd.xfer; + len = -r->req.cmd.xfer; + } else { + len = r->req.cmd.xfer; } - return r->req.cmd.xfer; + scsi_req_unref(&r->req); + return len; } static int get_blocksize(BlockDriverState *bdrv) @@ -466,7 +473,7 @@ static void scsi_generic_purge_requests(SCSIGenericState *s) if (r->req.aiocb) { bdrv_aio_cancel(r->req.aiocb); } - scsi_remove_request(r); + scsi_req_dequeue(&r->req); } } @@ -558,6 +565,7 @@ static SCSIDeviceInfo scsi_generic_info = { .qdev.reset = scsi_generic_reset, .init = scsi_generic_initfn, .destroy = scsi_destroy, + .free_req = scsi_free_request, .send_command = scsi_send_command, .read_data = scsi_read_data, .write_data = scsi_write_data, diff --git a/hw/scsi.h b/hw/scsi.h index 20cf397..7d97dfa 100644 --- a/hw/scsi.h +++ b/hw/scsi.h @@ -29,6 +29,7 @@ enum SCSIXferMode { typedef struct SCSIRequest { SCSIBus *bus; SCSIDevice *dev; + uint32_t refcount; uint32_t tag; uint32_t lun; uint32_t status; @@ -65,6 +66,7 @@ struct SCSIDeviceInfo { DeviceInfo qdev; scsi_qdev_initfn init; void (*destroy)(SCSIDevice *s); + void (*free_req)(SCSIRequest *req); int32_t (*send_command)(SCSIDevice *s, uint32_t tag, uint8_t *buf, int lun); void (*read_data)(SCSIDevice *s, uint32_t tag); @@ -103,6 +105,9 @@ int scsi_bus_legacy_handle_cmdline(SCSIBus *bus); SCSIRequest *scsi_req_alloc(size_t size, SCSIDevice *d, uint32_t tag, uint32_t lun); SCSIRequest *scsi_req_find(SCSIDevice *d, uint32_t tag); void scsi_req_free(SCSIRequest *req); +void scsi_req_dequeue(SCSIRequest *req); +SCSIRequest *scsi_req_ref(SCSIRequest *req); +void scsi_req_unref(SCSIRequest *req); int scsi_req_parse(SCSIRequest *req, uint8_t *buf); void scsi_req_print(SCSIRequest *req);