From 3dbf6a54046052d79743822c9206af191e582ab0 Mon Sep 17 00:00:00 2001 From: Alan Stern Date: Mon, 15 Dec 2008 10:31:28 -0500 Subject: [SCSI] Fix uninitialized variable error in scsi_io_completion This patch (as1191) adds a missing "default" case in scsi_io_completion(), thereby fixing an "uninitialized variable" error. It also adds a missing newline to a log entry. Signed-off-by: Alan Stern Signed-off-by: James Bottomley --- drivers/scsi/scsi_lib.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) (limited to 'drivers/scsi/scsi_lib.c') diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c index f2f51e0333e..8c73bb4e0a2 100644 --- a/drivers/scsi/scsi_lib.c +++ b/drivers/scsi/scsi_lib.c @@ -1029,6 +1029,10 @@ void scsi_io_completion(struct scsi_cmnd *cmd, unsigned int good_bytes) case 0x09: /* self test in progress */ action = ACTION_DELAYED_RETRY; break; + default: + description = "Device not ready"; + action = ACTION_FAIL; + break; } } else { description = "Device not ready"; @@ -1054,7 +1058,7 @@ void scsi_io_completion(struct scsi_cmnd *cmd, unsigned int good_bytes) /* Give up and fail the remainder of the request */ if (!(req->cmd_flags & REQ_QUIET)) { if (description) - scmd_printk(KERN_INFO, cmd, "%s", + scmd_printk(KERN_INFO, cmd, "%s\n", description); scsi_print_result(cmd); if (driver_byte(result) & DRIVER_SENSE) -- cgit v1.2.3-70-g09d2 From 4f5299ac4e3a03d5c596c00d726fa932c600609d Mon Sep 17 00:00:00 2001 From: James Bottomley Date: Fri, 2 Jan 2009 10:42:21 -0600 Subject: [SCSI] scsi_lib: don't decrement busy counters when inserting commands A bug was introduced by commit b60af5b0adf0da24c673598c8d3fb4d4189a15ce Author: Alan Stern Date: Mon Nov 3 15:56:47 2008 -0500 [SCSI] simplify scsi_io_completion() because the simplification uses scsi_queue_insert(). The problem with this function is that it expects to be called from the completion path while the command is still outstanding, so it decrements the device and host busy counts to do the requeue. The problem is that scsi_io_completion() is a path executed well after these counts have *already* been decremented, leading to a double decrement if the command goes down any error path leading to ACTION_DELAYED_RETRY. The fix is to allow a private function __scsi_queue_insert() with a flag to say whether the busy counters should be decremented. This is made static to scsi_lib.c to discourage other use. Reported-by: Mike Christie Signed-off-by: James Bottomley --- drivers/scsi/scsi_lib.c | 61 +++++++++++++++++++++++++++++++------------------ 1 file changed, 39 insertions(+), 22 deletions(-) (limited to 'drivers/scsi/scsi_lib.c') diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c index 8c73bb4e0a2..911514c1fea 100644 --- a/drivers/scsi/scsi_lib.c +++ b/drivers/scsi/scsi_lib.c @@ -91,26 +91,19 @@ static void scsi_unprep_request(struct request *req) scsi_put_command(cmd); } -/* - * Function: scsi_queue_insert() - * - * Purpose: Insert a command in the midlevel queue. - * - * Arguments: cmd - command that we are adding to queue. - * reason - why we are inserting command to queue. - * - * Lock status: Assumed that lock is not held upon entry. - * - * Returns: Nothing. - * - * Notes: We do this for one of two cases. Either the host is busy - * and it cannot accept any more commands for the time being, - * or the device returned QUEUE_FULL and can accept no more - * commands. - * Notes: This could be called either from an interrupt context or a - * normal process context. +/** + * __scsi_queue_insert - private queue insertion + * @cmd: The SCSI command being requeued + * @reason: The reason for the requeue + * @unbusy: Whether the queue should be unbusied + * + * This is a private queue insertion. The public interface + * scsi_queue_insert() always assumes the queue should be unbusied + * because it's always called before the completion. This function is + * for a requeue after completion, which should only occur in this + * file. */ -int scsi_queue_insert(struct scsi_cmnd *cmd, int reason) +static int __scsi_queue_insert(struct scsi_cmnd *cmd, int reason, int unbusy) { struct Scsi_Host *host = cmd->device->host; struct scsi_device *device = cmd->device; @@ -150,7 +143,8 @@ int scsi_queue_insert(struct scsi_cmnd *cmd, int reason) * Decrement the counters, since these commands are no longer * active on the host/device. */ - scsi_device_unbusy(device); + if (unbusy) + scsi_device_unbusy(device); /* * Requeue this command. It will go before all other commands @@ -172,6 +166,29 @@ int scsi_queue_insert(struct scsi_cmnd *cmd, int reason) return 0; } +/* + * Function: scsi_queue_insert() + * + * Purpose: Insert a command in the midlevel queue. + * + * Arguments: cmd - command that we are adding to queue. + * reason - why we are inserting command to queue. + * + * Lock status: Assumed that lock is not held upon entry. + * + * Returns: Nothing. + * + * Notes: We do this for one of two cases. Either the host is busy + * and it cannot accept any more commands for the time being, + * or the device returned QUEUE_FULL and can accept no more + * commands. + * Notes: This could be called either from an interrupt context or a + * normal process context. + */ +int scsi_queue_insert(struct scsi_cmnd *cmd, int reason) +{ + return __scsi_queue_insert(cmd, reason, 1); +} /** * scsi_execute - insert request and wait for the result * @sdev: scsi device @@ -1075,11 +1092,11 @@ void scsi_io_completion(struct scsi_cmnd *cmd, unsigned int good_bytes) break; case ACTION_RETRY: /* Retry the same command immediately */ - scsi_queue_insert(cmd, SCSI_MLQUEUE_EH_RETRY); + __scsi_queue_insert(cmd, SCSI_MLQUEUE_EH_RETRY, 0); break; case ACTION_DELAYED_RETRY: /* Retry the same command after a delay */ - scsi_queue_insert(cmd, SCSI_MLQUEUE_DEVICE_BUSY); + __scsi_queue_insert(cmd, SCSI_MLQUEUE_DEVICE_BUSY, 0); break; } } -- cgit v1.2.3-70-g09d2 From 3e695f89c5debb735e4ff051e9e58d8fb4e95110 Mon Sep 17 00:00:00 2001 From: "Martin K. Petersen" Date: Sun, 4 Jan 2009 03:04:31 -0500 Subject: [SCSI] Fix error handling for DIF/DIX patch commit b60af5b0adf0da24c673598c8d3fb4d4189a15ce Author: Alan Stern Date: Mon Nov 3 15:56:47 2008 -0500 [SCSI] simplify scsi_io_completion() broke DIX error handling. Also, we are now using EILSEQ to indicate integrity errors to the upper layers (as opposed to regular EIO failures). This allows filesystems to inspect buffers and decide whether to retry the I/O. Update scsi_io_completion() accordingly. Signed-off-by: Martin K. Petersen Signed-off-by: James Bottomley --- drivers/scsi/scsi_lib.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) (limited to 'drivers/scsi/scsi_lib.c') diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c index 911514c1fea..cc613bae4ad 100644 --- a/drivers/scsi/scsi_lib.c +++ b/drivers/scsi/scsi_lib.c @@ -980,6 +980,8 @@ void scsi_io_completion(struct scsi_cmnd *cmd, unsigned int good_bytes) return; this_count = blk_rq_bytes(req); + error = -EIO; + if (host_byte(result) == DID_RESET) { /* Third party bus reset or reset for error recovery * reasons. Just retry the command and see what @@ -1021,13 +1023,18 @@ void scsi_io_completion(struct scsi_cmnd *cmd, unsigned int good_bytes) /* This will issue a new 6-byte command. */ cmd->device->use_10_for_rw = 0; action = ACTION_REPREP; + } else if (sshdr.asc == 0x10) /* DIX */ { + description = "Host Data Integrity Failure"; + action = ACTION_FAIL; + error = -EILSEQ; } else action = ACTION_FAIL; break; case ABORTED_COMMAND: if (sshdr.asc == 0x10) { /* DIF */ + description = "Target Data Integrity Failure"; action = ACTION_FAIL; - description = "Data Integrity Failure"; + error = -EILSEQ; } else action = ACTION_RETRY; break; -- cgit v1.2.3-70-g09d2 From 79ed24297236b7430d6ce0a1511ff70cf5b6015a Mon Sep 17 00:00:00 2001 From: James Bottomley Date: Tue, 6 Jan 2009 13:15:20 -0600 Subject: [SCSI] scsi_lib: fix DID_RESET status problems Andrew Vaszquez said: > There's a problem that is causing commands returned by the LLD with > a DID_RESET status to be reissued with cleared cmd->sdb data which > in our tests are manifesting in firmware detected overruns. Here's > a snippet of a READ_10 scsi_cmnd upon completion by the storage The problem is caused by: commit b60af5b0adf0da24c673598c8d3fb4d4189a15ce Author: Alan Stern Date: Mon Nov 3 15:56:47 2008 -0500 [SCSI] simplify scsi_io_completion() Because scsi_release_buffers() is called before commands that go through the ACTION_RETRY and ACTION_DELAYED_RETRY legs are requeued. However, they're not re-prepared, so nothing ever reallocates the buffer resources to them. Fix this by releasing the buffers only if we're not going to go down these legs (but scsi_release_buffers() on all legs including two in scsi_end_request(); this latter needs a special version __scsi_release_buffers() because the final one can be called after the request has been freed, so the bidi test in scsi_release_buffers(), which touches the request has to be skipped). Reported-by: Andrew Vasquez Signed-off-by: James Bottomley --- drivers/scsi/scsi_lib.c | 43 +++++++++++++++++++++++++++---------------- 1 file changed, 27 insertions(+), 16 deletions(-) (limited to 'drivers/scsi/scsi_lib.c') diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c index cc613bae4ad..940dc32ff0d 100644 --- a/drivers/scsi/scsi_lib.c +++ b/drivers/scsi/scsi_lib.c @@ -701,6 +701,8 @@ void scsi_run_host_queues(struct Scsi_Host *shost) scsi_run_queue(sdev->request_queue); } +static void __scsi_release_buffers(struct scsi_cmnd *, int); + /* * Function: scsi_end_request() * @@ -749,6 +751,7 @@ static struct scsi_cmnd *scsi_end_request(struct scsi_cmnd *cmd, int error, * leftovers in the front of the * queue, and goose the queue again. */ + scsi_release_buffers(cmd); scsi_requeue_command(q, cmd); cmd = NULL; } @@ -760,6 +763,7 @@ static struct scsi_cmnd *scsi_end_request(struct scsi_cmnd *cmd, int error, * This will goose the queue request function at the end, so we don't * need to worry about launching another command. */ + __scsi_release_buffers(cmd, 0); scsi_next_command(cmd); return NULL; } @@ -815,6 +819,26 @@ static void scsi_free_sgtable(struct scsi_data_buffer *sdb) __sg_free_table(&sdb->table, SCSI_MAX_SG_SEGMENTS, scsi_sg_free); } +static void __scsi_release_buffers(struct scsi_cmnd *cmd, int do_bidi_check) +{ + + if (cmd->sdb.table.nents) + scsi_free_sgtable(&cmd->sdb); + + memset(&cmd->sdb, 0, sizeof(cmd->sdb)); + + if (do_bidi_check && scsi_bidi_cmnd(cmd)) { + struct scsi_data_buffer *bidi_sdb = + cmd->request->next_rq->special; + scsi_free_sgtable(bidi_sdb); + kmem_cache_free(scsi_sdb_cache, bidi_sdb); + cmd->request->next_rq->special = NULL; + } + + if (scsi_prot_sg_count(cmd)) + scsi_free_sgtable(cmd->prot_sdb); +} + /* * Function: scsi_release_buffers() * @@ -834,21 +858,7 @@ static void scsi_free_sgtable(struct scsi_data_buffer *sdb) */ void scsi_release_buffers(struct scsi_cmnd *cmd) { - if (cmd->sdb.table.nents) - scsi_free_sgtable(&cmd->sdb); - - memset(&cmd->sdb, 0, sizeof(cmd->sdb)); - - if (scsi_bidi_cmnd(cmd)) { - struct scsi_data_buffer *bidi_sdb = - cmd->request->next_rq->special; - scsi_free_sgtable(bidi_sdb); - kmem_cache_free(scsi_sdb_cache, bidi_sdb); - cmd->request->next_rq->special = NULL; - } - - if (scsi_prot_sg_count(cmd)) - scsi_free_sgtable(cmd->prot_sdb); + __scsi_release_buffers(cmd, 1); } EXPORT_SYMBOL(scsi_release_buffers); @@ -962,7 +972,6 @@ void scsi_io_completion(struct scsi_cmnd *cmd, unsigned int good_bytes) } BUG_ON(blk_bidi_rq(req)); /* bidi not support for !blk_pc_request yet */ - scsi_release_buffers(cmd); /* * Next deal with any sectors which we were able to correctly @@ -1080,6 +1089,7 @@ void scsi_io_completion(struct scsi_cmnd *cmd, unsigned int good_bytes) switch (action) { case ACTION_FAIL: /* Give up and fail the remainder of the request */ + scsi_release_buffers(cmd); if (!(req->cmd_flags & REQ_QUIET)) { if (description) scmd_printk(KERN_INFO, cmd, "%s\n", @@ -1095,6 +1105,7 @@ void scsi_io_completion(struct scsi_cmnd *cmd, unsigned int good_bytes) /* Unprep the request and put it back at the head of the queue. * A new command will be prepared and issued. */ + scsi_release_buffers(cmd); scsi_requeue_command(q, cmd); break; case ACTION_RETRY: -- cgit v1.2.3-70-g09d2