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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to