]> git.karo-electronics.de Git - karo-tx-linux.git/blobdiff - drivers/usb/storage/uas.c
uas: Remove cmnd reference from the cmd urb
[karo-tx-linux.git] / drivers / usb / storage / uas.c
index 33db53f1ce9ef7b060cf4004efb341327676273b..8421dcd8ed4434e5622d51f006cb2a8876acf0cf 100644 (file)
@@ -61,8 +61,6 @@ struct uas_dev_info {
        struct scsi_cmnd *cmnd[MAX_CMNDS];
        spinlock_t lock;
        struct work_struct work;
-       struct list_head inflight_list;
-       struct list_head dead_list;
 };
 
 enum {
@@ -78,8 +76,7 @@ enum {
        DATA_OUT_URB_INFLIGHT   = (1 << 10),
        COMMAND_COMPLETED       = (1 << 11),
        COMMAND_ABORTED         = (1 << 12),
-       UNLINK_DATA_URBS        = (1 << 13),
-       IS_IN_WORK_LIST         = (1 << 14),
+       IS_IN_WORK_LIST         = (1 << 13),
 };
 
 /* Overrides scsi_pointer */
@@ -89,7 +86,6 @@ struct uas_cmd_info {
        struct urb *cmd_urb;
        struct urb *data_in_urb;
        struct urb *data_out_urb;
-       struct list_head list;
 };
 
 /* I hate forward declarations, but I actually have a loop */
@@ -100,45 +96,26 @@ static int uas_try_complete(struct scsi_cmnd *cmnd, const char *caller);
 static void uas_free_streams(struct uas_dev_info *devinfo);
 static void uas_log_cmd_state(struct scsi_cmnd *cmnd, const char *caller);
 
-/* Must be called with devinfo->lock held, will temporary unlock the lock */
-static void uas_unlink_data_urbs(struct uas_dev_info *devinfo,
-                                struct uas_cmd_info *cmdinfo,
-                                unsigned long *lock_flags)
-{
-       /*
-        * The UNLINK_DATA_URBS flag makes sure uas_try_complete
-        * (called by urb completion) doesn't release cmdinfo
-        * underneath us.
-        */
-       cmdinfo->state |= UNLINK_DATA_URBS;
-       spin_unlock_irqrestore(&devinfo->lock, *lock_flags);
-
-       if (cmdinfo->data_in_urb)
-               usb_unlink_urb(cmdinfo->data_in_urb);
-       if (cmdinfo->data_out_urb)
-               usb_unlink_urb(cmdinfo->data_out_urb);
-
-       spin_lock_irqsave(&devinfo->lock, *lock_flags);
-       cmdinfo->state &= ~UNLINK_DATA_URBS;
-}
-
 static void uas_do_work(struct work_struct *work)
 {
        struct uas_dev_info *devinfo =
                container_of(work, struct uas_dev_info, work);
        struct uas_cmd_info *cmdinfo;
+       struct scsi_cmnd *cmnd;
        unsigned long flags;
-       int err;
+       int i, err;
 
        spin_lock_irqsave(&devinfo->lock, flags);
 
        if (devinfo->resetting)
                goto out;
 
-       list_for_each_entry(cmdinfo, &devinfo->inflight_list, list) {
-               struct scsi_pointer *scp = (void *)cmdinfo;
-               struct scsi_cmnd *cmnd = container_of(scp, struct scsi_cmnd,
-                                                     SCp);
+       for (i = 0; i < devinfo->qdepth; i++) {
+               if (!devinfo->cmnd[i])
+                       continue;
+
+               cmnd = devinfo->cmnd[i];
+               cmdinfo = (void *)&cmnd->SCp;
 
                if (!(cmdinfo->state & IS_IN_WORK_LIST))
                        continue;
@@ -153,35 +130,6 @@ out:
        spin_unlock_irqrestore(&devinfo->lock, flags);
 }
 
-static void uas_mark_cmd_dead(struct uas_dev_info *devinfo,
-                             struct uas_cmd_info *cmdinfo,
-                             int result, const char *caller)
-{
-       struct scsi_pointer *scp = (void *)cmdinfo;
-       struct scsi_cmnd *cmnd = container_of(scp, struct scsi_cmnd, SCp);
-
-       uas_log_cmd_state(cmnd, caller);
-       lockdep_assert_held(&devinfo->lock);
-       WARN_ON_ONCE(cmdinfo->state & COMMAND_ABORTED);
-       cmdinfo->state |= COMMAND_ABORTED;
-       cmdinfo->state &= ~IS_IN_WORK_LIST;
-       cmnd->result = result << 16;
-       list_move_tail(&cmdinfo->list, &devinfo->dead_list);
-}
-
-static void uas_abort_inflight(struct uas_dev_info *devinfo, int result,
-                              const char *caller)
-{
-       struct uas_cmd_info *cmdinfo;
-       struct uas_cmd_info *temp;
-       unsigned long flags;
-
-       spin_lock_irqsave(&devinfo->lock, flags);
-       list_for_each_entry_safe(cmdinfo, temp, &devinfo->inflight_list, list)
-               uas_mark_cmd_dead(devinfo, cmdinfo, result, caller);
-       spin_unlock_irqrestore(&devinfo->lock, flags);
-}
-
 static void uas_add_work(struct uas_cmd_info *cmdinfo)
 {
        struct scsi_pointer *scp = (void *)cmdinfo;
@@ -193,24 +141,26 @@ static void uas_add_work(struct uas_cmd_info *cmdinfo)
        schedule_work(&devinfo->work);
 }
 
-static void uas_zap_dead(struct uas_dev_info *devinfo)
+static void uas_zap_pending(struct uas_dev_info *devinfo, int result)
 {
        struct uas_cmd_info *cmdinfo;
-       struct uas_cmd_info *temp;
+       struct scsi_cmnd *cmnd;
        unsigned long flags;
+       int i, err;
 
        spin_lock_irqsave(&devinfo->lock, flags);
-       list_for_each_entry_safe(cmdinfo, temp, &devinfo->dead_list, list) {
-               struct scsi_pointer *scp = (void *)cmdinfo;
-               struct scsi_cmnd *cmnd = container_of(scp, struct scsi_cmnd,
-                                                     SCp);
+       for (i = 0; i < devinfo->qdepth; i++) {
+               if (!devinfo->cmnd[i])
+                       continue;
+
+               cmnd = devinfo->cmnd[i];
+               cmdinfo = (void *)&cmnd->SCp;
                uas_log_cmd_state(cmnd, __func__);
-               WARN_ON_ONCE(!(cmdinfo->state & COMMAND_ABORTED));
-               /* all urbs are killed, clear inflight bits */
-               cmdinfo->state &= ~(COMMAND_INFLIGHT |
-                                   DATA_IN_URB_INFLIGHT |
-                                   DATA_OUT_URB_INFLIGHT);
-               uas_try_complete(cmnd, __func__);
+               /* Sense urbs were killed, clear COMMAND_INFLIGHT manually */
+               cmdinfo->state &= ~COMMAND_INFLIGHT;
+               cmnd->result = result << 16;
+               err = uas_try_complete(cmnd, __func__);
+               WARN_ON(err != 0);
        }
        spin_unlock_irqrestore(&devinfo->lock, flags);
 }
@@ -281,8 +231,8 @@ static void uas_log_cmd_state(struct scsi_cmnd *cmnd, const char *caller)
 {
        struct uas_cmd_info *ci = (void *)&cmnd->SCp;
 
-       scmd_printk(KERN_INFO, cmnd, "%s %p tag %d, inflight:"
-                   "%s%s%s%s%s%s%s%s%s%s%s%s%s%s\n",
+       scmd_printk(KERN_INFO, cmnd,
+                   "%s %p tag %d, inflight:%s%s%s%s%s%s%s%s%s%s%s%s%s\n",
                    caller, cmnd, uas_get_tag(cmnd),
                    (ci->state & SUBMIT_STATUS_URB)     ? " s-st"  : "",
                    (ci->state & ALLOC_DATA_IN_URB)     ? " a-in"  : "",
@@ -296,7 +246,6 @@ static void uas_log_cmd_state(struct scsi_cmnd *cmnd, const char *caller)
                    (ci->state & DATA_OUT_URB_INFLIGHT) ? " OUT"   : "",
                    (ci->state & COMMAND_COMPLETED)     ? " done"  : "",
                    (ci->state & COMMAND_ABORTED)       ? " abort" : "",
-                   (ci->state & UNLINK_DATA_URBS)      ? " unlink": "",
                    (ci->state & IS_IN_WORK_LIST)       ? " work"  : "");
 }
 
@@ -308,16 +257,12 @@ static int uas_try_complete(struct scsi_cmnd *cmnd, const char *caller)
        lockdep_assert_held(&devinfo->lock);
        if (cmdinfo->state & (COMMAND_INFLIGHT |
                              DATA_IN_URB_INFLIGHT |
-                             DATA_OUT_URB_INFLIGHT |
-                             UNLINK_DATA_URBS))
+                             DATA_OUT_URB_INFLIGHT))
                return -EBUSY;
        WARN_ON_ONCE(cmdinfo->state & COMMAND_COMPLETED);
        cmdinfo->state |= COMMAND_COMPLETED;
-       usb_free_urb(cmdinfo->data_in_urb);
-       usb_free_urb(cmdinfo->data_out_urb);
        if (cmdinfo->state & COMMAND_ABORTED)
                scmd_printk(KERN_INFO, cmnd, "abort completed\n");
-       list_del(&cmdinfo->list);
        devinfo->cmnd[uas_get_tag(cmnd) - 1] = NULL;
        cmnd->scsi_done(cmnd);
        return 0;
@@ -341,6 +286,8 @@ static void uas_stat_cmplt(struct urb *urb)
        struct iu *iu = urb->transfer_buffer;
        struct Scsi_Host *shost = urb->context;
        struct uas_dev_info *devinfo = (struct uas_dev_info *)shost->hostdata;
+       struct urb *data_in_urb = NULL;
+       struct urb *data_out_urb = NULL;
        struct scsi_cmnd *cmnd;
        struct uas_cmd_info *cmdinfo;
        unsigned long flags;
@@ -371,6 +318,12 @@ static void uas_stat_cmplt(struct urb *urb)
 
        cmnd = devinfo->cmnd[idx];
        cmdinfo = (void *)&cmnd->SCp;
+
+       if (!(cmdinfo->state & COMMAND_INFLIGHT)) {
+               scmd_printk(KERN_ERR, cmnd, "unexpected status cmplt\n");
+               goto out;
+       }
+
        switch (iu->iu_id) {
        case IU_ID_STATUS:
                if (urb->actual_length < 16)
@@ -381,7 +334,8 @@ static void uas_stat_cmplt(struct urb *urb)
                        uas_sense(urb, cmnd);
                if (cmnd->result != 0) {
                        /* cancel data transfers on error */
-                       uas_unlink_data_urbs(devinfo, cmdinfo, &flags);
+                       data_in_urb = usb_get_urb(cmdinfo->data_in_urb);
+                       data_out_urb = usb_get_urb(cmdinfo->data_out_urb);
                }
                cmdinfo->state &= ~COMMAND_INFLIGHT;
                uas_try_complete(cmnd, __func__);
@@ -409,6 +363,16 @@ static void uas_stat_cmplt(struct urb *urb)
 out:
        usb_free_urb(urb);
        spin_unlock_irqrestore(&devinfo->lock, flags);
+
+       /* Unlinking of data urbs must be done without holding the lock */
+       if (data_in_urb) {
+               usb_unlink_urb(data_in_urb);
+               usb_put_urb(data_in_urb);
+       }
+       if (data_out_urb) {
+               usb_unlink_urb(data_out_urb);
+               usb_put_urb(data_out_urb);
+       }
 }
 
 static void uas_data_cmplt(struct urb *urb)
@@ -424,9 +388,11 @@ static void uas_data_cmplt(struct urb *urb)
        if (cmdinfo->data_in_urb == urb) {
                sdb = scsi_in(cmnd);
                cmdinfo->state &= ~DATA_IN_URB_INFLIGHT;
+               cmdinfo->data_in_urb = NULL;
        } else if (cmdinfo->data_out_urb == urb) {
                sdb = scsi_out(cmnd);
                cmdinfo->state &= ~DATA_OUT_URB_INFLIGHT;
+               cmdinfo->data_out_urb = NULL;
        }
        if (sdb == NULL) {
                WARN_ON_ONCE(1);
@@ -436,6 +402,12 @@ static void uas_data_cmplt(struct urb *urb)
        if (devinfo->resetting)
                goto out;
 
+       /* Data urbs should not complete before the cmd urb is submitted */
+       if (cmdinfo->state & SUBMIT_CMD_URB) {
+               scmd_printk(KERN_ERR, cmnd, "unexpected data cmplt\n");
+               goto out;
+       }
+
        if (urb->status) {
                if (urb->status != -ECONNRESET) {
                        uas_log_cmd_state(cmnd, __func__);
@@ -450,17 +422,15 @@ static void uas_data_cmplt(struct urb *urb)
        }
        uas_try_complete(cmnd, __func__);
 out:
+       usb_free_urb(urb);
        spin_unlock_irqrestore(&devinfo->lock, flags);
 }
 
 static void uas_cmd_cmplt(struct urb *urb)
 {
-       struct scsi_cmnd *cmnd = urb->context;
+       if (urb->status)
+               dev_err(&urb->dev->dev, "cmd cmplt err %d\n", urb->status);
 
-       if (urb->status) {
-               uas_log_cmd_state(cmnd, __func__);
-               scmd_printk(KERN_ERR, cmnd, "cmd cmplt err %d\n", urb->status);
-       }
        usb_free_urb(urb);
 }
 
@@ -538,7 +508,7 @@ static struct urb *uas_alloc_cmd_urb(struct uas_dev_info *devinfo, gfp_t gfp,
        memcpy(iu->cdb, cmnd->cmnd, cmnd->cmd_len);
 
        usb_fill_bulk_urb(urb, udev, devinfo->cmd_pipe, iu, sizeof(*iu) + len,
-                                                       uas_cmd_cmplt, cmnd);
+                                                       uas_cmd_cmplt, NULL);
        urb->transfer_flags |= URB_FREE_BUFFER;
  out:
        return urb;
@@ -736,7 +706,6 @@ static int uas_queuecommand_lck(struct scsi_cmnd *cmnd,
        }
 
        devinfo->cmnd[stream - 1] = cmnd;
-       list_add_tail(&cmdinfo->list, &devinfo->inflight_list);
        spin_unlock_irqrestore(&devinfo->lock, flags);
        return 0;
 }
@@ -764,11 +733,11 @@ static int uas_eh_bus_reset_handler(struct scsi_cmnd *cmnd)
        devinfo->resetting = 1;
        spin_unlock_irqrestore(&devinfo->lock, flags);
 
-       uas_abort_inflight(devinfo, DID_RESET, __func__);
        usb_kill_anchored_urbs(&devinfo->cmd_urbs);
        usb_kill_anchored_urbs(&devinfo->sense_urbs);
        usb_kill_anchored_urbs(&devinfo->data_urbs);
-       uas_zap_dead(devinfo);
+       uas_zap_pending(devinfo, DID_RESET);
+
        err = usb_reset_device(udev);
 
        spin_lock_irqsave(&devinfo->lock, flags);
@@ -950,8 +919,6 @@ static int uas_probe(struct usb_interface *intf, const struct usb_device_id *id)
        init_usb_anchor(&devinfo->data_urbs);
        spin_lock_init(&devinfo->lock);
        INIT_WORK(&devinfo->work, uas_do_work);
-       INIT_LIST_HEAD(&devinfo->inflight_list);
-       INIT_LIST_HEAD(&devinfo->dead_list);
 
        result = uas_configure_endpoints(devinfo);
        if (result)
@@ -1079,11 +1046,11 @@ static void uas_disconnect(struct usb_interface *intf)
        spin_unlock_irqrestore(&devinfo->lock, flags);
 
        cancel_work_sync(&devinfo->work);
-       uas_abort_inflight(devinfo, DID_NO_CONNECT, __func__);
        usb_kill_anchored_urbs(&devinfo->cmd_urbs);
        usb_kill_anchored_urbs(&devinfo->sense_urbs);
        usb_kill_anchored_urbs(&devinfo->data_urbs);
-       uas_zap_dead(devinfo);
+       uas_zap_pending(devinfo, DID_NO_CONNECT);
+
        scsi_remove_host(shost);
        uas_free_streams(devinfo);
        scsi_host_put(shost);