> -----Original Message-----
> From: Hannes Reinecke [mailto:h...@suse.de]
> Sent: Wednesday, 01 October, 2014 1:23 AM
> To: James Bottomley
> Cc: Christoph Hellwig; linux-scsi@vger.kernel.org; Elliott, Robert (Server
> Storage); Hannes Reinecke
> Subject: [PATCH 24/24] scsi_error: document scsi_try_to_abort_cmd
>
> scsi_try_to_abort_cmd() should only return
> SUCCESS or FAILED. So document that in the
> function description and simplify the
> logging message.
>
> Suggested-by: Christoph Hellwig <h...@infradead.org>
> Cc: Robert Elliott <elli...@hp.com>
> Signed-off-by: Hannes Reinecke <h...@suse.de>
> ---
> drivers/scsi/scsi_error.c | 20 +++++++++++++++++---
> 1 file changed, 17 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
> index a41ef5b..75dd203 100644
> --- a/drivers/scsi/scsi_error.c
> +++ b/drivers/scsi/scsi_error.c
> @@ -157,8 +157,7 @@ scmd_eh_abort_handler(struct work_struct *work)
> } else {
> SCSI_LOG_ERROR_RECOVERY(3,
> scmd_printk(KERN_INFO, scmd,
> - "scmd %p abort failed, rtn %d\n",
> - scmd, rtn));
> + "scmd %p abort failed\n", scmd));
> }
> }
>
> @@ -869,7 +868,22 @@ static int scsi_try_bus_device_reset(struct scsi_cmnd
> *scmd)
> return rtn;
> }
>
> -static int scsi_try_to_abort_cmd(struct scsi_host_template *hostt, struct
> scsi_cmnd *scmd)
> +/**
> + * scsi_try_to_abort_cmd - Ask host to abort a SCSI command
> + * @scmd: SCSI cmd used to send a target reset
> + *
> + * Return value:
> + * SUCCESS or FAILED
> + *
> + * Notes:
> + * SUCCESS does not necessarily indicate that the command
> + * has been aborted; it only indicates that the LLDDs
> + * has cleared all references to that command.
> + * LLDDs should return FAILED only if an abort was required
> + * but could not be executed.
> + */
> +static int scsi_try_to_abort_cmd(struct scsi_host_template *hostt,
> + struct scsi_cmnd *scmd)
> {
> if (!hostt->eh_abort_handler)
> return FAILED;
Since the rest of that function is just:
return hostt->eh_abort_handler(scmd);
this relies on the LLD to use only those values:
#define SUCCESS 0x2002
#define FAILED 0x2003
which is hard to ensure.
Randomly picking on one, megaraid.c registers this
function:
.eh_abort_handler = megaraid_abort,
megaraid_abort returns the return code from
megaraid_abort_and_reset, which returns TRUE or FALSE,
not SUCCESS and FAILED.
Printing the return value helps expose such problems
(if the code path is exercised).
---
Rob Elliott HP Server Storage
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
