Skip to contentSkip to conversation

status_byte() and SAM TASK_ABORTED

PATCH Mike Anderson 1 participant 1 message Dec 16, 2004 SCSI

lore
Share & tools
lore
#0 lore raw

Recently the old issue of the status_byte() macro masking off a TASK_ABORTED status was brought to my attention by Martin Peschke and Maxim Shchetynin.

For context here are some old discussions on this issue.
http://marc.theaimsgroup.com/?t=105601234500006&r=1&w=2
http://marc.theaimsgroup.com/?t=105609933000005&r=1&w=2
http://marc.theaimsgroup.com/?t=105304284100002&r=1&w=2
http://marc.theaimsgroup.com/?t=102225346500001&r=1&w=2

Kurt refreshed a patch for 2.6 that increases the mask value for the status_byte macro (provided below). While this stops TASK_ABORTED falling into the GOOD case statement in scsi_decide_disposition without adding TASK_ABORTED to the case statement we will start up the error handler on this command which is probably not what we want.

It was mentioned that masking off the TASK_ABORTED as we do now could lead to a DI issue, but in experimentation with a hacked scsi_debug to return TASK_ABORTED I did not see good status for blk IO through sd. In code review it would appear there possibly could be an issue sg_io passing good_status back to a user.

Example out with hacked scsi_debug fo blk IO through sd:
scsi_debug: cmd 28 00 00 00 12 00 00 02 00 00
scsi_debug: ... <5 0 0 0> non-zero result=0x40 SCSI error : <5 0 0 0> return code = 0x40
end_request: I/O error, dev sdd, sector 4608 Buffer I/O error on device sdd, logical block 576

If not this patch it would seem that we should adopt some solution to not ignore TASK_ABORTED.

-andmike
--
Michael Anderson
[email protected]

include/scsi/scsi.h renamed from include/scsi/scsi.h.orig +4 -2
@@ -169,8 +169,10 @@
 #define RESERVATION_CONFLICT 0x0c
 #define COMMAND_TERMINATED   0x11
 #define QUEUE_FULL           0x14
+#define ACA_ACTIVE           0x18
+#define TASK_ABORTED         0x20
 
-#define STATUS_MASK          0x3e
+#define STATUS_MASK          0xfe
 
 /*
  *  SENSE KEYS
@@ -348,7 +350,7 @@
  *      host_byte   = set by low-level driver to indicate status.
  *      driver_byte = set by mid-level.
  */
-#define status_byte(result) (((result) >> 1) & 0x1f)
+#define status_byte(result) (((result) >> 1) & 0x7f)
 #define msg_byte(result)    (((result) >> 8) & 0xff)
 #define host_byte(result)   (((result) >> 16) & 0xff)
 #define driver_byte(result) (((result) >> 24) & 0xff)