linux-scsi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: vikas.chaudhary@qlogic.com
To: jbottomley@parallels.com, michaelc@cs.wisc.edu
Cc: linux-scsi@vger.kernel.org, vikas.chaudhary@qlogic.com,
	lalit.chandivade@qlogic.com, ravi.anand@qlogic.com,
	Tej Parkash <tej.parkash@qlogic.com>
Subject: [PATCH 4/5] qla4xxx: Properly handle SCSI underrun while processing status IOCBs.
Date: Tue,  7 Aug 2012 07:57:16 -0400	[thread overview]
Message-ID: <1344340637-25526-5-git-send-email-vikas.chaudhary@qlogic.com> (raw)
In-Reply-To: <1344340637-25526-1-git-send-email-vikas.chaudhary@qlogic.com>

From: Lalit Chandivade <lalit.chandivade@qlogic.com>

The current code would incorrectly return a DID_OK for a
CHECK CONDITION with Recovered error sense key causing incorrect
completion of a command when there is a dropped frame.

Signed-off-by: Lalit Chandivade <lalit.chandivade@qlogic.com>
Signed-off-by: Tej Parkash <tej.parkash@qlogic.com>
Signed-off-by: Vikas Chaudhary <vikas.chaudhary@qlogic.com>
---
 drivers/scsi/qla4xxx/ql4_isr.c |  102 +++++++++++++++++++++++-----------------
 1 files changed, 59 insertions(+), 43 deletions(-)

