Hi,
On 08-02-16 10:54, Oliver Neukum wrote:
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?
The current code is this:
/*
* For now we do not support actually sending an abort to the device, so
* this eh always fails. Still we must define it to make sure that we've
* dropped all references to the cmnd in question once this function exits.
*/
static int uas_eh_abort_handler(struct scsi_cmnd *cmnd)
{
struct uas_cmd_info *cmdinfo = (void *)&cmnd->SCp;
struct uas_dev_info *devinfo = (void *)cmnd->device->hostdata;
struct urb *data_in_urb = NULL;
struct urb *data_out_urb = NULL;
unsigned long flags;
spin_lock_irqsave(&devinfo->lock, flags);
And AFAICT you're note removing this taking of the 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,
Hans
--
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
