[PATCH 4/4] uas: improve abort handler

2012-11-29 Thread Gerd Hoffmann 
Two changes.  First we check whenever the request is linked in the work
list and if so take it out.  Second check whenever the command is
actually in flight before asking the device to cancel it via task
management, and in case it isn't just zap the data urbs and finish it.

Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
 drivers/usb/storage/uas.c |   18 --
 1 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index dd23b61..5f498db 100644
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -717,8 +717,22 @@ static int uas_eh_abort_handler(struct scsi_cmnd *cmnd)
uas_log_cmd_state(cmnd, __func__);
spin_lock_irqsave(devinfo-lock, flags);
cmdinfo-state |= COMMAND_ABORTED;
-   spin_unlock_irqrestore(devinfo-lock, flags);
-   ret = uas_eh_task_mgmt(cmnd, ABORT TASK, TMF_ABORT_TASK);
+   if (cmdinfo-state  IS_IN_WORK_LIST) {
+   spin_lock_irq(uas_work_lock);
+   list_del(cmdinfo-list);
+   cmdinfo-state = ~IS_IN_WORK_LIST;
+   spin_unlock_irq(uas_work_lock);
+   }
+   if (cmdinfo-state  COMMAND_INFLIGHT) {
+   spin_unlock_irqrestore(devinfo-lock, flags);
+   ret = uas_eh_task_mgmt(cmnd, ABORT TASK, TMF_ABORT_TASK);
+   } else {
+   spin_unlock_irqrestore(devinfo-lock, flags);
+   uas_unlink_data_urbs(devinfo, cmdinfo);
+   spin_lock_irqsave(devinfo-lock, flags);
+   uas_try_complete(cmnd, __func__);
+   spin_unlock_irqrestore(devinfo-lock, flags);
+   }
return ret;
 }
 
-- 
1.7.1

--
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

Re: [PATCH 4/4] uas: improve abort handler

2012-11-29 Thread Oliver Neukum 
On Thursday 29 November 2012 14:06:15 Gerd Hoffmann wrote:
 diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
 index dd23b61..5f498db 100644
 --- a/drivers/usb/storage/uas.c
 +++ b/drivers/usb/storage/uas.c
 @@ -717,8 +717,22 @@ static int uas_eh_abort_handler(struct scsi_cmnd *cmnd)
 uas_log_cmd_state(cmnd, __func__);
 spin_lock_irqsave(devinfo-lock, flags);
 cmdinfo-state |= COMMAND_ABORTED;
 -   spin_unlock_irqrestore(devinfo-lock, flags);
 -   ret = uas_eh_task_mgmt(cmnd, ABORT TASK, TMF_ABORT_TASK);
 +   if (cmdinfo-state  IS_IN_WORK_LIST) {
 +   spin_lock_irq(uas_work_lock);

a) it makes no sense to take the _irq version while you hold an _irqsave
b) are you sure this sequence of locks is safe deadlockwise?

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

Re: [PATCH 4/4] uas: improve abort handler

2012-11-29 Thread Gerd Hoffmann 
On 11/29/12 14:29, Oliver Neukum wrote:
 On Thursday 29 November 2012 14:06:15 Gerd Hoffmann wrote:
 diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
 index dd23b61..5f498db 100644
 --- a/drivers/usb/storage/uas.c
 +++ b/drivers/usb/storage/uas.c
 @@ -717,8 +717,22 @@ static int uas_eh_abort_handler(struct scsi_cmnd *cmnd)
 uas_log_cmd_state(cmnd, __func__);
 spin_lock_irqsave(devinfo-lock, flags);
 cmdinfo-state |= COMMAND_ABORTED;
 -   spin_unlock_irqrestore(devinfo-lock, flags);
 -   ret = uas_eh_task_mgmt(cmnd, ABORT TASK, TMF_ABORT_TASK);
 +   if (cmdinfo-state  IS_IN_WORK_LIST) {
 +   spin_lock_irq(uas_work_lock);
 
 a) it makes no sense to take the _irq version while you hold an _irqsave

Will fix.

 b) are you sure this sequence of locks is safe deadlockwise?

Yes.  No other lock is acquired anywhere while holding uas_work_lock.

cheers,
  Gerd

--
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