James, I fixed the things you pointed out on the previous review. As discussed, I didn't change the code to reuse the request yet. We can follow up on that later.
Thanks, >8 Usually, re-sending the SCSI command is enough to recover from a Unit Attention (UA). This adds a generic retry code to the SCSI command path in case of an UA, before giving up and returning the error condition to the caller. I added the UA verification into scsi_execute instead of scsi_execute_req because there are at least a few callers that invoke scsi_execute directly and would benefit from the internal UA retry. Also, I didn't use scsi_normalize_sense to not duplicate functionality with scsi_execute_req_flags. Instead, scsi_execute uses a small helper function that verifies only the UA condition directly from the raw sense buffer. If this design is not OK, I can refactor to use scsi_normalize_sense. This prevents us from duplicating the retry code in at least a few places. In particular, it fixes an issue found in some IBM enclosures, in which the device may return an Unit Attention during probe, breaking the bind with the ses module: scsi 1:0:7:0: Failed to get diagnostic page 0x8000002 scsi 1:0:7:0: Failed to bind enclosure -19 Link: https://patchwork.kernel.org/patch/9336763/ Suggested-by: Brian King <[email protected]> Suggested-by: James Bottomley <[email protected]> Signed-off-by: Gabriel Krisman Bertazi <[email protected]> --- drivers/scsi/scsi_lib.c | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c index c71344aebdbb..4f6a91d6ee34 100644 --- a/drivers/scsi/scsi_lib.c +++ b/drivers/scsi/scsi_lib.c @@ -163,6 +163,16 @@ void scsi_queue_insert(struct scsi_cmnd *cmd, int reason) { __scsi_queue_insert(cmd, reason, 1); } + +static inline bool scsi_sense_unit_attention(const char *sense) +{ + int resp = sense[0] & 0x7f; + + return ((resp & 0x70) && + ((resp >= 0x72 && (sense[1] & 0xf) == UNIT_ATTENTION) || + (resp < 0x72 && (sense[2] & 0xf) == UNIT_ATTENTION))); +} + /** * scsi_execute - insert request and wait for the result * @sdev: scsi device @@ -187,7 +197,14 @@ int scsi_execute(struct scsi_device *sdev, const unsigned char *cmd, struct request *req; int write = (data_direction == DMA_TO_DEVICE); int ret = DRIVER_ERROR << 24; + unsigned char sense_buf[SCSI_SENSE_BUFFERSIZE]; + if (!sense) { + sense = sense_buf; + memset(sense, 0, SCSI_SENSE_BUFFERSIZE); + } + + retry: req = blk_get_request(sdev->request_queue, write, __GFP_RECLAIM); if (IS_ERR(req)) return ret; @@ -210,6 +227,13 @@ int scsi_execute(struct scsi_device *sdev, const unsigned char *cmd, */ blk_execute_rq(req->q, NULL, req, 1); + if (scsi_sense_unit_attention(sense) && req->retries > 0) { + memset(sense, 0, SCSI_SENSE_BUFFERSIZE); + retries = req->retries - 1; + blk_put_request(req); + goto retry; + } + /* * Some devices (USB mass-storage in particular) may transfer * garbage data together with a residue indicating that the data -- 2.7.4 -- 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
