status_byte() and SAM TASK_ABORTED
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 00scsi_debug: ... <5 0 0 0> non-zero result=0x40 SCSI error : <5 0 0 0> return code = 0x40end_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)