| Message ID | 20221206131346.2045375-1-niklas.cassel@wdc.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [v2] scsi_error: do not queue pointless abort workqueue functions | expand |
On 06/12/2022 13:13, Niklas Cassel wrote: > From: Hannes Reinecke <hare@suse.de> > > If a host template doesn't implement the .eh_abort_handler() > there is no point in queueing the abort workqueue function; > all it does is invoking SCSI EH anyway. > So return 'FAILED' from scsi_abort_command() if the .eh_abort_handler() > is not implemented and save us from having to wait for the > abort workqueue function to complete. > > Cc: Niklas Cassel <niklas.cassel@wdc.com> > Cc: Damien Le Moal <damien.lemoal@opensource.wdc.com> > Cc: John Garry <john.g.garry@oracle.com> > Signed-off-by: Hannes Reinecke <hare@suse.de> > [niklas: moved the check to the top of scsi_abort_command()] > Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com> FWIW, Reviewed-by: John Garry <john.g.garry@oracle.com> > --- > Changes since v1: > -moved the check to the top of scsi_abort_command(), as there is no need to > perform the SCSI_EH_ABORT_SCHEDULED check if there is no eh_abort_handler. > > I know that John gave a review comment on V1 that it is possible to not > allocate the shost->tmf_work_q in case there is no eh_abort_handler, > however, that is more of a micro optimization. I'd say that would be a separate change. > This patch significantly > reduces the latency before SCSI EH is invoked for libata. > > It would be nice if we could get this patched merged for 6.2, for which > the merge window opens soon. > > drivers/scsi/scsi_error.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c > index a7960ad2d386..2aa2c2aee6e7 100644 > --- a/drivers/scsi/scsi_error.c > +++ b/drivers/scsi/scsi_error.c > @@ -231,6 +231,11 @@ scsi_abort_command(struct scsi_cmnd *scmd) > struct Scsi_Host *shost = sdev->host; > unsigned long flags; > > + if (!shost->hostt->eh_abort_handler) { > + /* No abort handler, fail command directly */ > + return FAILED; > + } > + > if (scmd->eh_eflags & SCSI_EH_ABORT_SCHEDULED) { > /* > * Retry after abort failed, escalate to next level.
Niklas, > If a host template doesn't implement the .eh_abort_handler() there is > no point in queueing the abort workqueue function; all it does is > invoking SCSI EH anyway. So return 'FAILED' from scsi_abort_command() > if the .eh_abort_handler() is not implemented and save us from having > to wait for the abort workqueue function to complete. Applied to 6.2/scsi-staging, thanks!
diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c index a7960ad2d386..2aa2c2aee6e7 100644 --- a/drivers/scsi/scsi_error.c +++ b/drivers/scsi/scsi_error.c @@ -231,6 +231,11 @@ scsi_abort_command(struct scsi_cmnd *scmd) struct Scsi_Host *shost = sdev->host; unsigned long flags; + if (!shost->hostt->eh_abort_handler) { + /* No abort handler, fail command directly */ + return FAILED; + } + if (scmd->eh_eflags & SCSI_EH_ABORT_SCHEDULED) { /* * Retry after abort failed, escalate to next level.