libsas: don't treat underrun as an error on SMP tasks

PATCH In review James Bottomley 2 participants 3 messages Dec 29, 2007 – Dec 30, 2007

Subsystems (3)

SCSI LSI Logic MPT fusion drivers (FC/SAS/SPI) SCSI libsas

lore

Share & tools

lore

James Bottomley

author maintainer

2007-12-29 17:49 UTC #0 lore raw

PATCH 1/1 libsas: don't treat underrun as an error on SMP tasks

All SMP tasks sent through bsg generate messages like:

sas: smp_execute_task: task to dev 500605b000001450 response: 0x0 status 0x81

Three times (because the task gets retried). Firstly, don't retry either overrun or underrun (the data buffer isn't going to change size) and secondly, just report the underrun but don't set an error for it. This is necessary so bsg can report back the residual.

James

drivers/scsi/libsas/sas_expander.c +15 -0

index 76555b1..1578059 100644
@@ -109,6 +109,16 @@ static int smp_execute_task(struct domain_device *dev, void *req, int req_size,
 		    task->task_status.stat == SAM_GOOD) {
 			res = 0;
 			break;
+		} if (task->task_status.resp == SAS_TASK_COMPLETE &&
+		      task->task_status.stat == SAS_DATA_UNDERRUN) {
+			/* no error, but return the number of bytes of
+			 * underrun */
+			res = task->task_status.residual;
+			break;
+		} if (task->task_status.resp == SAS_TASK_COMPLETE &&
+		      task->task_status.stat == SAS_DATA_OVERRUN) {
+			res = -EMSGSIZE;
+			break;
 		} else {
 			SAS_DPRINTK("%s: task to dev %016llx response: 0x%x "
 				    "status 0x%x\n", __FUNCTION__,

@@ -1924,6 +1934,11 @@ int sas_smp_handler(struct Scsi_Host *shost, struct sas_rphy *rphy,
 
 	ret = smp_execute_task(dev, bio_data(req->bio), req->data_len,
 			       bio_data(rsp->bio), rsp->data_len);
+	if (ret > 0) {
+		/* positive number is the untransferred residual */
+		rsp->data_len = ret;
+		ret = 0;
+	}
 
 	return ret;
 }

FUJITA Tomonori

2007-12-30 10:35 UTC #1 lore raw

Quoted 33 lines · On Sat, 29 Dec 2007 11:49:53 -0600 James Bottomley…

All SMP tasks sent through bsg generate messages like:
sas: smp_execute_task: task to dev 500605b000001450 response: 0x0 status 0x81
Three times (because the task gets retried). Firstly, don't retry either overrun or underrun (the data buffer isn't going to change size) and secondly, just report the underrun but don't set an error for it. This is necessary so bsg can report back the residual.
James
drivers/scsi/libsas/sas_expander.c +10 -0
index 76555b1..1578059 100644
@@ -109,6 +109,16 @@ static int smp_execute_task(struct domain_device *dev, void *req, int req_size,
 		    task->task_status.stat == SAM_GOOD) {
 			res = 0;
 			break;
+		} if (task->task_status.resp == SAS_TASK_COMPLETE &&
+		      task->task_status.stat == SAS_DATA_UNDERRUN) {
+			/* no error, but return the number of bytes of
+			 * underrun */
+			res = task->task_status.residual;
+			break;
+		} if (task->task_status.resp == SAS_TASK_COMPLETE &&
+		      task->task_status.stat == SAS_DATA_OVERRUN) {
+			res = -EMSGSIZE;
+			break;
 		} else {
 			SAS_DPRINTK("%s: task to dev %016llx response: 0x%x "
 				    "status 0x%x\n", __FUNCTION__,

drivers/scsi/libsas/sas_expander.c int sas_smp_handler(struct Scsi_Host *shost, struct sas_rphy *rphy, · line 1941

Quoted 8 lines

drivers/scsi/libsas/sas_expander.c +5 -0

@@ -1924,6 +1934,11 @@ int sas_smp_handler(struct Scsi_Host *shost, struct sas_rphy *rphy,
 
 	ret = smp_execute_task(dev, bio_data(req->bio), req->data_len,
 			       bio_data(rsp->bio), rsp->data_len);
+	if (ret > 0) {
+		/* positive number is the untransferred residual */
+		rsp->data_len = ret;
+		ret = 0;
+	}

Would be better to update dout_resid too (on sucess, we can set req->data_len to zero, I think)?

Here's a patch to do the same thing for mpt sas, an updated version of:

http://marc.info/?l=linux-scsi&m=119811872823947&w=2

--
From: FUJITA Tomonori <[email protected]>
Subject: [PATCH] mpt fusion: mptsas_smp_handler updates resid

This patch fixes mptsas_smp_handler to update both din_resid or dout_resid on success. bsg can report back the residual.

Signed-off-by FUJITA Tomonori <[email protected]>

---
 drivers/message/fusion/mptsas.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

drivers/message/fusion/mptsas.c +2 -0

index e4c94f9..f77b329 100644
@@ -1343,6 +1343,8 @@ static int mptsas_smp_handler(struct Scsi_Host *shost, struct sas_rphy *rphy,
 		smprep = (SmpPassthroughReply_t *)ioc->sas_mgmt.reply;
 		memcpy(req->sense, smprep, sizeof(*smprep));
 		req->sense_len = sizeof(*smprep);
+		req->data_len = 0;
+		rsp->data_len -= smprep->ResponseDataLength;
 	} else {
 		printk(MYIOC_s_ERR_FMT "%s: smp passthru reply failed to be returned\n",
 		    ioc->name, __FUNCTION__);

Signature · 1 line

1.5.3.4

James Bottomley

author maintainer

2007-12-30 15:48 UTC #2 lore raw

Quoted 37 lines · FUJITA Tomonori

Quoted 34 lines · On Sat, 29 Dec 2007 11:49:53 -0600 James Bottomley…
All SMP tasks sent through bsg generate messages like:
sas: smp_execute_task: task to dev 500605b000001450 response: 0x0 status 0x81
Three times (because the task gets retried). Firstly, don't retry either overrun or underrun (the data buffer isn't going to change size) and secondly, just report the underrun but don't set an error for it. This is necessary so bsg can report back the residual.
James
drivers/scsi/libsas/sas_expander.c +10 -0
index 76555b1..1578059 100644
@@ -109,6 +109,16 @@ static int smp_execute_task(struct domain_device *dev, void *req, int req_size,
 		    task->task_status.stat == SAM_GOOD) {
 			res = 0;
 			break;
+		} if (task->task_status.resp == SAS_TASK_COMPLETE &&
+		      task->task_status.stat == SAS_DATA_UNDERRUN) {
+			/* no error, but return the number of bytes of
+			 * underrun */
+			res = task->task_status.residual;
+			break;
+		} if (task->task_status.resp == SAS_TASK_COMPLETE &&
+		      task->task_status.stat == SAS_DATA_OVERRUN) {
+			res = -EMSGSIZE;
+			break;
 		} else {
 			SAS_DPRINTK("%s: task to dev %016llx response: 0x%x "
 				    "status 0x%x\n", __FUNCTION__,
@@ -1924,6 +1934,11 @@ int sas_smp_handler(struct Scsi_Host *shost, struct sas_rphy *rphy,
 

Quoted 10 lines

drivers/scsi/libsas/sas_expander.c int sas_smp_handler(struct Scsi_Host *shost, struct sas_rphy *rphy, · line 1941
Quoted 7 lines
drivers/scsi/libsas/sas_expander.c +5 -0
 	ret = smp_execute_task(dev, bio_data(req->bio), req->data_len,
 			       bio_data(rsp->bio), rsp->data_len);
+	if (ret > 0) {
+		/* positive number is the untransferred residual */
+		rsp->data_len = ret;
+		ret = 0;
+	}
Would be better to update dout_resid too (on sucess, we can set req->data_len to zero, I think)?

Yes, I'll add that.

Quoted 4 lines

Here's a patch to do the same thing for mpt sas, an updated version of:
http://marc.info/?l=linux-scsi&m=119811872823947&w=2

And this.

James

Quoted 25 lines

--
From: FUJITA Tomonori <[email protected]>
Subject: [PATCH] mpt fusion: mptsas_smp_handler updates resid
This patch fixes mptsas_smp_handler to update both din_resid or dout_resid on success. bsg can report back the residual.
Signed-off-by FUJITA Tomonori <[email protected]>
---
 drivers/message/fusion/mptsas.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)
drivers/message/fusion/mptsas.c +2 -0
index e4c94f9..f77b329 100644
@@ -1343,6 +1343,8 @@ static int mptsas_smp_handler(struct Scsi_Host *shost, struct sas_rphy *rphy,
 		smprep = (SmpPassthroughReply_t *)ioc->sas_mgmt.reply;
 		memcpy(req->sense, smprep, sizeof(*smprep));
 		req->sense_len = sizeof(*smprep);
+		req->data_len = 0;
+		rsp->data_len -= smprep->ResponseDataLength;
 	} else {
 		printk(MYIOC_s_ERR_FMT "%s: smp passthru reply failed to be returned\n",
 		    ioc->name, __FUNCTION__);