Re: [PATCH 5/24][RFC] dpt_i2o: Use new scsi_eh_cpy_sense()
On Mon, Feb 04 2008 at 20:32 +0200, Salyzyn, Mark [EMAIL PROTECTED] wrote: ACK with condition that community accepts the RFC's entire premise. The removed code that shunted the REQUEST_SENSE was based on the assumption that the sense data in the current scsi command packet was left over from the previous command's execution with a check condition as the scsi command packet is reused to issue the REQUEST_SENSE. For a new, or second from the target's point of view, request sense to the target issued by these older kernels would always return an erased sense. The dpt_i2o driver does not itself maintain the sense history, nor does the Firmware. This behavior, I believe, is not the case for current kernels so the code fragment made little sense (pun not intended). If my historical knowledge is correct, this (now removed) workaround makes no more sense because the scsi layer correctly manages adapters that produce auto-request sense and does not ever turn around the command and send a second request for sense information. Given this understanding, I have no problem with the removed fragment of REQUEST_SENSE shunting. However, I do urge some target error recovery testing, tape drives being the likely type of target affected by this change. I have no such hardware to confirm... Sincerely -- Mark Salyzyn I have removed this test because the midlayer does a scsi_eh_reset_sense() just before the new invocation of a command. So even if the second bad REQUEST_SENSE comes this will not filter it out anymore. If such a thing still happens? A driver state machine must be used to filter it out, or of course midlayer should be fixed. Boaz - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 5/24][RFC] dpt_i2o: Use new scsi_eh_cpy_sense()
I do not think the midlayer needs to be fixed. I think this was a bug/feature that presented itself in the 2.2 tree when we were developing this driver in 1996... Sincerely -- Mark Salyzyn -Original Message- From: Boaz Harrosh [mailto:[EMAIL PROTECTED] Sent: Tuesday, February 05, 2008 3:52 AM To: Salyzyn, Mark Cc: James Bottomley; FUJITA Tomonori; Christoph Hellwig; Jens Axboe; Jeff Garzik; linux-scsi; Andrew Morton Subject: Re: [PATCH 5/24][RFC] dpt_i2o: Use new scsi_eh_cpy_sense() On Mon, Feb 04 2008 at 20:32 +0200, Salyzyn, Mark [EMAIL PROTECTED] wrote: ACK with condition that community accepts the RFC's entire premise. The removed code that shunted the REQUEST_SENSE was based on the assumption that the sense data in the current scsi command packet was left over from the previous command's execution with a check condition as the scsi command packet is reused to issue the REQUEST_SENSE. For a new, or second from the target's point of view, request sense to the target issued by these older kernels would always return an erased sense. The dpt_i2o driver does not itself maintain the sense history, nor does the Firmware. This behavior, I believe, is not the case for current kernels so the code fragment made little sense (pun not intended). If my historical knowledge is correct, this (now removed) workaround makes no more sense because the scsi layer correctly manages adapters that produce auto-request sense and does not ever turn around the command and send a second request for sense information. Given this understanding, I have no problem with the removed fragment of REQUEST_SENSE shunting. However, I do urge some target error recovery testing, tape drives being the likely type of target affected by this change. I have no such hardware to confirm... Sincerely -- Mark Salyzyn I have removed this test because the midlayer does a scsi_eh_reset_sense() just before the new invocation of a command. So even if the second bad REQUEST_SENSE comes this will not filter it out anymore. If such a thing still happens? A driver state machine must be used to filter it out, or of course midlayer should be fixed. Boaz - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 5/24][RFC] dpt_i2o: Use new scsi_eh_cpy_sense()
ACK with condition that community accepts the RFC's entire premise.
The removed code that shunted the REQUEST_SENSE was based on the assumption
that the sense data in the current scsi command packet was left over from the
previous command's execution with a check condition as the scsi command packet
is reused to issue the REQUEST_SENSE. For a new, or second from the target's
point of view, request sense to the target issued by these older kernels would
always return an erased sense. The dpt_i2o driver does not itself maintain the
sense history, nor does the Firmware. This behavior, I believe, is not the case
for current kernels so the code fragment made little sense (pun not intended).
If my historical knowledge is correct, this (now removed) workaround makes no
more sense because the scsi layer correctly manages adapters that produce
auto-request sense and does not ever turn around the command and send a second
request for sense information.
Given this understanding, I have no problem with the removed fragment of
REQUEST_SENSE shunting. However, I do urge some target error recovery testing,
tape drives being the likely type of target affected by this change. I have no
such hardware to confirm...
Sincerely -- Mark Salyzyn
-Original Message-
From: [EMAIL PROTECTED]
[mailto:[EMAIL PROTECTED] On Behalf Of Boaz Harrosh
Sent: Monday, February 04, 2008 10:59 AM
To: James Bottomley; FUJITA Tomonori; Christoph Hellwig; Jens
Axboe; Jeff Garzik; linux-scsi
Cc: Andrew Morton
Subject: [PATCH 5/24][RFC] dpt_i2o: Use new scsi_eh_cpy_sense()
- Abstract away scsi_cmnd-sense_buffer for later removal.
- Removed a filtering out of a REQUEST_SENSE at .queuecommand
In the case of sense beeing clean. This is no longer relevant
since scsi-ml will always send a zero out sense buffer even
on a resend, so this means outside REQUEST_SENSE would never
go through. If this is intended then comment and check should
change.
Signed-off-by: Boaz Harrosh [EMAIL PROTECTED]
---
drivers/scsi/dpt_i2o.c | 25 +++--
1 files changed, 7 insertions(+), 18 deletions(-)
diff --git a/drivers/scsi/dpt_i2o.c b/drivers/scsi/dpt_i2o.c
index c9dd839..6ee6fcd 100644
--- a/drivers/scsi/dpt_i2o.c
+++ b/drivers/scsi/dpt_i2o.c
@@ -71,6 +71,7 @@ MODULE_DESCRIPTION(Adaptec I2O RAID Driver);
#include scsi/scsi_device.h
#include scsi/scsi_host.h
#include scsi/scsi_tcq.h
+#include scsi/scsi_eh.h
#include dpt/dptsig.h
#include dpti.h
@@ -385,18 +386,6 @@ static int adpt_queue(struct scsi_cmnd *
cmd, void (*done) (struct scsi_cmnd *))
struct adpt_device* pDev = NULL;/* dpt per
device information */
cmd-scsi_done = done;
- /*
-* SCSI REQUEST_SENSE commands will be executed
automatically by the
-* Host Adapter for any errors, so they should not be executed
-* explicitly unless the Sense Data is zero
indicating that no error
-* occurred.
-*/
-
- if ((cmd-cmnd[0] == REQUEST_SENSE)
(cmd-sense_buffer[0] != 0)) {
- cmd-result = (DID_OK 16);
- cmd-scsi_done(cmd);
- return 0;
- }
pHba = (adpt_hba*)cmd-device-host-hostdata[0];
if (!pHba) {
@@ -2226,8 +2215,6 @@ static s32 adpt_i2o_to_scsi(void
__iomem *reply, struct scsi_cmnd* cmd)
pHba = (adpt_hba*) cmd-device-host-hostdata[0];
- cmd-sense_buffer[0] = '\0'; // initialize sense
valid flag to false
-
if(!(reply_flags MSG_FAIL)) {
switch(detailed_status I2O_SCSI_DSC_MASK) {
case I2O_SCSI_DSC_SUCCESS:
@@ -2297,11 +2284,13 @@ static s32 adpt_i2o_to_scsi(void
__iomem *reply, struct scsi_cmnd* cmd)
// copy over the request sense data if it was a check
// condition status
if (dev_status == SAM_STAT_CHECK_CONDITION) {
- u32 len = min(SCSI_SENSE_BUFFERSIZE, 40);
+ u8 sense_buffer[40];
+ u32 len = sizeof(sense_buffer);
// Copy over the sense data
- memcpy_fromio(cmd-sense_buffer,
(reply+28) , len);
- if(cmd-sense_buffer[0] == 0x70 /*
class 7 */
- cmd-sense_buffer[2] == DATA_PROTECT ){
+ memcpy_fromio(sense_buffer, (reply+28) , len);
+ scsi_eh_cpy_sense(cmd, sense_buffer, len);
+ if (sense_buffer[0] == 0x70 /* class 7 */
+ sense_buffer[2] == DATA_PROTECT){
/* This is to handle an array
failed */
cmd-result = (DID_TIME_OUT 16);
printk(KERN_WARNING%s: SCSI
Data Protect-Device (%d,%d,%d) hba_status=0x%x,
dev_status=0x%x, cmd=0x%x\n,
--
1.5.3.3
-
To unsubscribe from this list: send
