On Thu, May 04, 2023 at 04:41:11PM +0200, Maxime Bizon wrote:
>
> On Thursday 04 May 2023 à 10:05:38 (-0400), Alan Stern wrote:
>
> > Maxime, would you like to submit a revised version of your patch? The
> > key difference is that it should abort the currently executing command
> > (if there is one), regardless of whether the srb value matches.
>
> Yes
>
> before I do a format submission, is this what you have in mind ?
Yes, except that I would not make command_abort_any() a separate
routine. Just put it inline in device_reset().
Alan Stern
>
> diff --git a/drivers/usb/storage/scsiglue.c b/drivers/usb/storage/scsiglue.c
> index 8931df5a85fd..380b04273969 100644
> --- a/drivers/usb/storage/scsiglue.c
> +++ b/drivers/usb/storage/scsiglue.c
> @@ -406,10 +406,8 @@ static DEF_SCSI_QCMD(queuecommand)
> ***********************************************************************/
>
> /* Command timeout and abort */
> -static int command_abort(struct scsi_cmnd *srb)
> +static int command_abort_matching(struct us_data *us, struct scsi_cmnd *srb_match)
> {
> - struct us_data *us = host_to_us(srb->device->host);
> -
> usb_stor_dbg(us, "%s called\n", __func__);
>
> /*
> @@ -418,10 +416,17 @@ static int command_abort(struct scsi_cmnd *srb)
> */
> scsi_lock(us_to_host(us));
>
> - /* Is this command still active? */
> - if (us->srb != srb) {
> + /* is there any active pending command to abort ? */
> + if (!us->srb) {
> scsi_unlock(us_to_host(us));
> usb_stor_dbg(us, "-- nothing to abort\n");
> + return SUCCESS;
> + }
> +
> + /* Does the command match the passed srb if any ? */
> + if (srb_match && us->srb != srb_match) {
> + scsi_unlock(us_to_host(us));
> + usb_stor_dbg(us, "-- pending command mismatch\n");
> return FAILED;
> }
>
> @@ -444,6 +449,16 @@ static int command_abort(struct scsi_cmnd *srb)
> return SUCCESS;
> }
>
> +static int command_abort_any(struct us_data *us)
> +{
> + return command_abort_matching(us, NULL);
> +}
> +
> +static int command_abort(struct scsi_cmnd *srb)
> +{
> + return command_abort_matching(host_to_us(srb->device->host), srb);
> +}
> +
> /*
> * This invokes the transport reset mechanism to reset the state of the
> * device
> @@ -455,6 +470,8 @@ static int device_reset(struct scsi_cmnd *srb)
>
> usb_stor_dbg(us, "%s called\n", __func__);
>
> + command_abort_any(us);
> +
> /* lock the device pointers and do the reset */
> mutex_lock(&(us->dev_mutex));
> result = us->transport_reset(us);
>
>
> --
> Maxime
