> -----Original Message----- > From: Hannes Reinecke [mailto:hare@xxxxxxx] > Sent: Wednesday, 01 October, 2014 1:23 AM > To: James Bottomley > Cc: Christoph Hellwig; linux-scsi@xxxxxxxxxxxxxxx; 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 <hch@xxxxxxxxxxxxx> > Cc: Robert Elliott <elliott@xxxxxx> > Signed-off-by: Hannes Reinecke <hare@xxxxxxx> > --- > 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 majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
