On Thu, 2016-02-04 at 15:41 +0100, Hans de Goede wrote:
Hi,
> > @@ -754,9 +798,18 @@ static int uas_eh_abort_handler(struct
> scsi_cmnd *cmnd)
> > if (err < 0) /* unkillable */
> > goto give_up;
> >
> > - wait_for_completion_timeout(&devinfo->deathknell,
> USB_CTRL_GET_TIMEOUT);
> > + time = wait_for_completion_timeout(&devinfo->deathknell,
> USB_CTRL_GET_TIMEOUT);
>
> AFAICT you're still doing this with the uas drivers own spinlock held,
> which is not good.
but this is called directly from the SCSI layer. How can it hold
an internal lock?
> > /* in case of timeout */
> > usb_kill_urb(devinfo->management_urb);
> > + if (time) {
> > + cmdinfo->state &= ~COMMAND_ABORTING;
> > + /*
> > + * manually finish as resources must be freed only
> once
> > + */
> > + cmnd->result = DID_ABORT << 16;
> > + cmnd->scsi_done(cmnd);
> > + }
> > +
>
> Hi, this should probably be done after killing the data urbs, since
> those
> contain a reference to the cmnd, and calling scsi_done returns the
> cmnd to
> the core at which point we should not longer touch it.
>
True. It should happen last in fact.
Regards
Oliver
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
