From patchwork Tue May 3 16:50:17 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Paolo Bonzini X-Patchwork-Id: 93853 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 31F80B6EF7 for ; Wed, 4 May 2011 02:55:05 +1000 (EST) Received: from localhost ([::1]:57798 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QHIsI-0002XP-Ku for incoming@patchwork.ozlabs.org; Tue, 03 May 2011 12:55:02 -0400 Received: from eggs.gnu.org ([140.186.70.92]:42947) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QHIo8-00048Z-Av for qemu-devel@nongnu.org; Tue, 03 May 2011 12:50:45 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QHIo7-0000De-0L for qemu-devel@nongnu.org; Tue, 03 May 2011 12:50:44 -0400 Received: from mail-wy0-f173.google.com ([74.125.82.173]:61623) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QHIo6-0000Ci-OM for qemu-devel@nongnu.org; Tue, 03 May 2011 12:50:42 -0400 Received: by mail-wy0-f173.google.com with SMTP id 42so248862wyb.4 for ; Tue, 03 May 2011 09:50:42 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:sender:from:to:subject:date:message-id:x-mailer :in-reply-to:references; bh=C2+RFjd5O6Yi0mSmRjKRiRYIBGi1SFu1dLJyPnP3tx0=; b=iKZO2bW5cp7miRcykdlR5ypfSGntAOJaqZyXnuBF1z3y8D9mWSPsosfgfJPvlyOaZq MCT6LE61Ny+XdgD5De3Ff4KwcDipYRhEzTwbWU+WjadGfsy1hthPHzpzGoU+5t1J17gb rlJSZCAzMZ4OxFkEdwnnY/Sm8hrQd7cPPmpw8= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=sender:from:to:subject:date:message-id:x-mailer:in-reply-to :references; b=EWV32XB85LW6Ic7OiUO1PAF49IzgC8CYNTOn9DkKUSoNsZdtEE4rvYILx3drqyLt4a lIO1+wAestpwe03nDkKlP+vqulwpWJcoPOwh7eiLj8fuioec41F2iZNWEXAdz+tLZf3d 3X0imomWqlWNNJrzPXcXFAr8IeALIwVZoko2Y= Received: by 10.227.128.138 with SMTP id k10mr34544wbs.82.1304441442217; Tue, 03 May 2011 09:50:42 -0700 (PDT) Received: from localhost.localdomain (93-34-184-88.ip51.fastwebnet.it [93.34.184.88]) by mx.google.com with ESMTPS id bs4sm167243wbb.52.2011.05.03.09.50.41 (version=TLSv1/SSLv3 cipher=OTHER); Tue, 03 May 2011 09:50:41 -0700 (PDT) From: Paolo Bonzini To: qemu-devel@nongnu.org Date: Tue, 3 May 2011 18:50:17 +0200 Message-Id: <1304441432-27434-6-git-send-email-pbonzini@redhat.com> X-Mailer: git-send-email 1.7.4.4 In-Reply-To: <1304441432-27434-1-git-send-email-pbonzini@redhat.com> References: <1304441432-27434-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.173 Subject: [Qemu-devel] [PATCH 05/20] 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 --- hw/scsi-bus.c | 22 ++++++++++++++++++++-- hw/scsi-disk.c | 20 +++++++++++++------- hw/scsi-generic.c | 23 ++++++++++++++++------- hw/scsi.h | 5 +++++ 4 files changed, 54 insertions(+), 16 deletions(-) diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c index 63d9a68..c0bc275 100644 --- a/hw/scsi-bus.c +++ b/hw/scsi-bus.c @@ -136,6 +136,7 @@ SCSIRequest *scsi_req_alloc(size_t size, SCSIDevice *d, uint32_t tag, uint32_t l SCSIRequest *req; req = qemu_mallocz(size); + req->refcount = 2; req->bus = scsi_bus_from_device(d); req->dev = d; req->tag = tag; @@ -159,21 +160,23 @@ 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); + assert(req->refcount == 0); qemu_free(req); } + static int scsi_req_length(SCSIRequest *req, uint8_t *cmd) { switch (cmd[0] >> 5) { @@ -495,6 +498,19 @@ 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) { + req->dev->info->free_req(req); + } +} + void scsi_req_data(SCSIRequest *req, int len) { trace_scsi_req_data(req->dev->id, req->lun, req->tag, len); @@ -532,10 +548,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..ba7ffa1 100644 --- a/hw/scsi-disk.c +++ b/hw/scsi-disk.c @@ -95,8 +95,10 @@ 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); } @@ -131,7 +133,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 +146,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 +1031,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 +1093,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 +1180,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 +1192,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 +1210,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 +1293,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 3db734a..5739067 100644 --- a/hw/scsi-generic.c +++ b/hw/scsi-generic.c @@ -74,8 +74,10 @@ 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); } @@ -112,7 +114,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 +128,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 +321,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 +350,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 +377,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 +395,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 +474,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 +566,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 d1753f9..ecf1aeb 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);