scsi: Fix failed request error code
Subsystems (2)
PATCH v2 1/1 scsi: Fix failed request error code
With the introduction of commit e39a97353e53 ("scsi: core: return BLK_STS_OK for DID_OK in __scsi_error_from_host_byte()"), a command that failed with hostbyte=DID_OK and driverbyte=DRIVER_SENSE but lacking additional sense information will have a return code set to BLK_STS_OK. This results in the request issuer to see successful request execution despite the failure. An example of such case is an unaligned write on a host managed ZAC disk connected to a SAS HBA with a malfunctioning SAT. The unaligned write command gets aborted but has no additional sense information.
sd 10:0:0:0: [sde] tag#3905 FAILED Result: hostbyte=DID_OK driverbyte=DRIVER_SENSE sd 10:0:0:0: [sde] tag#3905 Sense Key : Aborted Command [current]
sd 10:0:0:0: [sde] tag#3905 Add. Sense: No additional sense information
sd 10:0:0:0: [sde] tag#3905 CDB: Write(16) 8a 00 00 00 00 00 02 0c 00 01 00 00 00 01 00 00print_req_error: I/O error, dev sde, sector 274726920
In scsi_io_completion(), sense key handling to not change the request error code and success being reported to the issuer.
Fix this by making sure that the error code always indicates an error if scsi_io_completion() decide that the action to be taken for a failed command is to not retry it and terminate it immediately (ACTION_FAIL) .
--- drivers/scsi/scsi_lib.c | 6 ++++++ 1 file changed, 6 insertions(+)
drivers/scsi/scsi_lib.c +6 -0
@@ -905,6 +905,12 @@ void scsi_io_completion(struct scsi_cmnd *cmd, unsigned int good_bytes) goto requeue; error =__scsi_error_from_host_byte (cmd, result); +/* +* If the hostbyte was DID_OK, but the sense code is valid +* we always should set BLK_STS_IOERR. +*/ +if (error ==BLK_STS_OK && sense_valid) + error =BLK_STS_IOERR ;if (host_byte (result) ==DID_RESET ) {/* Third party bus reset or reset for error recovery
Signature · 1 line
2.12.3
Quoted 7 lines · Hannes Reinecke
drivers/scsi/scsi_lib.c +0
-0@@ -905,6 +905,12 @@ void scsi_io_completion(struct scsi_cmnd *cmd, unsigned int good_bytes) goto requeue;
drivers/scsi/scsi_lib.c void scsi_io_completion(struct scsi_cmnd *cmd, unsigned int good_bytes) · line 916
Quoted 10 lines
drivers/scsi/scsi_lib.c +6
-0error =__scsi_error_from_host_byte (cmd, result); +/* +* If the hostbyte was DID_OK, but the sense code is valid +* we always should set BLK_STS_IOERR. +*/ +if (error ==BLK_STS_OK && sense_valid) + error =BLK_STS_IOERR ;if (host_byte (result) ==DID_RESET ) {/* Third party bus reset or reset for error recovery
__scsi_error_from_host_byte() has two callers. Why does this patch only update one of these two callers?
Regarding commit e39a97353e53, the description of that commit is as follows:
"When converting __scsi_error_from_host_byte() to BLK_STS error codes the case DID_OK was forgotten, resulting in it always returning an error." However, the comment above that function reads as follows: "translate SCSI error code into errno". If I have a look at the v4.12 SCSI core (before the blkstatus_t conversion) then I see that __scsi_error_from_host_byte() never returns 0. That means that the description of commit e39a97353e53 ("scsi:
core: return BLK_STS_OK for DID_OK in __scsi_error_from_host_byte()") is wrong. Does that mean that commit e39a97353e53 should be reverted?
Thanks,
Bart.
Quoted 36 lines · Hannes Reinecke
With the introduction of commit
e39a97353e53("scsi: core: returnBLK_STS_OKforDID_OKin__scsi_error_from_host_byte()"), a command that failed with hostbyte=DID_OK and driverbyte=DRIVER_SENSE but lacking additional sense information will have a return code set toBLK_STS_OK. This results in the request issuer to see successful request execution despite the failure. An example of such case is an unaligned write on a host managed ZAC disk connected to a SAS HBA with a malfunctioning SAT. The unaligned write command gets aborted but has no additional sense information.sd 10:0:0:0: [sde] tag#3905 FAILED Result: hostbyte=DID_OK driverbyte=DRIVER_SENSE sd 10:0:0:0: [sde] tag#3905 Sense Key : Aborted Command [current]
sd 10:0:0:0: [sde] tag#3905 Add. Sense: No additional sense information
sd 10:0:0:0: [sde] tag#3905 CDB: Write(16) 8a 00 00 00 00 00 02 0c 00 01 00 00 00 01 00 00print_req_error: I/O error, dev sde, sector 274726920In
scsi_io_completion(), sense key handling to not change the request error code and success being reported to the issuer.Fix this by making sure that the error code always indicates an error if
scsi_io_completion()decide that the action to be taken for a failed command is to not retry it and terminate it immediately (ACTION_FAIL) .Signed-off-by Damien Le Moal <[email protected]>Signed-off-by Hannes Reinecke <[email protected]>Fixes e39a97353e53 ("scsi: core: return BLK_STS_OK for DID_OK in __scsi_error_from_host_byte()")Cc <[email protected]>--- drivers/scsi/scsi_lib.c | 6 ++++++ 1 file changed, 6 insertions(+)drivers/scsi/scsi_lib.c +0
-0
drivers/scsi/scsi_lib.c void scsi_io_completion(struct scsi_cmnd *cmd, unsigned int good_bytes) · line 912
Quoted 8 lines
drivers/scsi/scsi_lib.c +5
-0@@ -905,6 +905,12 @@ void scsi_io_completion(struct scsi_cmnd *cmd, unsigned int good_bytes) goto requeue; error =__scsi_error_from_host_byte (cmd, result); +/* +* If the hostbyte was DID_OK, but the sense code is valid +* we always should set BLK_STS_IOERR. +*/ +if (error ==BLK_STS_OK && sense_valid)
&& !sense_deferred
Doug Gilbert
Quoted 5 lines
drivers/scsi/scsi_lib.c +1
-0+ error =BLK_STS_IOERR ;if (host_byte (result) ==DID_RESET ) {/* Third party bus reset or reset for error recovery