Re: [PATCH v2 2/6] qla2xxx: Simplify the code for aborting SCSI commands

Daniel Wagner
2020-01-23T10:17:34Z

On Wed, Jan 22, 2020 at 08:23:41PM -0800, Bart Van Assche wrote:
> Since the SCSI core does not reuse the tag of the SCSI command that is
> being aborted by .eh_abort() before .eh_abort() has finished it is not
> necessary to check from inside that callback whether or not the SCSI command
> has already completed. Instead, rely on the firmware to return an error code
> when attempting to abort a command that has already completed. Additionally,
> rely on the firmware to return an error code when attempting to abort an
> already aborted command.
>
> In qla2x00_abort_srb(), use blk_mq_request_started() instead of
> sp->completed and sp->aborted.
>
> This patch eliminates several race conditions triggered by the removed member
> variables.

I can only guess here what the races are but I agree removing the
logic here and relying on the SCSI layer to handle it correctly makes
sense.

> Acked-by: Himanshu Madhani <hmadhani@marvell.com>
> Reviewed-by: Roman Bolshakov <r.bolshakov@yadro.com>
> Cc: Quinn Tran <qutran@marvell.com>
> Cc: Martin Wilck <mwilck@suse.com>
> Cc: Daniel Wagner <dwagner@suse.de>
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>

Reviewed-by: Daniel Wagner <dwagner@suse.de>

> +/*
> + * The caller must ensure that no completion interrupts will happen
> + * while this function is in progress.
> + */

So could we add something like WARN_ON(irqs_disabled())?