On 10/16/2013 09:22 PM, James Bottomley wrote:
On Mon, 2013-09-02 at 13:58 +0200, Hannes Reinecke wrote:Commit 18a4d0a22ed6c54b67af7718c305cd010f09ddf8 (Handle disk devices which can not process medium access commands) was introduced to offline any device which cannot process medium access commands. However, commit 3eef6257de48ff84a5d98ca533685df8a3beaeb8 (Reduce error recovery time by reducing use of TURs) reduced the number of TURs by sending it only on the first failing command, which might or might not be a medium access command. So in combination this results in an erratic device offlining during EH; if the command where the TUR was sent upon happens to be a medium access command the device will be set offline, if not everything proceeds as normal. So instead of checking the EH command in the ->eh_action callback we should rather call ->eh_action when we're about to finish the command _and_ have sent a TUR previously. This should then set the device offline as advertised. Cc: Martin K. Petersen <martin.petersen@xxxxxxxxxx> Cc: Ewan Milne <emilne@xxxxxxxxxx> Signed-off-by: Hannes Reinecke <hare@xxxxxxx> --- drivers/scsi/scsi_error.c | 28 +++++++++++++++++++--------- 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c index abf0916..c88cb7e 100644 --- a/drivers/scsi/scsi_error.c +++ b/drivers/scsi/scsi_error.c @@ -941,12 +941,6 @@ retry: scsi_eh_restore_cmnd(scmd, &ses); - if (scmd->request->cmd_type != REQ_TYPE_BLOCK_PC) { - struct scsi_driver *sdrv = scsi_cmd_to_driver(scmd); - if (sdrv->eh_action) - rtn = sdrv->eh_action(scmd, cmnd, cmnd_size, rtn); - } - return rtn; } @@ -964,6 +958,18 @@ static int scsi_request_sense(struct scsi_cmnd *scmd) return scsi_send_eh_cmnd(scmd, NULL, 0, scmd->device->eh_timeout, ~0); } +static int scsi_eh_action(struct scsi_cmnd *scmd, int rtn) +{ + static unsigned char tur_command[6] = {TEST_UNIT_READY, 0, 0, 0, 0, 0}; + + if (scmd->request->cmd_type != REQ_TYPE_BLOCK_PC) { + struct scsi_driver *sdrv = scsi_cmd_to_driver(scmd); + if (sdrv->eh_action) + rtn = sdrv->eh_action(scmd, tur_command, 6, rtn);This is all a bit pointless. You've altered eh_action so it's always input an eh TUR command, so just eliminate the check of the eh command and assume it's a TUR in the implementation (i.e. fix up sd.c) Once that's done, I think the patch looks like the one below, is that OK?
Yes, the patch looks okay. No objections from my side. Acked-by: Hannes Reinecke <hare@xxxxxxx> Cheers, Hannes -- Dr. Hannes Reinecke zSeries & Storage hare@xxxxxxx +49 911 74053 688 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg) -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