diff --git a/drivers/scsi/qla4xxx/ql4_isr.c b/drivers/scsi/qla4xxx/ql4_isr.c
index fc542a9..db8c8e5 100644
--- a/drivers/scsi/qla4xxx/ql4_isr.c
+++ b/drivers/scsi/qla4xxx/ql4_isr.c
@@ -243,56 +243,72 @@ static void qla4xxx_status_entry(struct scsi_qla_host *ha,
 
 		scsi_set_resid(cmd, residual);
 
-		/*
-		 * If there is scsi_status, it takes precedense over
-		 * underflow condition.
-		 */
-		if (scsi_status != 0) {
-			cmd->result = DID_OK << 16 | scsi_status;
+		if (sts_entry->iscsiFlags & ISCSI_FLAG_RESIDUAL_UNDER) {
+
+			/* Both the firmware and target reported UNDERRUN:
+			 *
+			 * MID-LAYER UNDERFLOW case:
+			 * Some kernels do not properly detect midlayer
+			 * underflow, so we manually check it and return
+			 * ERROR if the minimum required data was not
+			 * received.
+			 *
+			 * ALL OTHER cases:
+			 * Fall thru to check scsi_status
+			 */
+			if (!scsi_status && (scsi_bufflen(cmd) - residual) <
+			    cmd->underflow) {
+				DEBUG2(ql4_printk(KERN_INFO, ha,
+						  "scsi%ld:%d:%d:%d: %s: Mid-layer Data underrun, xferlen = 0x%x,residual = 0x%x\n",
+						   ha->host_no,
+						   cmd->device->channel,
+						   cmd->device->id,
+						   cmd->device->lun, __func__,
+						   scsi_bufflen(cmd),
+						   residual));
 
-			if (scsi_status != SCSI_CHECK_CONDITION)
+				cmd->result = DID_ERROR << 16;
 				break;
+			}
+
+		} else if (scsi_status != SAM_STAT_TASK_SET_FULL &&
+			   scsi_status != SAM_STAT_BUSY) {
 
-			/* Copy Sense Data into sense buffer. */
-			qla4xxx_copy_sense(ha, sts_entry, srb);
-		} else {
 			/*
-			 * If RISC reports underrun and target does not
-			 * report it then we must have a lost frame, so
-			 * tell upper layer to retry it by reporting a
-			 * bus busy.
+			 * The firmware reports UNDERRUN, but the target does
+			 * not report it:
+			 *
+			 *   scsi_status     |    host_byte       device_byte
+			 *                   |     (19:16)          (7:0)
+			 *   =============   |    =========       ===========
+			 *   TASK_SET_FULL   |    DID_OK          scsi_status
+			 *   BUSY            |    DID_OK          scsi_status
+			 *   ALL OTHERS      |    DID_ERROR       scsi_status
+			 *
+			 *   Note: If scsi_status is task set full or busy,
+			 *   then this else if would fall thru to check the
+			 *   scsi_status and return DID_OK.
 			 */
-			if ((sts_entry->iscsiFlags &
-			     ISCSI_FLAG_RESIDUAL_UNDER) == 0) {
-				cmd->result = DID_BUS_BUSY << 16;
-			} else if ((scsi_bufflen(cmd) - residual) <
-				   cmd->underflow) {
-				/*
-				 * Handle mid-layer underflow???
-				 *
-				 * For kernels less than 2.4, the driver must
-				 * return an error if an underflow is detected.
-				 * For kernels equal-to and above 2.4, the
-				 * mid-layer will appearantly handle the
-				 * underflow by detecting the residual count --
-				 * unfortunately, we do not see where this is
-				 * actually being done.	 In the interim, we
-				 * will return DID_ERROR.
-				 */
-				DEBUG2(printk("scsi%ld:%d:%d:%d: %s: "
-					"Mid-layer Data underrun1, "
-					"xferlen = 0x%x, "
-					"residual = 0x%x\n", ha->host_no,
-					cmd->device->channel,
-					cmd->device->id,
-					cmd->device->lun, __func__,
-					scsi_bufflen(cmd), residual));
 
-				cmd->result = DID_ERROR << 16;
-			} else {
-				cmd->result = DID_OK << 16;
-			}
+			DEBUG2(ql4_printk(KERN_INFO, ha,
+					  "scsi%ld:%d:%d:%d: %s: Dropped frame(s) detected (0x%x of 0x%x bytes).\n",
+					  ha->host_no,
+					  cmd->device->channel,
+					  cmd->device->id,
+					  cmd->device->lun, __func__,
+					  residual,
+					  scsi_bufflen(cmd)));
+
+			cmd->result = DID_ERROR << 16 | scsi_status;
+			goto check_scsi_status;
 		}
+
+		cmd->result = DID_OK << 16 | scsi_status;
+
+check_scsi_status:
+		if (scsi_status == SAM_STAT_CHECK_CONDITION)
+			qla4xxx_copy_sense(ha, sts_entry, srb);
+
 		break;
 
 	case SCS_DEVICE_LOGGED_OUT:
-- 
1.7.8.GIT


  parent reply	other threads:[~2012-08-07 12:15 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-08-07 11:57 [PATCH 0/5] qla4xxx: Updates for scsi "misc" branch vikas.chaudhary
2012-08-07 11:57 ` [PATCH 1/5] qla4xxx: Fix memory corruption issue in qla4xxx_ep_connect vikas.chaudhary
2012-08-07 11:57 ` [PATCH 2/5] qla4xxx: Fix gcc warning for x86 system vikas.chaudhary
2012-08-07 11:57 ` [PATCH 3/5] qla4xxx: Fix multiple conn login event issue during session recovery vikas.chaudhary
2012-08-07 11:57 ` vikas.chaudhary [this message]
2012-08-07 11:57 ` [PATCH 5/5] qla4xxx: Update driver version to 5.02.00-k19 vikas.chaudhary
2012-08-07 16:52 ` [PATCH 0/5] qla4xxx: Updates for scsi "misc" branch Mike Christie

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1344340637-25526-5-git-send-email-vikas.chaudhary@qlogic.com \
    --to=vikas.chaudhary@qlogic.com \
    --cc=jbottomley@parallels.com \
    --cc=lalit.chandivade@qlogic.com \
    --cc=linux-scsi@vger.kernel.org \
    --cc=michaelc@cs.wisc.edu \
    --cc=ravi.anand@qlogic.com \
    --cc=tej.parkash@qlogic.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).