diff options
| author | James Bottomley <James.Bottomley@HansenPartnership.com> | 2009-01-06 14:15:20 -0500 |
|---|---|---|
| committer | James Bottomley <James.Bottomley@HansenPartnership.com> | 2009-01-07 16:15:44 -0500 |
| commit | 79ed24297236b7430d6ce0a1511ff70cf5b6015a (patch) | |
| tree | 396b2696f4ec57851856702f27019a81a3aefecd | |
| parent | 4be98c0ca304c8a47998b29a7993664f71791250 (diff) | |
[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 <stern@rowland.harvard.edu>
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 <andrew.vasquez@qlogic.com>
Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
| -rw-r--r-- | drivers/scsi/scsi_lib.c | 43 |
1 files changed, 27 insertions, 16 deletions
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c index cc613bae4ad3..940dc32ff0dc 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) | |||
| 701 | scsi_run_queue(sdev->request_queue); | 701 | scsi_run_queue(sdev->request_queue); |
| 702 | } | 702 | } |
| 703 | 703 | ||
| 704 | static void __scsi_release_buffers(struct scsi_cmnd *, int); | ||
| 705 | |||
| 704 | /* | 706 | /* |
| 705 | * Function: scsi_end_request() | 707 | * Function: scsi_end_request() |
| 706 | * | 708 | * |
| @@ -749,6 +751,7 @@ static struct scsi_cmnd *scsi_end_request(struct scsi_cmnd *cmd, int error, | |||
| 749 | * leftovers in the front of the | 751 | * leftovers in the front of the |
| 750 | * queue, and goose the queue again. | 752 | * queue, and goose the queue again. |
| 751 | */ | 753 | */ |
| 754 | scsi_release_buffers(cmd); | ||
| 752 | scsi_requeue_command(q, cmd); | 755 | scsi_requeue_command(q, cmd); |
| 753 | cmd = NULL; | 756 | cmd = NULL; |
| 754 | } | 757 | } |
| @@ -760,6 +763,7 @@ static struct scsi_cmnd *scsi_end_request(struct scsi_cmnd *cmd, int error, | |||
| 760 | * This will goose the queue request function at the end, so we don't | 763 | * This will goose the queue request function at the end, so we don't |
| 761 | * need to worry about launching another command. | 764 | * need to worry about launching another command. |
| 762 | */ | 765 | */ |
| 766 | __scsi_release_buffers(cmd, 0); | ||
| 763 | scsi_next_command(cmd); | 767 | scsi_next_command(cmd); |
| 764 | return NULL; | 768 | return NULL; |
| 765 | } | 769 | } |
| @@ -815,6 +819,26 @@ static void scsi_free_sgtable(struct scsi_data_buffer *sdb) | |||
| 815 | __sg_free_table(&sdb->table, SCSI_MAX_SG_SEGMENTS, scsi_sg_free); | 819 | __sg_free_table(&sdb->table, SCSI_MAX_SG_SEGMENTS, scsi_sg_free); |
| 816 | } | 820 | } |
| 817 | 821 | ||
| 822 | static void __scsi_release_buffers(struct scsi_cmnd *cmd, int do_bidi_check) | ||
| 823 | { | ||
| 824 | |||
| 825 | if (cmd->sdb.table.nents) | ||
| 826 | scsi_free_sgtable(&cmd->sdb); | ||
| 827 | |||
| 828 | memset(&cmd->sdb, 0, sizeof(cmd->sdb)); | ||
| 829 | |||
| 830 | if (do_bidi_check && scsi_bidi_cmnd(cmd)) { | ||
| 831 | struct scsi_data_buffer *bidi_sdb = | ||
| 832 | cmd->request->next_rq->special; | ||
| 833 | scsi_free_sgtable(bidi_sdb); | ||
| 834 | kmem_cache_free(scsi_sdb_cache, bidi_sdb); | ||
| 835 | cmd->request->next_rq->special = NULL; | ||
| 836 | } | ||
| 837 | |||
| 838 | if (scsi_prot_sg_count(cmd)) | ||
| 839 | scsi_free_sgtable(cmd->prot_sdb); | ||
| 840 | } | ||
| 841 | |||
| 818 | /* | 842 | /* |
| 819 | * Function: scsi_release_buffers() | 843 | * Function: scsi_release_buffers() |
| 820 | * | 844 | * |
| @@ -834,21 +858,7 @@ static void scsi_free_sgtable(struct scsi_data_buffer *sdb) | |||
| 834 | */ | 858 | */ |
| 835 | void scsi_release_buffers(struct scsi_cmnd *cmd) | 859 | void scsi_release_buffers(struct scsi_cmnd *cmd) |
| 836 | { | 860 | { |
| 837 | if (cmd->sdb.table.nents) | 861 | __scsi_release_buffers(cmd, 1); |
| 838 | scsi_free_sgtable(&cmd->sdb); | ||
| 839 | |||
| 840 | memset(&cmd->sdb, 0, sizeof(cmd->sdb)); | ||
| 841 | |||
| 842 | if (scsi_bidi_cmnd(cmd)) { | ||
| 843 | struct scsi_data_buffer *bidi_sdb = | ||
| 844 | cmd->request->next_rq->special; | ||
| 845 | scsi_free_sgtable(bidi_sdb); | ||
| 846 | kmem_cache_free(scsi_sdb_cache, bidi_sdb); | ||
| 847 | cmd->request->next_rq->special = NULL; | ||
| 848 | } | ||
| 849 | |||
| 850 | if (scsi_prot_sg_count(cmd)) | ||
| 851 | scsi_free_sgtable(cmd->prot_sdb); | ||
| 852 | } | 862 | } |
| 853 | EXPORT_SYMBOL(scsi_release_buffers); | 863 | EXPORT_SYMBOL(scsi_release_buffers); |
| 854 | 864 | ||
| @@ -962,7 +972,6 @@ void scsi_io_completion(struct scsi_cmnd *cmd, unsigned int good_bytes) | |||
| 962 | } | 972 | } |
| 963 | 973 | ||
| 964 | BUG_ON(blk_bidi_rq(req)); /* bidi not support for !blk_pc_request yet */ | 974 | BUG_ON(blk_bidi_rq(req)); /* bidi not support for !blk_pc_request yet */ |
| 965 | scsi_release_buffers(cmd); | ||
| 966 | 975 | ||
| 967 | /* | 976 | /* |
| 968 | * Next deal with any sectors which we were able to correctly | 977 | * 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) | |||
| 1080 | switch (action) { | 1089 | switch (action) { |
| 1081 | case ACTION_FAIL: | 1090 | case ACTION_FAIL: |
| 1082 | /* Give up and fail the remainder of the request */ | 1091 | /* Give up and fail the remainder of the request */ |
| 1092 | scsi_release_buffers(cmd); | ||
| 1083 | if (!(req->cmd_flags & REQ_QUIET)) { | 1093 | if (!(req->cmd_flags & REQ_QUIET)) { |
| 1084 | if (description) | 1094 | if (description) |
| 1085 | scmd_printk(KERN_INFO, cmd, "%s\n", | 1095 | scmd_printk(KERN_INFO, cmd, "%s\n", |
| @@ -1095,6 +1105,7 @@ void scsi_io_completion(struct scsi_cmnd *cmd, unsigned int good_bytes) | |||
| 1095 | /* Unprep the request and put it back at the head of the queue. | 1105 | /* Unprep the request and put it back at the head of the queue. |
| 1096 | * A new command will be prepared and issued. | 1106 | * A new command will be prepared and issued. |
| 1097 | */ | 1107 | */ |
| 1108 | scsi_release_buffers(cmd); | ||
| 1098 | scsi_requeue_command(q, cmd); | 1109 | scsi_requeue_command(q, cmd); |
| 1099 | break; | 1110 | break; |
| 1100 | case ACTION_RETRY: | 1111 | case ACTION_RETRY: |
