From ab945eff8396bc3329cc97274320e8d2c6585077 Mon Sep 17 00:00:00 2001 From: Sanjeev Sharma Date: Tue, 12 Aug 2014 12:10:21 +0530 Subject: uas: replace WARN_ON_ONCE() with lockdep_assert_held() on some architecture spin_is_locked() always return false in uniprocessor configuration and therefore it would be advise to replace with lockdep_assert_held(). Signed-off-by: Sanjeev Sharma Acked-by: Hans de Goede Signed-off-by: Greg Kroah-Hartman --- drivers/usb/storage/uas.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'drivers/usb/storage') diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index 3f42785f653c..05b2d8e077d9 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -154,7 +154,7 @@ static void uas_mark_cmd_dead(struct uas_dev_info *devinfo, struct scsi_cmnd *cmnd = container_of(scp, struct scsi_cmnd, SCp); uas_log_cmd_state(cmnd, caller); - WARN_ON_ONCE(!spin_is_locked(&devinfo->lock)); + lockdep_assert_held(&devinfo->lock); WARN_ON_ONCE(cmdinfo->state & COMMAND_ABORTED); cmdinfo->state |= COMMAND_ABORTED; cmdinfo->state &= ~IS_IN_WORK_LIST; @@ -181,7 +181,7 @@ static void uas_add_work(struct uas_cmd_info *cmdinfo) struct scsi_cmnd *cmnd = container_of(scp, struct scsi_cmnd, SCp); struct uas_dev_info *devinfo = cmnd->device->hostdata; - WARN_ON_ONCE(!spin_is_locked(&devinfo->lock)); + lockdep_assert_held(&devinfo->lock); cmdinfo->state |= IS_IN_WORK_LIST; schedule_work(&devinfo->work); } @@ -283,7 +283,7 @@ static int uas_try_complete(struct scsi_cmnd *cmnd, const char *caller) struct uas_cmd_info *cmdinfo = (void *)&cmnd->SCp; struct uas_dev_info *devinfo = (void *)cmnd->device->hostdata; - WARN_ON_ONCE(!spin_is_locked(&devinfo->lock)); + lockdep_assert_held(&devinfo->lock); if (cmdinfo->state & (COMMAND_INFLIGHT | DATA_IN_URB_INFLIGHT | DATA_OUT_URB_INFLIGHT | @@ -622,7 +622,7 @@ static int uas_submit_urbs(struct scsi_cmnd *cmnd, struct urb *urb; int err; - WARN_ON_ONCE(!spin_is_locked(&devinfo->lock)); + lockdep_assert_held(&devinfo->lock); if (cmdinfo->state & SUBMIT_STATUS_URB) { urb = uas_submit_sense_urb(cmnd, gfp, cmdinfo->stream); if (!urb) -- cgit v1.2.3 From 593078525c8b234a35a36ff551b8716464e86481 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Mon, 15 Sep 2014 16:04:12 +0200 Subject: uas: Add a quirk for rejecting ATA_12 and ATA_16 commands And set this quirk for the Seagate Expansion Desk (0bc2:2312), as that one seems to hang upon receiving an ATA_12 or ATA_16 command. https://bugzilla.kernel.org/show_bug.cgi?id=79511 https://bbs.archlinux.org/viewtopic.php?id=183190 While at it also add missing documentation for the u value for usb-storage quirks. Cc: stable@vger.kernel.org # 3.16, 3.17 Signed-off-by: Hans de Goede -- Changes in v2: Add documentation for new t and u usb-storage.quirks flags Changes in v3: Fix typo in documentation Changes in v4: Also apply the quirk to (0bc2:3312) Changes in v5: Rebased on 3.17-rc5, drop u documentation, already upstream Signed-off-by: Greg Kroah-Hartman --- Documentation/kernel-parameters.txt | 2 ++ drivers/usb/storage/uas.c | 13 +++++++++++++ drivers/usb/storage/unusual_uas.h | 23 +++++++++++++---------- drivers/usb/storage/usb.c | 6 +++++- include/linux/usb_usual.h | 2 ++ 5 files changed, 35 insertions(+), 11 deletions(-) (limited to 'drivers/usb/storage') diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt index 10d51c2f10d7..dd4fe9880e4a 100644 --- a/Documentation/kernel-parameters.txt +++ b/Documentation/kernel-parameters.txt @@ -3541,6 +3541,8 @@ bytes respectively. Such letter suffixes can also be entirely omitted. bogus residue values); s = SINGLE_LUN (the device has only one Logical Unit); + t = NO_ATA_1X (don't allow ATA(12) and ATA(16) + commands, uas only); u = IGNORE_UAS (don't bind to the uas driver); w = NO_WP_DETECT (don't test whether the medium is write-protected). diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index 05b2d8e077d9..8c7d4a239f4c 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -28,6 +28,7 @@ #include #include "uas-detect.h" +#include "scsiglue.h" /* * The r00-r01c specs define this version of the SENSE IU data structure. @@ -49,6 +50,7 @@ struct uas_dev_info { struct usb_anchor cmd_urbs; struct usb_anchor sense_urbs; struct usb_anchor data_urbs; + unsigned long flags; int qdepth, resetting; struct response_iu response; unsigned cmd_pipe, status_pipe, data_in_pipe, data_out_pipe; @@ -714,6 +716,15 @@ static int uas_queuecommand_lck(struct scsi_cmnd *cmnd, BUILD_BUG_ON(sizeof(struct uas_cmd_info) > sizeof(struct scsi_pointer)); + if ((devinfo->flags & US_FL_NO_ATA_1X) && + (cmnd->cmnd[0] == ATA_12 || cmnd->cmnd[0] == ATA_16)) { + memcpy(cmnd->sense_buffer, usb_stor_sense_invalidCDB, + sizeof(usb_stor_sense_invalidCDB)); + cmnd->result = SAM_STAT_CHECK_CONDITION; + cmnd->scsi_done(cmnd); + return 0; + } + spin_lock_irqsave(&devinfo->lock, flags); if (devinfo->resetting) { @@ -1080,6 +1091,8 @@ static int uas_probe(struct usb_interface *intf, const struct usb_device_id *id) devinfo->resetting = 0; devinfo->running_task = 0; devinfo->shutdown = 0; + devinfo->flags = id->driver_info; + usb_stor_adjust_quirks(udev, &devinfo->flags); init_usb_anchor(&devinfo->cmd_urbs); init_usb_anchor(&devinfo->sense_urbs); init_usb_anchor(&devinfo->data_urbs); diff --git a/drivers/usb/storage/unusual_uas.h b/drivers/usb/storage/unusual_uas.h index 7244444df8ee..3ff2dd4c78ca 100644 --- a/drivers/usb/storage/unusual_uas.h +++ b/drivers/usb/storage/unusual_uas.h @@ -40,13 +40,16 @@ * and don't forget to CC: the USB development list */ -/* - * This is an example entry for the US_FL_IGNORE_UAS flag. Once we have an - * actual entry using US_FL_IGNORE_UAS this entry should be removed. - * - * UNUSUAL_DEV( 0xabcd, 0x1234, 0x0100, 0x0100, - * "Example", - * "Storage with broken UAS", - * USB_SC_DEVICE, USB_PR_DEVICE, NULL, - * US_FL_IGNORE_UAS), - */ +/* https://bugzilla.kernel.org/show_bug.cgi?id=79511 */ +UNUSUAL_DEV(0x0bc2, 0x2312, 0x0000, 0x9999, + "Seagate", + "Expansion Desk", + USB_SC_DEVICE, USB_PR_DEVICE, NULL, + US_FL_NO_ATA_1X), + +/* https://bbs.archlinux.org/viewtopic.php?id=183190 */ +UNUSUAL_DEV(0x0bc2, 0x3312, 0x0000, 0x9999, + "Seagate", + "Expansion Desk", + USB_SC_DEVICE, USB_PR_DEVICE, NULL, + US_FL_NO_ATA_1X), diff --git a/drivers/usb/storage/usb.c b/drivers/usb/storage/usb.c index cedb29252a92..b9d1b9357287 100644 --- a/drivers/usb/storage/usb.c +++ b/drivers/usb/storage/usb.c @@ -478,7 +478,8 @@ void usb_stor_adjust_quirks(struct usb_device *udev, unsigned long *fflags) US_FL_CAPACITY_OK | US_FL_IGNORE_RESIDUE | US_FL_SINGLE_LUN | US_FL_NO_WP_DETECT | US_FL_NO_READ_DISC_INFO | US_FL_NO_READ_CAPACITY_16 | - US_FL_INITIAL_READ10 | US_FL_WRITE_CACHE); + US_FL_INITIAL_READ10 | US_FL_WRITE_CACHE | + US_FL_NO_ATA_1X); p = quirks; while (*p) { @@ -543,6 +544,9 @@ void usb_stor_adjust_quirks(struct usb_device *udev, unsigned long *fflags) case 's': f |= US_FL_SINGLE_LUN; break; + case 't': + f |= US_FL_NO_ATA_1X; + break; case 'u': f |= US_FL_IGNORE_UAS; break; diff --git a/include/linux/usb_usual.h b/include/linux/usb_usual.h index 9b7de1b46437..d271f88f30ad 100644 --- a/include/linux/usb_usual.h +++ b/include/linux/usb_usual.h @@ -73,6 +73,8 @@ /* Device advertises UAS but it is broken */ \ US_FLAG(BROKEN_FUA, 0x01000000) \ /* Cannot handle FUA in WRITE or READ CDBs */ \ + US_FLAG(NO_ATA_1X, 0x02000000) \ + /* Cannot handle ATA_12 or ATA_16 CDBs */ \ #define US_FLAG(name, value) US_FL_##name = value , enum { US_DO_ALL_FLAGS }; -- cgit v1.2.3 From 734016b00b50a3c6a0e1fc1b7b217e783f5123a1 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Tue, 16 Sep 2014 18:36:52 +0200 Subject: uas: Add no-report-opcodes quirk Besides the ASM1051 (*) needing sdev->no_report_opcodes = 1, it turns out that the JMicron JMS567 also needs it to work properly with uas (usb-storage always sets it). Since some of the scsi devs were not to keen on the idea to outrightly set sdev->no_report_opcodes = 1 for all uas devices, so add a quirk for this, and set it for the JMS567. *) Which has become a non-issue since we've completely blacklisted uas on the ASM1051 for other reasons Cc: stable@vger.kernel.org Reported-and-tested-by: Claudio Bizzarri Signed-off-by: Hans de Goede Signed-off-by: Greg Kroah-Hartman --- Documentation/kernel-parameters.txt | 2 ++ drivers/usb/storage/uas.c | 4 ++++ drivers/usb/storage/unusual_uas.h | 7 +++++++ drivers/usb/storage/usb.c | 5 ++++- include/linux/usb_usual.h | 2 ++ 5 files changed, 19 insertions(+), 1 deletion(-) (limited to 'drivers/usb/storage') diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt index dd4fe9880e4a..1edd5fdc629d 100644 --- a/Documentation/kernel-parameters.txt +++ b/Documentation/kernel-parameters.txt @@ -3522,6 +3522,8 @@ bytes respectively. Such letter suffixes can also be entirely omitted. READ_DISC_INFO command); e = NO_READ_CAPACITY_16 (don't use READ_CAPACITY_16 command); + f = NO_REPORT_OPCODES (don't use report opcodes + command, uas only); h = CAPACITY_HEURISTICS (decrease the reported device capacity by one sector if the number is odd); diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index 8c7d4a239f4c..cad02ac88564 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -961,6 +961,10 @@ static int uas_slave_alloc(struct scsi_device *sdev) static int uas_slave_configure(struct scsi_device *sdev) { struct uas_dev_info *devinfo = sdev->hostdata; + + if (devinfo->flags & US_FL_NO_REPORT_OPCODES) + sdev->no_report_opcodes = 1; + scsi_set_tag_type(sdev, MSG_ORDERED_TAG); scsi_activate_tcq(sdev, devinfo->qdepth - 2); return 0; diff --git a/drivers/usb/storage/unusual_uas.h b/drivers/usb/storage/unusual_uas.h index 3ff2dd4c78ca..3e6243719df8 100644 --- a/drivers/usb/storage/unusual_uas.h +++ b/drivers/usb/storage/unusual_uas.h @@ -53,3 +53,10 @@ UNUSUAL_DEV(0x0bc2, 0x3312, 0x0000, 0x9999, "Expansion Desk", USB_SC_DEVICE, USB_PR_DEVICE, NULL, US_FL_NO_ATA_1X), + +/* Reported-by: Claudio Bizzarri */ +UNUSUAL_DEV(0x152d, 0x0567, 0x0000, 0x9999, + "JMicron", + "JMS567", + USB_SC_DEVICE, USB_PR_DEVICE, NULL, + US_FL_NO_REPORT_OPCODES), diff --git a/drivers/usb/storage/usb.c b/drivers/usb/storage/usb.c index b9d1b9357287..f60e7d463636 100644 --- a/drivers/usb/storage/usb.c +++ b/drivers/usb/storage/usb.c @@ -479,7 +479,7 @@ void usb_stor_adjust_quirks(struct usb_device *udev, unsigned long *fflags) US_FL_SINGLE_LUN | US_FL_NO_WP_DETECT | US_FL_NO_READ_DISC_INFO | US_FL_NO_READ_CAPACITY_16 | US_FL_INITIAL_READ10 | US_FL_WRITE_CACHE | - US_FL_NO_ATA_1X); + US_FL_NO_ATA_1X | US_FL_NO_REPORT_OPCODES); p = quirks; while (*p) { @@ -517,6 +517,9 @@ void usb_stor_adjust_quirks(struct usb_device *udev, unsigned long *fflags) case 'e': f |= US_FL_NO_READ_CAPACITY_16; break; + case 'f': + f |= US_FL_NO_REPORT_OPCODES; + break; case 'h': f |= US_FL_CAPACITY_HEURISTICS; break; diff --git a/include/linux/usb_usual.h b/include/linux/usb_usual.h index d271f88f30ad..a7f2604c5f25 100644 --- a/include/linux/usb_usual.h +++ b/include/linux/usb_usual.h @@ -75,6 +75,8 @@ /* Cannot handle FUA in WRITE or READ CDBs */ \ US_FLAG(NO_ATA_1X, 0x02000000) \ /* Cannot handle ATA_12 or ATA_16 CDBs */ \ + US_FLAG(NO_REPORT_OPCODES, 0x04000000) \ + /* Cannot handle MI_REPORT_SUPPORTED_OPERATION_CODES */ \ #define US_FLAG(name, value) US_FL_##name = value , enum { US_DO_ALL_FLAGS }; -- cgit v1.2.3 From f9554a6b199360c2f888173fd600e1eb7ff165ef Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Wed, 17 Sep 2014 10:10:58 +0200 Subject: uas: Add US_FL_NO_ATA_1X quirk for Seagate (0bc2:ab20) drives https://bbs.archlinux.org/viewtopic.php?pid=1457492 Signed-off-by: Hans de Goede Cc: stable Signed-off-by: Greg Kroah-Hartman --- drivers/usb/storage/unusual_uas.h | 7 +++++++ 1 file changed, 7 insertions(+) (limited to 'drivers/usb/storage') diff --git a/drivers/usb/storage/unusual_uas.h b/drivers/usb/storage/unusual_uas.h index 3e6243719df8..94fb09fe8409 100644 --- a/drivers/usb/storage/unusual_uas.h +++ b/drivers/usb/storage/unusual_uas.h @@ -54,6 +54,13 @@ UNUSUAL_DEV(0x0bc2, 0x3312, 0x0000, 0x9999, USB_SC_DEVICE, USB_PR_DEVICE, NULL, US_FL_NO_ATA_1X), +/* https://bbs.archlinux.org/viewtopic.php?id=183190 */ +UNUSUAL_DEV(0x0bc2, 0xab20, 0x0000, 0x9999, + "Seagate", + "Backup+ BK", + USB_SC_DEVICE, USB_PR_DEVICE, NULL, + US_FL_NO_ATA_1X), + /* Reported-by: Claudio Bizzarri */ UNUSUAL_DEV(0x152d, 0x0567, 0x0000, 0x9999, "JMicron", -- cgit v1.2.3 From 710f1bf16ab1b1558f099b62c5011c4cbba6a7bb Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Tue, 23 Sep 2014 15:48:50 +0200 Subject: uas: Add another ASM1051 usb-id to the uas blacklist As most ASM1051 based devices, this one has unfixable issues with uas too. Cc: stable@vger.kernel.org # 3.16 Signed-off-by: Hans de Goede Signed-off-by: Greg Kroah-Hartman --- drivers/usb/storage/unusual_uas.h | 8 ++++++++ 1 file changed, 8 insertions(+) (limited to 'drivers/usb/storage') diff --git a/drivers/usb/storage/unusual_uas.h b/drivers/usb/storage/unusual_uas.h index 94fb09fe8409..8511b54a65d9 100644 --- a/drivers/usb/storage/unusual_uas.h +++ b/drivers/usb/storage/unusual_uas.h @@ -67,3 +67,11 @@ UNUSUAL_DEV(0x152d, 0x0567, 0x0000, 0x9999, "JMS567", USB_SC_DEVICE, USB_PR_DEVICE, NULL, US_FL_NO_REPORT_OPCODES), + +/* Most ASM1051 based devices have issues with uas, blacklist them all */ +/* Reported-by: Hans de Goede */ +UNUSUAL_DEV(0x174c, 0x5106, 0x0000, 0x9999, + "ASMedia", + "ASM1051", + USB_SC_DEVICE, USB_PR_DEVICE, NULL, + US_FL_IGNORE_UAS), -- cgit v1.2.3 From 5df2be63332a661a8d7234ca15c23bc48ed8e2a2 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Sat, 13 Sep 2014 12:26:29 +0200 Subject: uas: Remove task-management / abort error handling code There are various bug reports about oopses / hangs with the uas driver, which all point to the abort-command and logical-unit-reset (task-management) error handling paths. Getting these right is very hard, there are quite a few corner cases, and testing is almost impossible since under normal operation these code paths are not used at all. Another problem is that there are also some cases where it simply is not clear what to do at all. E.g. over usb-2 multiple outstanding commands share the same endpoint. What if a command gets aborted while its sense urb is half way through completing (so some data has been transfered but not all). Since the urb is not yet complete we don't know if the sense urb is actually for this command, or for one of the other oustanding commands. If it is for one of the other commands and we cancel it, then we end up in an undefined state. But if it is actually for the command we're aborting, and the abort succeeds, then it may never complete... This exact same problem applies to logical unit resets too, if there are multiple luns, then commands outstanding on both luns share the sense endpoint. If there is only a single lun, then doing a logical unit reset is little better then doing a full usb device reset. So summarizing because: 1) abort / lun-reset is very tricky to get right 2) Not being able to test the tricky code, which means it will have bugs 3) This being a code path which under normal operation will never happen, so being slow / sub-optimal here is not really an issue 4) Under error conditions we will still be able to recover through usb device resets. 5) This may be a bit slower in some cases, but this is actually faster in cases where the bridge ship has locked up, which seems to be the most common error case sofar. This commit removes the abort / lun-reset error handling paths, and also the taks-mgmt code since those are the only 2 task-mgmt users. Leaving only the (tested and testable) usb-device-reset error handling path in place. Note I realize that this is somewhat of a big hammer, but currently people are seeing very hard to debug oopses with uas. First let focus on making uas work reliable, then we can later look into adding more fine grained error handling. Signed-off-by: Hans de Goede Signed-off-by: Greg Kroah-Hartman --- drivers/usb/storage/uas.c | 177 +--------------------------------------------- 1 file changed, 1 insertion(+), 176 deletions(-) (limited to 'drivers/usb/storage') diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index cad02ac88564..d49742581241 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -2,7 +2,7 @@ * USB Attached SCSI * Note that this is not the same as the USB Mass Storage driver * - * Copyright Hans de Goede for Red Hat, Inc. 2013 + * Copyright Hans de Goede for Red Hat, Inc. 2013 - 2014 * Copyright Matthew Wilcox for Intel Corp, 2010 * Copyright Sarah Sharp for Intel Corp, 2010 * @@ -52,11 +52,9 @@ struct uas_dev_info { struct usb_anchor data_urbs; unsigned long flags; int qdepth, resetting; - struct response_iu response; unsigned cmd_pipe, status_pipe, data_in_pipe, data_out_pipe; unsigned use_streams:1; unsigned uas_sense_old:1; - unsigned running_task:1; unsigned shutdown:1; struct scsi_cmnd *cmnd; spinlock_t lock; @@ -207,7 +205,6 @@ static void uas_zap_dead(struct uas_dev_info *devinfo) DATA_OUT_URB_INFLIGHT); uas_try_complete(cmnd, __func__); } - devinfo->running_task = 0; spin_unlock_irqrestore(&devinfo->lock, flags); } @@ -350,13 +347,6 @@ static void uas_stat_cmplt(struct urb *urb) cmnd = scsi_host_find_tag(shost, tag - 1); if (!cmnd) { - if (iu->iu_id == IU_ID_RESPONSE) { - if (!devinfo->running_task) - dev_warn(&urb->dev->dev, - "stat urb: recv unexpected response iu\n"); - /* store results for uas_eh_task_mgmt() */ - memcpy(&devinfo->response, iu, sizeof(devinfo->response)); - } usb_free_urb(urb); spin_unlock_irqrestore(&devinfo->lock, flags); return; @@ -536,56 +526,6 @@ static struct urb *uas_alloc_cmd_urb(struct uas_dev_info *devinfo, gfp_t gfp, return NULL; } -static int uas_submit_task_urb(struct scsi_cmnd *cmnd, gfp_t gfp, - u8 function, u16 stream_id) -{ - struct uas_dev_info *devinfo = (void *)cmnd->device->hostdata; - struct usb_device *udev = devinfo->udev; - struct urb *urb = usb_alloc_urb(0, gfp); - struct task_mgmt_iu *iu; - int err = -ENOMEM; - - if (!urb) - goto err; - - iu = kzalloc(sizeof(*iu), gfp); - if (!iu) - goto err; - - iu->iu_id = IU_ID_TASK_MGMT; - iu->tag = cpu_to_be16(stream_id); - int_to_scsilun(cmnd->device->lun, &iu->lun); - - iu->function = function; - switch (function) { - case TMF_ABORT_TASK: - if (blk_rq_tagged(cmnd->request)) - iu->task_tag = cpu_to_be16(cmnd->request->tag + 2); - else - iu->task_tag = cpu_to_be16(1); - break; - } - - usb_fill_bulk_urb(urb, udev, devinfo->cmd_pipe, iu, sizeof(*iu), - uas_cmd_cmplt, cmnd); - urb->transfer_flags |= URB_FREE_BUFFER; - - usb_anchor_urb(urb, &devinfo->cmd_urbs); - err = usb_submit_urb(urb, gfp); - if (err) { - usb_unanchor_urb(urb); - uas_log_cmd_state(cmnd, __func__); - scmd_printk(KERN_ERR, cmnd, "task submission err %d\n", err); - goto err; - } - - return 0; - -err: - usb_free_urb(urb); - return err; -} - /* * Why should I request the Status IU before sending the Command IU? Spec * says to, but also says the device may receive them in any order. Seems @@ -787,118 +727,6 @@ static int uas_queuecommand_lck(struct scsi_cmnd *cmnd, static DEF_SCSI_QCMD(uas_queuecommand) -static int uas_eh_task_mgmt(struct scsi_cmnd *cmnd, - const char *fname, u8 function) -{ - struct Scsi_Host *shost = cmnd->device->host; - struct uas_dev_info *devinfo = (struct uas_dev_info *)shost->hostdata; - u16 tag = devinfo->qdepth; - unsigned long flags; - struct urb *sense_urb; - int result = SUCCESS; - - spin_lock_irqsave(&devinfo->lock, flags); - - if (devinfo->resetting) { - spin_unlock_irqrestore(&devinfo->lock, flags); - return FAILED; - } - - if (devinfo->running_task) { - shost_printk(KERN_INFO, shost, - "%s: %s: error already running a task\n", - __func__, fname); - spin_unlock_irqrestore(&devinfo->lock, flags); - return FAILED; - } - - devinfo->running_task = 1; - memset(&devinfo->response, 0, sizeof(devinfo->response)); - sense_urb = uas_submit_sense_urb(cmnd, GFP_ATOMIC, - devinfo->use_streams ? tag : 0); - if (!sense_urb) { - shost_printk(KERN_INFO, shost, - "%s: %s: submit sense urb failed\n", - __func__, fname); - devinfo->running_task = 0; - spin_unlock_irqrestore(&devinfo->lock, flags); - return FAILED; - } - if (uas_submit_task_urb(cmnd, GFP_ATOMIC, function, tag)) { - shost_printk(KERN_INFO, shost, - "%s: %s: submit task mgmt urb failed\n", - __func__, fname); - devinfo->running_task = 0; - spin_unlock_irqrestore(&devinfo->lock, flags); - usb_kill_urb(sense_urb); - return FAILED; - } - spin_unlock_irqrestore(&devinfo->lock, flags); - - if (usb_wait_anchor_empty_timeout(&devinfo->sense_urbs, 3000) == 0) { - /* - * Note we deliberately do not clear running_task here. If we - * allow new tasks to be submitted, there is no way to figure - * out if a received response_iu is for the failed task or for - * the new one. A bus-reset will eventually clear running_task. - */ - shost_printk(KERN_INFO, shost, - "%s: %s timed out\n", __func__, fname); - return FAILED; - } - - spin_lock_irqsave(&devinfo->lock, flags); - devinfo->running_task = 0; - if (be16_to_cpu(devinfo->response.tag) != tag) { - shost_printk(KERN_INFO, shost, - "%s: %s failed (wrong tag %d/%d)\n", __func__, - fname, be16_to_cpu(devinfo->response.tag), tag); - result = FAILED; - } else if (devinfo->response.response_code != RC_TMF_COMPLETE) { - shost_printk(KERN_INFO, shost, - "%s: %s failed (rc 0x%x)\n", __func__, - fname, devinfo->response.response_code); - result = FAILED; - } - spin_unlock_irqrestore(&devinfo->lock, flags); - - return result; -} - -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; - unsigned long flags; - int ret; - - spin_lock_irqsave(&devinfo->lock, flags); - - if (devinfo->resetting) { - spin_unlock_irqrestore(&devinfo->lock, flags); - return FAILED; - } - - uas_mark_cmd_dead(devinfo, cmdinfo, DID_ABORT, __func__); - if (cmdinfo->state & COMMAND_INFLIGHT) { - spin_unlock_irqrestore(&devinfo->lock, flags); - ret = uas_eh_task_mgmt(cmnd, "ABORT TASK", TMF_ABORT_TASK); - } else { - uas_unlink_data_urbs(devinfo, cmdinfo, &flags); - uas_try_complete(cmnd, __func__); - spin_unlock_irqrestore(&devinfo->lock, flags); - ret = SUCCESS; - } - return ret; -} - -static int uas_eh_device_reset_handler(struct scsi_cmnd *cmnd) -{ - sdev_printk(KERN_INFO, cmnd->device, "%s\n", __func__); - return uas_eh_task_mgmt(cmnd, "LOGICAL UNIT RESET", - TMF_LOGICAL_UNIT_RESET); -} - static int uas_eh_bus_reset_handler(struct scsi_cmnd *cmnd) { struct scsi_device *sdev = cmnd->device; @@ -976,8 +804,6 @@ static struct scsi_host_template uas_host_template = { .queuecommand = uas_queuecommand, .slave_alloc = uas_slave_alloc, .slave_configure = uas_slave_configure, - .eh_abort_handler = uas_eh_abort_handler, - .eh_device_reset_handler = uas_eh_device_reset_handler, .eh_bus_reset_handler = uas_eh_bus_reset_handler, .can_queue = 65536, /* Is there a limit on the _host_ ? */ .this_id = -1, @@ -1093,7 +919,6 @@ static int uas_probe(struct usb_interface *intf, const struct usb_device_id *id) devinfo->intf = intf; devinfo->udev = udev; devinfo->resetting = 0; - devinfo->running_task = 0; devinfo->shutdown = 0; devinfo->flags = id->driver_info; usb_stor_adjust_quirks(udev, &devinfo->flags); -- cgit v1.2.3 From b7b5d11fae766ee0e92821df2694c41f15f98954 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Sat, 13 Sep 2014 12:26:30 +0200 Subject: uas: Fix resetting flag handling - Make sure we always hold the lock when setting / checking resetting - Check resetting before checking urb->status - Add missing check for resetting to uas_data_cmplt - Add missing check for resetting to uas_do_work Signed-off-by: Hans de Goede Signed-off-by: Greg Kroah-Hartman --- drivers/usb/storage/uas.c | 49 +++++++++++++++++++++++++++++++++-------------- 1 file changed, 35 insertions(+), 14 deletions(-) (limited to 'drivers/usb/storage') diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index d49742581241..50d2d85e1051 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -129,6 +129,10 @@ static void uas_do_work(struct work_struct *work) int 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, @@ -143,6 +147,7 @@ static void uas_do_work(struct work_struct *work) else schedule_work(&devinfo->work); } +out: spin_unlock_irqrestore(&devinfo->lock, flags); } @@ -322,6 +327,11 @@ static void uas_stat_cmplt(struct urb *urb) unsigned long flags; u16 tag; + spin_lock_irqsave(&devinfo->lock, flags); + + if (devinfo->resetting) + goto out; + if (urb->status) { if (urb->status == -ENOENT) { dev_err(&urb->dev->dev, "stat urb: killed, stream %d\n", @@ -330,27 +340,17 @@ static void uas_stat_cmplt(struct urb *urb) dev_err(&urb->dev->dev, "stat urb: status %d\n", urb->status); } - usb_free_urb(urb); - return; - } - - if (devinfo->resetting) { - usb_free_urb(urb); - return; + goto out; } - spin_lock_irqsave(&devinfo->lock, flags); tag = be16_to_cpup(&iu->tag) - 1; if (tag == 0) cmnd = devinfo->cmnd; else cmnd = scsi_host_find_tag(shost, tag - 1); - if (!cmnd) { - usb_free_urb(urb); - spin_unlock_irqrestore(&devinfo->lock, flags); - return; - } + if (!cmnd) + goto out; cmdinfo = (void *)&cmnd->SCp; switch (iu->iu_id) { @@ -391,6 +391,7 @@ static void uas_stat_cmplt(struct urb *urb) scmd_printk(KERN_ERR, cmnd, "Bogus IU (%d) received on status pipe\n", iu->iu_id); } +out: usb_free_urb(urb); spin_unlock_irqrestore(&devinfo->lock, flags); } @@ -404,6 +405,7 @@ static void uas_data_cmplt(struct urb *urb) unsigned long flags; spin_lock_irqsave(&devinfo->lock, flags); + if (cmdinfo->data_in_urb == urb) { sdb = scsi_in(cmnd); cmdinfo->state &= ~DATA_IN_URB_INFLIGHT; @@ -413,7 +415,13 @@ static void uas_data_cmplt(struct urb *urb) } if (sdb == NULL) { WARN_ON_ONCE(1); - } else if (urb->status) { + goto out; + } + + if (devinfo->resetting) + goto out; + + if (urb->status) { if (urb->status != -ECONNRESET) { uas_log_cmd_state(cmnd, __func__); scmd_printk(KERN_ERR, cmnd, @@ -426,6 +434,7 @@ static void uas_data_cmplt(struct urb *urb) sdb->resid = sdb->length - urb->actual_length; } uas_try_complete(cmnd, __func__); +out: spin_unlock_irqrestore(&devinfo->lock, flags); } @@ -732,6 +741,7 @@ static int uas_eh_bus_reset_handler(struct scsi_cmnd *cmnd) struct scsi_device *sdev = cmnd->device; struct uas_dev_info *devinfo = sdev->hostdata; struct usb_device *udev = devinfo->udev; + unsigned long flags; int err; err = usb_lock_device_for_reset(udev, devinfo->intf); @@ -742,14 +752,21 @@ static int uas_eh_bus_reset_handler(struct scsi_cmnd *cmnd) } shost_printk(KERN_INFO, sdev->host, "%s start\n", __func__); + + spin_lock_irqsave(&devinfo->lock, flags); 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); err = usb_reset_device(udev); + + spin_lock_irqsave(&devinfo->lock, flags); devinfo->resetting = 0; + spin_unlock_irqrestore(&devinfo->lock, flags); usb_unlock_device(udev); @@ -1049,8 +1066,12 @@ static void uas_disconnect(struct usb_interface *intf) { struct Scsi_Host *shost = usb_get_intfdata(intf); struct uas_dev_info *devinfo = (struct uas_dev_info *)shost->hostdata; + unsigned long flags; + spin_lock_irqsave(&devinfo->lock, flags); devinfo->resetting = 1; + 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); -- cgit v1.2.3 From e0620001e4e318d85ebf43a95eec15fae26ed706 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Sat, 13 Sep 2014 12:26:31 +0200 Subject: uas: Add uas_get_tag() helper function Factor out the mapping of scsi-tags -> uas-tags/stream-ids to a helper function so that there is a single place where this "magic" happens. Signed-off-by: Hans de Goede Signed-off-by: Greg Kroah-Hartman --- drivers/usb/storage/uas.c | 33 +++++++++++++++++++++------------ 1 file changed, 21 insertions(+), 12 deletions(-) (limited to 'drivers/usb/storage') diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index 50d2d85e1051..3debefbd858f 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -259,13 +259,29 @@ static void uas_sense_old(struct urb *urb, struct scsi_cmnd *cmnd) cmnd->result = sense_iu->status; } +/* + * scsi-tags go from 0 - (nr_tags - 1), uas tags need to match stream-ids, + * which go from 1 - nr_streams. And we use 1 for untagged commands. + */ +static int uas_get_tag(struct scsi_cmnd *cmnd) +{ + int tag; + + if (blk_rq_tagged(cmnd->request)) + tag = cmnd->request->tag + 2; + else + tag = 1; + + return tag; +} + 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", - caller, cmnd, cmnd->request->tag, + caller, cmnd, uas_get_tag(cmnd), (ci->state & SUBMIT_STATUS_URB) ? " s-st" : "", (ci->state & ALLOC_DATA_IN_URB) ? " a-in" : "", (ci->state & SUBMIT_DATA_IN_URB) ? " s-in" : "", @@ -516,10 +532,7 @@ static struct urb *uas_alloc_cmd_urb(struct uas_dev_info *devinfo, gfp_t gfp, goto free; iu->iu_id = IU_ID_COMMAND; - if (blk_rq_tagged(cmnd->request)) - iu->tag = cpu_to_be16(cmnd->request->tag + 2); - else - iu->tag = cpu_to_be16(1); + iu->tag = cpu_to_be16(uas_get_tag(cmnd)); iu->prio_attr = UAS_SIMPLE_TAG; iu->len = len; int_to_scsilun(sdev->lun, &iu->lun); @@ -690,17 +703,13 @@ static int uas_queuecommand_lck(struct scsi_cmnd *cmnd, memset(cmdinfo, 0, sizeof(*cmdinfo)); - if (blk_rq_tagged(cmnd->request)) { - cmdinfo->stream = cmnd->request->tag + 2; - } else { + if (!blk_rq_tagged(cmnd->request)) devinfo->cmnd = cmnd; - cmdinfo->stream = 1; - } cmnd->scsi_done = done; - cmdinfo->state = SUBMIT_STATUS_URB | - ALLOC_CMD_URB | SUBMIT_CMD_URB; + cmdinfo->stream = uas_get_tag(cmnd); + cmdinfo->state = SUBMIT_STATUS_URB | ALLOC_CMD_URB | SUBMIT_CMD_URB; switch (cmnd->sc_data_direction) { case DMA_FROM_DEVICE: -- cgit v1.2.3 From 5e61aede477ee108de3f9e57f19cacd8ce3ffe52 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Sat, 13 Sep 2014 12:26:32 +0200 Subject: uas: Do not use scsi_host_find_tag Using scsi_host_find_tag with tags returned by the device is unsafe for multiple reasons: 1) It returns tags->rqs[tag], which may be non NULL even when the cmnd is not owned by us 2) It returns tags->rqs[tag], without holding any locks protecting it 3) It returns tags->rqs[tag], without doing any boundary checking Instead keep our own list which maps tags -> inflight cmnds. Signed-off-by: Hans de Goede Signed-off-by: Greg Kroah-Hartman --- drivers/usb/storage/uas.c | 39 ++++++++++++++++++--------------------- 1 file changed, 18 insertions(+), 21 deletions(-) (limited to 'drivers/usb/storage') diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index 3debefbd858f..33db53f1ce9e 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -30,6 +30,8 @@ #include "uas-detect.h" #include "scsiglue.h" +#define MAX_CMNDS 256 + /* * The r00-r01c specs define this version of the SENSE IU data structure. * It's still in use by several different firmware releases. @@ -56,7 +58,7 @@ struct uas_dev_info { unsigned use_streams:1; unsigned uas_sense_old:1; unsigned shutdown:1; - struct scsi_cmnd *cmnd; + struct scsi_cmnd *cmnd[MAX_CMNDS]; spinlock_t lock; struct work_struct work; struct list_head inflight_list; @@ -316,6 +318,7 @@ static int uas_try_complete(struct scsi_cmnd *cmnd, const char *caller) 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,7 +344,7 @@ static void uas_stat_cmplt(struct urb *urb) struct scsi_cmnd *cmnd; struct uas_cmd_info *cmdinfo; unsigned long flags; - u16 tag; + unsigned int idx; spin_lock_irqsave(&devinfo->lock, flags); @@ -359,21 +362,17 @@ static void uas_stat_cmplt(struct urb *urb) goto out; } - tag = be16_to_cpup(&iu->tag) - 1; - if (tag == 0) - cmnd = devinfo->cmnd; - else - cmnd = scsi_host_find_tag(shost, tag - 1); - - if (!cmnd) + idx = be16_to_cpup(&iu->tag) - 1; + if (idx >= MAX_CMNDS || !devinfo->cmnd[idx]) { + dev_err(&urb->dev->dev, + "stat urb: no pending cmd for tag %d\n", idx + 1); goto out; + } + cmnd = devinfo->cmnd[idx]; cmdinfo = (void *)&cmnd->SCp; switch (iu->iu_id) { case IU_ID_STATUS: - if (devinfo->cmnd == cmnd) - devinfo->cmnd = NULL; - if (urb->actual_length < 16) devinfo->uas_sense_old = 1; if (devinfo->uas_sense_old) @@ -674,6 +673,7 @@ static int uas_queuecommand_lck(struct scsi_cmnd *cmnd, struct uas_dev_info *devinfo = sdev->hostdata; struct uas_cmd_info *cmdinfo = (void *)&cmnd->SCp; unsigned long flags; + unsigned int stream; int err; BUILD_BUG_ON(sizeof(struct uas_cmd_info) > sizeof(struct scsi_pointer)); @@ -696,19 +696,16 @@ static int uas_queuecommand_lck(struct scsi_cmnd *cmnd, return 0; } - if (devinfo->cmnd) { + stream = uas_get_tag(cmnd); + if (devinfo->cmnd[stream - 1]) { spin_unlock_irqrestore(&devinfo->lock, flags); return SCSI_MLQUEUE_DEVICE_BUSY; } - memset(cmdinfo, 0, sizeof(*cmdinfo)); - - if (!blk_rq_tagged(cmnd->request)) - devinfo->cmnd = cmnd; - cmnd->scsi_done = done; - cmdinfo->stream = uas_get_tag(cmnd); + memset(cmdinfo, 0, sizeof(*cmdinfo)); + cmdinfo->stream = stream; cmdinfo->state = SUBMIT_STATUS_URB | ALLOC_CMD_URB | SUBMIT_CMD_URB; switch (cmnd->sc_data_direction) { @@ -738,6 +735,7 @@ static int uas_queuecommand_lck(struct scsi_cmnd *cmnd, uas_add_work(cmdinfo); } + devinfo->cmnd[stream - 1] = cmnd; list_add_tail(&cmdinfo->list, &devinfo->inflight_list); spin_unlock_irqrestore(&devinfo->lock, flags); return 0; @@ -877,7 +875,6 @@ static int uas_configure_endpoints(struct uas_dev_info *devinfo) int r; devinfo->uas_sense_old = 0; - devinfo->cmnd = NULL; r = uas_find_endpoints(devinfo->intf->cur_altsetting, eps); if (r) @@ -897,7 +894,7 @@ static int uas_configure_endpoints(struct uas_dev_info *devinfo) devinfo->use_streams = 0; } else { devinfo->qdepth = usb_alloc_streams(devinfo->intf, eps + 1, - 3, 256, GFP_NOIO); + 3, MAX_CMNDS, GFP_NOIO); if (devinfo->qdepth < 0) return devinfo->qdepth; devinfo->use_streams = 1; -- cgit v1.2.3 From d89da03acec19b39506f3ef32e09134b50b4adb9 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Sat, 13 Sep 2014 12:26:33 +0200 Subject: uas: Check against unexpected completions The status urb should not complete before the command has been submitted, nor should we get a second status urb for the same tag after a IU_ID_STATUS. Data urbs should not complete before the command has been submitted, but may complete after the IU_ID_STATUS. Signed-off-by: Hans de Goede Signed-off-by: Greg Kroah-Hartman --- drivers/usb/storage/uas.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) (limited to 'drivers/usb/storage') diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index 33db53f1ce9e..7f56f31ed661 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -371,6 +371,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) @@ -436,6 +442,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__); -- cgit v1.2.3 From 60d9f67d478e7c8ed09e3a6888b29aca2d978979 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Sat, 13 Sep 2014 12:26:34 +0200 Subject: uas: Simplify unlink of data urbs on error There is no need for all the trickery with dropping the lock, we can simply reference the urbs while we hold the lock to ensure the urbs don't disappear beneath us, and do the actual unlink (+ unreference) after we've dropped the lock. This also fixes a race where we may loose of cmnd ownership to the scsi midlayer without holding the lock due to the midlayer re-claiming ownership through an abort (which will be handled by a future patch in this series). Signed-off-by: Hans de Goede Signed-off-by: Greg Kroah-Hartman --- drivers/usb/storage/uas.c | 48 ++++++++++++++++++----------------------------- 1 file changed, 18 insertions(+), 30 deletions(-) (limited to 'drivers/usb/storage') diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index 7f56f31ed661..e92c676f8e99 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -78,8 +78,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 */ @@ -100,28 +99,6 @@ 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 = @@ -281,8 +258,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 +273,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,8 +284,7 @@ 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; @@ -341,6 +316,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; @@ -387,7 +364,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__); @@ -415,6 +393,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) -- cgit v1.2.3 From 85fea82554ee74f0a2e17729a3911865df5fbba0 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Sat, 13 Sep 2014 12:26:35 +0200 Subject: uas: Free data urbs on completion Now that we no longer drop our lock to unlink the data urbs, we can simply free them on completion, making their handling consistent with the other urbs. Signed-off-by: Hans de Goede Signed-off-by: Greg Kroah-Hartman --- drivers/usb/storage/uas.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'drivers/usb/storage') diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index e92c676f8e99..b2d96fd3f5f9 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -288,8 +288,6 @@ static int uas_try_complete(struct scsi_cmnd *cmnd, const char *caller) 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); @@ -418,9 +416,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); @@ -450,6 +450,7 @@ static void uas_data_cmplt(struct urb *urb) } uas_try_complete(cmnd, __func__); out: + usb_free_urb(urb); spin_unlock_irqrestore(&devinfo->lock, flags); } -- cgit v1.2.3 From 1589349f74d669b767bc0971fb21372ad300452e Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Sat, 13 Sep 2014 12:26:36 +0200 Subject: uas: Simplify reset / disconnect handling Drop the whole dance with first moving cmnds to a dead-list. The resetting flag ensures that no new cmds / urbs will be submitted, and that any urb completions are short-circuited without trying to complete the scsi cmnd. Signed-off-by: Hans de Goede Signed-off-by: Greg Kroah-Hartman --- drivers/usb/storage/uas.c | 43 ++++++------------------------------------- 1 file changed, 6 insertions(+), 37 deletions(-) (limited to 'drivers/usb/storage') diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index b2d96fd3f5f9..dd8772b60ab8 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -62,7 +62,6 @@ struct uas_dev_info { spinlock_t lock; struct work_struct work; struct list_head inflight_list; - struct list_head dead_list; }; enum { @@ -130,35 +129,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; @@ -170,7 +140,7 @@ 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; @@ -182,11 +152,11 @@ static void uas_zap_dead(struct uas_dev_info *devinfo) struct scsi_cmnd *cmnd = container_of(scp, struct scsi_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); + cmnd->result = result << 16; uas_try_complete(cmnd, __func__); } spin_unlock_irqrestore(&devinfo->lock, flags); @@ -765,11 +735,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); @@ -952,7 +922,6 @@ static int uas_probe(struct usb_interface *intf, const struct usb_device_id *id) 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) @@ -1080,11 +1049,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); -- cgit v1.2.3 From 9c15c5738b5219fdc273e8923b2c1a9d5e8ce3b3 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Sat, 13 Sep 2014 12:26:37 +0200 Subject: uas: zap_pending: data urbs should have completed at this time The data urbs are all killed before calling zap_pending, and their completion handler should have cleared their inflight flag. Do not 0 the data inflight flags, and add a check for try_complete succeeding, as it should always succeed when called from zap_pending. Signed-off-by: Hans de Goede Signed-off-by: Greg Kroah-Hartman --- drivers/usb/storage/uas.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) (limited to 'drivers/usb/storage') diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index dd8772b60ab8..7daecd59cd38 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -145,6 +145,7 @@ static void uas_zap_pending(struct uas_dev_info *devinfo, int result) struct uas_cmd_info *cmdinfo; struct uas_cmd_info *temp; unsigned long flags; + int err; spin_lock_irqsave(&devinfo->lock, flags); list_for_each_entry_safe(cmdinfo, temp, &devinfo->dead_list, list) { @@ -152,12 +153,11 @@ static void uas_zap_pending(struct uas_dev_info *devinfo, int result) struct scsi_cmnd *cmnd = container_of(scp, struct scsi_cmnd, SCp); uas_log_cmd_state(cmnd, __func__); - /* all urbs are killed, clear inflight bits */ - cmdinfo->state &= ~(COMMAND_INFLIGHT | - DATA_IN_URB_INFLIGHT | - DATA_OUT_URB_INFLIGHT); + /* Sense urbs were killed, clear COMMAND_INFLIGHT manually */ + cmdinfo->state &= ~COMMAND_INFLIGHT; cmnd->result = result << 16; - uas_try_complete(cmnd, __func__); + err = uas_try_complete(cmnd, __func__); + WARN_ON(err != 0); } spin_unlock_irqrestore(&devinfo->lock, flags); } -- cgit v1.2.3 From 43cd99cb178ce3d0a1fb6faa898b30be6dcbc8b5 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Sat, 13 Sep 2014 12:26:38 +0200 Subject: uas: Drop inflight list We've the same info doubled in both the inflight list and the cmnd array, drop the list. Signed-off-by: Hans de Goede Signed-off-by: Greg Kroah-Hartman --- drivers/usb/storage/uas.c | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) (limited to 'drivers/usb/storage') diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index 7daecd59cd38..06c3eaede2ac 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -61,7 +61,6 @@ struct uas_dev_info { struct scsi_cmnd *cmnd[MAX_CMNDS]; spinlock_t lock; struct work_struct work; - struct list_head inflight_list; }; enum { @@ -87,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 */ @@ -103,18 +101,21 @@ 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; @@ -143,15 +144,17 @@ static void uas_add_work(struct uas_cmd_info *cmdinfo) 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 err; + 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__); /* Sense urbs were killed, clear COMMAND_INFLIGHT manually */ cmdinfo->state &= ~COMMAND_INFLIGHT; @@ -260,7 +263,6 @@ static int uas_try_complete(struct scsi_cmnd *cmnd, const char *caller) cmdinfo->state |= COMMAND_COMPLETED; 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; @@ -707,7 +709,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; } @@ -921,7 +922,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); result = uas_configure_endpoints(devinfo); if (result) -- cgit v1.2.3 From b6823c51fcd82e993275f5403e120279232ecaec Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Sat, 13 Sep 2014 12:26:39 +0200 Subject: uas: Remove cmnd reference from the cmd urb It is not strictly necessary for the cmd urb to have a reference to the cmnd, and without this reference it becomes easier to drop all references to a cmnd on an abort. Signed-off-by: Hans de Goede Signed-off-by: Greg Kroah-Hartman --- drivers/usb/storage/uas.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) (limited to 'drivers/usb/storage') diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index 06c3eaede2ac..8421dcd8ed44 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -428,12 +428,9 @@ out: 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); } @@ -511,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; -- cgit v1.2.3 From 616f0e6cab4698309ff9e48ee2a85b5eb78cf31a Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Sat, 13 Sep 2014 12:26:40 +0200 Subject: uas: Drop all references to a scsi_cmnd once it has been aborted Do not keep references around to a cmnd which is under error handling. Signed-off-by: Hans de Goede Signed-off-by: Greg Kroah-Hartman --- drivers/usb/storage/uas.c | 47 ++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 44 insertions(+), 3 deletions(-) (limited to 'drivers/usb/storage') diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index 8421dcd8ed44..b1a1acb43461 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -257,12 +257,11 @@ 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)) + DATA_OUT_URB_INFLIGHT | + COMMAND_ABORTED)) return -EBUSY; WARN_ON_ONCE(cmdinfo->state & COMMAND_COMPLETED); cmdinfo->state |= COMMAND_COMPLETED; - if (cmdinfo->state & COMMAND_ABORTED) - scmd_printk(KERN_INFO, cmnd, "abort completed\n"); devinfo->cmnd[uas_get_tag(cmnd) - 1] = NULL; cmnd->scsi_done(cmnd); return 0; @@ -712,6 +711,47 @@ static int uas_queuecommand_lck(struct scsi_cmnd *cmnd, static DEF_SCSI_QCMD(uas_queuecommand) +/* + * 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); + + uas_log_cmd_state(cmnd, __func__); + + /* Ensure that try_complete does not call scsi_done */ + cmdinfo->state |= COMMAND_ABORTED; + + /* Drop all refs to this cmnd, kill data urbs to break their ref */ + devinfo->cmnd[uas_get_tag(cmnd) - 1] = NULL; + if (cmdinfo->state & DATA_IN_URB_INFLIGHT) + data_in_urb = usb_get_urb(cmdinfo->data_in_urb); + if (cmdinfo->state & DATA_OUT_URB_INFLIGHT) + data_out_urb = usb_get_urb(cmdinfo->data_out_urb); + + spin_unlock_irqrestore(&devinfo->lock, flags); + + if (data_in_urb) { + usb_kill_urb(data_in_urb); + usb_put_urb(data_in_urb); + } + if (data_out_urb) { + usb_kill_urb(data_out_urb); + usb_put_urb(data_out_urb); + } + + return FAILED; +} + static int uas_eh_bus_reset_handler(struct scsi_cmnd *cmnd) { struct scsi_device *sdev = cmnd->device; @@ -797,6 +837,7 @@ static struct scsi_host_template uas_host_template = { .queuecommand = uas_queuecommand, .slave_alloc = uas_slave_alloc, .slave_configure = uas_slave_configure, + .eh_abort_handler = uas_eh_abort_handler, .eh_bus_reset_handler = uas_eh_bus_reset_handler, .can_queue = 65536, /* Is there a limit on the _host_ ? */ .this_id = -1, -- cgit v1.2.3 From 4c5481efb4346948ba7034432f86235a16ac9180 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Sat, 13 Sep 2014 12:26:41 +0200 Subject: uas: Fix memleak of non-submitted urbs Not all urbs we've allocated are necessarily also submitted, non-submitted urbs will not be free-ed by their completion handler. So we need to free them manually. There are 2 scenarios where this can happen: 1) We have failed to submit some urbs at abort / disconnect 2) When running over usb-2 we may have never tried to submit the data urbs when completing the scsi cmnd, because we never got a READ/WRITE_READY iu Signed-off-by: Hans de Goede Signed-off-by: Greg Kroah-Hartman --- drivers/usb/storage/uas.c | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) (limited to 'drivers/usb/storage') diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index b1a1acb43461..10a3dea041ed 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -249,6 +249,25 @@ static void uas_log_cmd_state(struct scsi_cmnd *cmnd, const char *caller) (ci->state & IS_IN_WORK_LIST) ? " work" : ""); } +static void uas_free_unsubmitted_urbs(struct scsi_cmnd *cmnd) +{ + struct uas_cmd_info *cmdinfo; + + if (!cmnd) + return; + + cmdinfo = (void *)&cmnd->SCp; + + if (cmdinfo->state & SUBMIT_CMD_URB) + usb_free_urb(cmdinfo->cmd_urb); + + /* data urbs may have never gotten their submit flag set */ + if (!(cmdinfo->state & DATA_IN_URB_INFLIGHT)) + usb_free_urb(cmdinfo->data_in_urb); + if (!(cmdinfo->state & DATA_OUT_URB_INFLIGHT)) + usb_free_urb(cmdinfo->data_out_urb); +} + static int uas_try_complete(struct scsi_cmnd *cmnd, const char *caller) { struct uas_cmd_info *cmdinfo = (void *)&cmnd->SCp; @@ -263,6 +282,7 @@ static int uas_try_complete(struct scsi_cmnd *cmnd, const char *caller) WARN_ON_ONCE(cmdinfo->state & COMMAND_COMPLETED); cmdinfo->state |= COMMAND_COMPLETED; devinfo->cmnd[uas_get_tag(cmnd) - 1] = NULL; + uas_free_unsubmitted_urbs(cmnd); cmnd->scsi_done(cmnd); return 0; } @@ -738,6 +758,8 @@ static int uas_eh_abort_handler(struct scsi_cmnd *cmnd) if (cmdinfo->state & DATA_OUT_URB_INFLIGHT) data_out_urb = usb_get_urb(cmdinfo->data_out_urb); + uas_free_unsubmitted_urbs(cmnd); + spin_unlock_irqrestore(&devinfo->lock, flags); if (data_in_urb) { -- cgit v1.2.3 From f9dc024a2da1fe6b0ce180b89fac085e1255a932 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Sat, 13 Sep 2014 12:26:42 +0200 Subject: uas: pre_reset and suspend: Fix a few races The purpose of uas_pre_reset is to: 1) Stop any new commands from being submitted while an externally triggered usb-device-reset is running 2) Wait for any pending commands to finish before allowing the usb-device-reset to continue The purpose of uas_suspend is to: 2) Wait for any pending commands to finish before suspending This commit fixes races in both paths: 1) For 1) we use scsi_block_requests, but the scsi midlayer calls queuecommand without holding any locks, so a queuecommand may already past the midlayer scsi_block_requests checks when we call it, add a check to uas_queuecommand to fix this 2) For 2) we were waiting for all sense-urbs to complete, there are 2 problems with this approach: a) data-urbs may complete after the sense urb, so we need to check for those too b) if a sense-urb completes with a iu id of READ/WRITE_READY a command is not yet done. We submit a new sense-urb immediately in this case, but that submit may fail (in which case it will get retried by uas_do_work), if this happens the sense_urbs anchor may become empty while the cmnd is not yet done Also unblock requests on timeout, to avoid things getting stuck in that case. Signed-off-by: Hans de Goede Signed-off-by: Greg Kroah-Hartman --- drivers/usb/storage/uas.c | 61 ++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 55 insertions(+), 6 deletions(-) (limited to 'drivers/usb/storage') diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index 10a3dea041ed..8d2e5450de91 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -667,6 +667,10 @@ static int uas_queuecommand_lck(struct scsi_cmnd *cmnd, BUILD_BUG_ON(sizeof(struct uas_cmd_info) > sizeof(struct scsi_pointer)); + /* Re-check scsi_block_requests now that we've the host-lock */ + if (cmnd->device->host->host_self_blocked) + return SCSI_MLQUEUE_DEVICE_BUSY; + if ((devinfo->flags & US_FL_NO_ATA_1X) && (cmnd->cmnd[0] == ATA_12 || cmnd->cmnd[0] == ATA_16)) { memcpy(cmnd->sense_buffer, usb_stor_sense_invalidCDB, @@ -1009,6 +1013,54 @@ set_alt0: return result; } +static int uas_cmnd_list_empty(struct uas_dev_info *devinfo) +{ + unsigned long flags; + int i, r = 1; + + spin_lock_irqsave(&devinfo->lock, flags); + + for (i = 0; i < devinfo->qdepth; i++) { + if (devinfo->cmnd[i]) { + r = 0; /* Not empty */ + break; + } + } + + spin_unlock_irqrestore(&devinfo->lock, flags); + + return r; +} + +/* + * Wait for any pending cmnds to complete, on usb-2 sense_urbs may temporarily + * get empty while there still is more work to do due to sense-urbs completing + * with a READ/WRITE_READY iu code, so keep waiting until the list gets empty. + */ +static int uas_wait_for_pending_cmnds(struct uas_dev_info *devinfo) +{ + unsigned long start_time; + int r; + + start_time = jiffies; + do { + flush_work(&devinfo->work); + + r = usb_wait_anchor_empty_timeout(&devinfo->sense_urbs, 5000); + if (r == 0) + return -ETIME; + + r = usb_wait_anchor_empty_timeout(&devinfo->data_urbs, 500); + if (r == 0) + return -ETIME; + + if (time_after(jiffies, start_time + 5 * HZ)) + return -ETIME; + } while (!uas_cmnd_list_empty(devinfo)); + + return 0; +} + static int uas_pre_reset(struct usb_interface *intf) { struct Scsi_Host *shost = usb_get_intfdata(intf); @@ -1023,10 +1075,9 @@ static int uas_pre_reset(struct usb_interface *intf) scsi_block_requests(shost); spin_unlock_irqrestore(shost->host_lock, flags); - /* Wait for any pending requests to complete */ - flush_work(&devinfo->work); - if (usb_wait_anchor_empty_timeout(&devinfo->sense_urbs, 5000) == 0) { + if (uas_wait_for_pending_cmnds(devinfo) != 0) { shost_printk(KERN_ERR, shost, "%s: timed out\n", __func__); + scsi_unblock_requests(shost); return 1; } @@ -1064,9 +1115,7 @@ static int uas_suspend(struct usb_interface *intf, pm_message_t message) struct Scsi_Host *shost = usb_get_intfdata(intf); struct uas_dev_info *devinfo = (struct uas_dev_info *)shost->hostdata; - /* Wait for any pending requests to complete */ - flush_work(&devinfo->work); - if (usb_wait_anchor_empty_timeout(&devinfo->sense_urbs, 5000) == 0) { + if (uas_wait_for_pending_cmnds(devinfo) != 0) { shost_printk(KERN_ERR, shost, "%s: timed out\n", __func__); return -ETIME; } -- cgit v1.2.3 From e5e558192f01857254938349f78cd492daee7d72 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Sat, 13 Sep 2014 12:26:43 +0200 Subject: uas: Use streams on upcoming 10Gbps / 3.1 USB Limit the no-streams case to speeds less then USB_SPEED_SUPER. Signed-off-by: Hans de Goede Signed-off-by: Greg Kroah-Hartman --- drivers/usb/storage/uas.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/usb/storage') diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index 8d2e5450de91..c69b9c5bb265 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -925,7 +925,7 @@ static int uas_configure_endpoints(struct uas_dev_info *devinfo) devinfo->data_out_pipe = usb_sndbulkpipe(udev, usb_endpoint_num(&eps[3]->desc)); - if (udev->speed != USB_SPEED_SUPER) { + if (udev->speed < USB_SPEED_SUPER) { devinfo->qdepth = 32; devinfo->use_streams = 0; } else { -- cgit v1.2.3 From 51b361737bcec832ea07650e27f93098e44c834b Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Sat, 13 Sep 2014 12:26:44 +0200 Subject: uas: Do not log urb status error on cancellation Check for both type of cancellation codes for sense and data urbs. Signed-off-by: Hans de Goede Signed-off-by: Greg Kroah-Hartman --- drivers/usb/storage/uas.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) (limited to 'drivers/usb/storage') diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index c69b9c5bb265..e5f3e9881247 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -318,10 +318,7 @@ static void uas_stat_cmplt(struct urb *urb) goto out; if (urb->status) { - if (urb->status == -ENOENT) { - dev_err(&urb->dev->dev, "stat urb: killed, stream %d\n", - urb->stream_id); - } else { + if (urb->status != -ENOENT && urb->status != -ECONNRESET) { dev_err(&urb->dev->dev, "stat urb: status %d\n", urb->status); } @@ -428,7 +425,7 @@ static void uas_data_cmplt(struct urb *urb) } if (urb->status) { - if (urb->status != -ECONNRESET) { + if (urb->status != -ENOENT && urb->status != -ECONNRESET) { uas_log_cmd_state(cmnd, __func__); scmd_printk(KERN_ERR, cmnd, "data cmplt err %d stream %d\n", -- cgit v1.2.3 From 6dcd8ec24052fefb7faee80b6ccc8ada860e33d7 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Sat, 13 Sep 2014 12:26:45 +0200 Subject: uas: Use scsi_print_command Use scsi_print_command to print commands during errors, rather then printing the rather meaningless pointer to the command. Signed-off-by: Hans de Goede Signed-off-by: Greg Kroah-Hartman --- drivers/usb/storage/uas.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'drivers/usb/storage') diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index e5f3e9881247..445f9499f8e9 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -232,8 +232,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\n", - caller, cmnd, uas_get_tag(cmnd), + "%s tag %d inflight:%s%s%s%s%s%s%s%s%s%s%s%s%s ", + caller, uas_get_tag(cmnd), (ci->state & SUBMIT_STATUS_URB) ? " s-st" : "", (ci->state & ALLOC_DATA_IN_URB) ? " a-in" : "", (ci->state & SUBMIT_DATA_IN_URB) ? " s-in" : "", @@ -247,6 +247,7 @@ static void uas_log_cmd_state(struct scsi_cmnd *cmnd, const char *caller) (ci->state & COMMAND_COMPLETED) ? " done" : "", (ci->state & COMMAND_ABORTED) ? " abort" : "", (ci->state & IS_IN_WORK_LIST) ? " work" : ""); + scsi_print_command(cmnd); } static void uas_free_unsubmitted_urbs(struct scsi_cmnd *cmnd) -- cgit v1.2.3 From eb7d664ae459114cbbee8ecef17f90b9c71d994c Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Sat, 13 Sep 2014 12:26:46 +0200 Subject: uas: Drop COMMAND_COMPLETED flag It was only used to sanity check against completing the same cmnd twice, but that is the case we're likely operating on free-ed memory, and doing sanity checks on free-ed memory is not really helpful. Signed-off-by: Hans de Goede Signed-off-by: Greg Kroah-Hartman --- drivers/usb/storage/uas.c | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) (limited to 'drivers/usb/storage') diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index 445f9499f8e9..4455fab398ec 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -74,9 +74,8 @@ enum { COMMAND_INFLIGHT = (1 << 8), DATA_IN_URB_INFLIGHT = (1 << 9), DATA_OUT_URB_INFLIGHT = (1 << 10), - COMMAND_COMPLETED = (1 << 11), - COMMAND_ABORTED = (1 << 12), - IS_IN_WORK_LIST = (1 << 13), + COMMAND_ABORTED = (1 << 11), + IS_IN_WORK_LIST = (1 << 12), }; /* Overrides scsi_pointer */ @@ -232,7 +231,7 @@ 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 tag %d inflight:%s%s%s%s%s%s%s%s%s%s%s%s%s ", + "%s tag %d inflight:%s%s%s%s%s%s%s%s%s%s%s%s ", caller, uas_get_tag(cmnd), (ci->state & SUBMIT_STATUS_URB) ? " s-st" : "", (ci->state & ALLOC_DATA_IN_URB) ? " a-in" : "", @@ -244,7 +243,6 @@ static void uas_log_cmd_state(struct scsi_cmnd *cmnd, const char *caller) (ci->state & COMMAND_INFLIGHT) ? " CMD" : "", (ci->state & DATA_IN_URB_INFLIGHT) ? " IN" : "", (ci->state & DATA_OUT_URB_INFLIGHT) ? " OUT" : "", - (ci->state & COMMAND_COMPLETED) ? " done" : "", (ci->state & COMMAND_ABORTED) ? " abort" : "", (ci->state & IS_IN_WORK_LIST) ? " work" : ""); scsi_print_command(cmnd); @@ -280,8 +278,6 @@ static int uas_try_complete(struct scsi_cmnd *cmnd, const char *caller) DATA_OUT_URB_INFLIGHT | COMMAND_ABORTED)) return -EBUSY; - WARN_ON_ONCE(cmdinfo->state & COMMAND_COMPLETED); - cmdinfo->state |= COMMAND_COMPLETED; devinfo->cmnd[uas_get_tag(cmnd) - 1] = NULL; uas_free_unsubmitted_urbs(cmnd); cmnd->scsi_done(cmnd); -- cgit v1.2.3 From 5ad22cfc13399cc46267e5685769d6e7a0bbe163 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Sat, 13 Sep 2014 12:26:47 +0200 Subject: uas: Remove support for old sense ui as used in pre-production hardware I've access to a number of different uas devices now, and none of them use old style sense urbs. The only case where these code-paths trigger is with the asm1051 and there they do the wrong thing, as the asm1051 sends 8 bytes status iu-s when it does not have any sense data, but uses new style sense iu-s regardless, as can be seen for scsi cmnds where there is sense data. Signed-off-by: Hans de Goede Signed-off-by: Greg Kroah-Hartman --- drivers/usb/storage/uas.c | 47 +---------------------------------------------- 1 file changed, 1 insertion(+), 46 deletions(-) (limited to 'drivers/usb/storage') diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index 4455fab398ec..720310ab3bf3 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -32,20 +32,6 @@ #define MAX_CMNDS 256 -/* - * The r00-r01c specs define this version of the SENSE IU data structure. - * It's still in use by several different firmware releases. - */ -struct sense_iu_old { - __u8 iu_id; - __u8 rsvd1; - __be16 tag; - __be16 len; - __u8 status; - __u8 service_response; - __u8 sense[SCSI_SENSE_BUFFERSIZE]; -}; - struct uas_dev_info { struct usb_interface *intf; struct usb_device *udev; @@ -56,7 +42,6 @@ struct uas_dev_info { int qdepth, resetting; unsigned cmd_pipe, status_pipe, data_in_pipe, data_out_pipe; unsigned use_streams:1; - unsigned uas_sense_old:1; unsigned shutdown:1; struct scsi_cmnd *cmnd[MAX_CMNDS]; spinlock_t lock; @@ -187,29 +172,6 @@ static void uas_sense(struct urb *urb, struct scsi_cmnd *cmnd) cmnd->result = sense_iu->status; } -static void uas_sense_old(struct urb *urb, struct scsi_cmnd *cmnd) -{ - struct sense_iu_old *sense_iu = urb->transfer_buffer; - struct scsi_device *sdev = cmnd->device; - - if (urb->actual_length > 8) { - unsigned len = be16_to_cpup(&sense_iu->len) - 2; - if (len + 8 != urb->actual_length) { - int newlen = min(len + 8, urb->actual_length) - 8; - if (newlen < 0) - newlen = 0; - sdev_printk(KERN_INFO, sdev, "%s: urb length %d " - "disagrees with IU sense data length %d, " - "using %d bytes of sense data\n", __func__, - urb->actual_length, len, newlen); - len = newlen; - } - memcpy(cmnd->sense_buffer, sense_iu->sense, len); - } - - cmnd->result = sense_iu->status; -} - /* * scsi-tags go from 0 - (nr_tags - 1), uas tags need to match stream-ids, * which go from 1 - nr_streams. And we use 1 for untagged commands. @@ -339,12 +301,7 @@ static void uas_stat_cmplt(struct urb *urb) switch (iu->iu_id) { case IU_ID_STATUS: - if (urb->actual_length < 16) - devinfo->uas_sense_old = 1; - if (devinfo->uas_sense_old) - uas_sense_old(urb, cmnd); - else - uas_sense(urb, cmnd); + uas_sense(urb, cmnd); if (cmnd->result != 0) { /* cancel data transfers on error */ data_in_urb = usb_get_urb(cmdinfo->data_in_urb); @@ -904,8 +861,6 @@ static int uas_configure_endpoints(struct uas_dev_info *devinfo) struct usb_device *udev = devinfo->udev; int r; - devinfo->uas_sense_old = 0; - r = uas_find_endpoints(devinfo->intf->cur_altsetting, eps); if (r) return r; -- cgit v1.2.3 From 102c00cb91f36f6f7afa6658b2436b04fb3d95b3 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Sat, 13 Sep 2014 12:26:48 +0200 Subject: uas: Remove protype hardware usb interface info We've removed all hack from the driver for pre-production hardware. Signed-off-by: Hans de Goede Signed-off-by: Greg Kroah-Hartman --- drivers/usb/storage/uas.c | 2 -- 1 file changed, 2 deletions(-) (limited to 'drivers/usb/storage') diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index 720310ab3bf3..6da4a48ae7d2 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -834,8 +834,6 @@ static struct usb_device_id uas_usb_ids[] = { # include "unusual_uas.h" { USB_INTERFACE_INFO(USB_CLASS_MASS_STORAGE, USB_SC_SCSI, USB_PR_BULK) }, { USB_INTERFACE_INFO(USB_CLASS_MASS_STORAGE, USB_SC_SCSI, USB_PR_UAS) }, - /* 0xaa is a prototype device I happen to have access to */ - { USB_INTERFACE_INFO(USB_CLASS_MASS_STORAGE, USB_SC_SCSI, 0xaa) }, { } }; MODULE_DEVICE_TABLE(usb, uas_usb_ids); -- cgit v1.2.3 From 1ad7ed5af3d85d0d8b3cdc5a4b823272b85c46cf Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Sat, 13 Sep 2014 12:26:49 +0200 Subject: uas: Cleanup uas_log_cmd_state usage Instead of doing: uas_log_cmd_state(cmnd, __func__) scmd_printk(KERN_ERR, cmnd, "error doing foo %d\n", err) On error, resulting in 2 log calls for a single error, make uas_log_cmd_state take a status code, and change calls like the above to: uas_log_cmd_state(cmnd, "error doing foo", err) Also change various sanity checks (which should never trigger) from: "scmd_printk(KERN_ERR, cmnd, "sanity foo failed\n")" to calling the new uas_log_cmd_state(), so that when they do trigger we get more info. Signed-off-by: Hans de Goede Signed-off-by: Greg Kroah-Hartman --- drivers/usb/storage/uas.c | 52 +++++++++++++++++------------------------------ 1 file changed, 19 insertions(+), 33 deletions(-) (limited to 'drivers/usb/storage') diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index 6da4a48ae7d2..816f56658628 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -78,7 +78,8 @@ static int uas_submit_urbs(struct scsi_cmnd *cmnd, static void uas_do_work(struct work_struct *work); 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); +static void uas_log_cmd_state(struct scsi_cmnd *cmnd, const char *prefix, + int status); static void uas_do_work(struct work_struct *work) { @@ -139,7 +140,7 @@ static void uas_zap_pending(struct uas_dev_info *devinfo, int result) cmnd = devinfo->cmnd[i]; cmdinfo = (void *)&cmnd->SCp; - uas_log_cmd_state(cmnd, __func__); + uas_log_cmd_state(cmnd, __func__, 0); /* Sense urbs were killed, clear COMMAND_INFLIGHT manually */ cmdinfo->state &= ~COMMAND_INFLIGHT; cmnd->result = result << 16; @@ -188,13 +189,14 @@ static int uas_get_tag(struct scsi_cmnd *cmnd) return tag; } -static void uas_log_cmd_state(struct scsi_cmnd *cmnd, const char *caller) +static void uas_log_cmd_state(struct scsi_cmnd *cmnd, const char *prefix, + int status) { struct uas_cmd_info *ci = (void *)&cmnd->SCp; scmd_printk(KERN_INFO, cmnd, - "%s tag %d inflight:%s%s%s%s%s%s%s%s%s%s%s%s ", - caller, uas_get_tag(cmnd), + "%s %d tag %d inflight:%s%s%s%s%s%s%s%s%s%s%s%s ", + prefix, status, uas_get_tag(cmnd), (ci->state & SUBMIT_STATUS_URB) ? " s-st" : "", (ci->state & ALLOC_DATA_IN_URB) ? " a-in" : "", (ci->state & SUBMIT_DATA_IN_URB) ? " s-in" : "", @@ -295,7 +297,7 @@ static void uas_stat_cmplt(struct urb *urb) cmdinfo = (void *)&cmnd->SCp; if (!(cmdinfo->state & COMMAND_INFLIGHT)) { - scmd_printk(KERN_ERR, cmnd, "unexpected status cmplt\n"); + uas_log_cmd_state(cmnd, "unexpected status cmplt", 0); goto out; } @@ -313,7 +315,7 @@ static void uas_stat_cmplt(struct urb *urb) case IU_ID_READ_READY: if (!cmdinfo->data_in_urb || (cmdinfo->state & DATA_IN_URB_INFLIGHT)) { - scmd_printk(KERN_ERR, cmnd, "unexpected read rdy\n"); + uas_log_cmd_state(cmnd, "unexpected read rdy", 0); break; } uas_xfer_data(urb, cmnd, SUBMIT_DATA_IN_URB); @@ -321,14 +323,13 @@ static void uas_stat_cmplt(struct urb *urb) case IU_ID_WRITE_READY: if (!cmdinfo->data_out_urb || (cmdinfo->state & DATA_OUT_URB_INFLIGHT)) { - scmd_printk(KERN_ERR, cmnd, "unexpected write rdy\n"); + uas_log_cmd_state(cmnd, "unexpected write rdy", 0); break; } uas_xfer_data(urb, cmnd, SUBMIT_DATA_OUT_URB); break; default: - scmd_printk(KERN_ERR, cmnd, - "Bogus IU (%d) received on status pipe\n", iu->iu_id); + uas_log_cmd_state(cmnd, "bogus IU", iu->iu_id); } out: usb_free_urb(urb); @@ -374,17 +375,13 @@ static void uas_data_cmplt(struct urb *urb) /* 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"); + uas_log_cmd_state(cmnd, "unexpected data cmplt", 0); goto out; } if (urb->status) { - if (urb->status != -ENOENT && urb->status != -ECONNRESET) { - uas_log_cmd_state(cmnd, __func__); - scmd_printk(KERN_ERR, cmnd, - "data cmplt err %d stream %d\n", - urb->status, urb->stream_id); - } + if (urb->status != -ENOENT && urb->status != -ECONNRESET) + uas_log_cmd_state(cmnd, "data cmplt err", urb->status); /* error: no data transfered */ sdb->resid = sdb->length; } else { @@ -508,10 +505,7 @@ static struct urb *uas_submit_sense_urb(struct scsi_cmnd *cmnd, err = usb_submit_urb(urb, gfp); if (err) { usb_unanchor_urb(urb); - uas_log_cmd_state(cmnd, __func__); - shost_printk(KERN_INFO, shost, - "sense urb submission error %d stream %d\n", - err, stream); + uas_log_cmd_state(cmnd, "sense submit err", err); usb_free_urb(urb); return NULL; } @@ -547,10 +541,7 @@ static int uas_submit_urbs(struct scsi_cmnd *cmnd, err = usb_submit_urb(cmdinfo->data_in_urb, gfp); if (err) { usb_unanchor_urb(cmdinfo->data_in_urb); - uas_log_cmd_state(cmnd, __func__); - scmd_printk(KERN_INFO, cmnd, - "data in urb submission error %d stream %d\n", - err, cmdinfo->data_in_urb->stream_id); + uas_log_cmd_state(cmnd, "data in submit err", err); return SCSI_MLQUEUE_DEVICE_BUSY; } cmdinfo->state &= ~SUBMIT_DATA_IN_URB; @@ -571,10 +562,7 @@ static int uas_submit_urbs(struct scsi_cmnd *cmnd, err = usb_submit_urb(cmdinfo->data_out_urb, gfp); if (err) { usb_unanchor_urb(cmdinfo->data_out_urb); - uas_log_cmd_state(cmnd, __func__); - scmd_printk(KERN_INFO, cmnd, - "data out urb submission error %d stream %d\n", - err, cmdinfo->data_out_urb->stream_id); + uas_log_cmd_state(cmnd, "data out submit err", err); return SCSI_MLQUEUE_DEVICE_BUSY; } cmdinfo->state &= ~SUBMIT_DATA_OUT_URB; @@ -593,9 +581,7 @@ static int uas_submit_urbs(struct scsi_cmnd *cmnd, err = usb_submit_urb(cmdinfo->cmd_urb, gfp); if (err) { usb_unanchor_urb(cmdinfo->cmd_urb); - uas_log_cmd_state(cmnd, __func__); - scmd_printk(KERN_INFO, cmnd, - "cmd urb submission error %d\n", err); + uas_log_cmd_state(cmnd, "cmd submit err", err); return SCSI_MLQUEUE_DEVICE_BUSY; } cmdinfo->cmd_urb = NULL; @@ -701,7 +687,7 @@ static int uas_eh_abort_handler(struct scsi_cmnd *cmnd) spin_lock_irqsave(&devinfo->lock, flags); - uas_log_cmd_state(cmnd, __func__); + uas_log_cmd_state(cmnd, __func__, 0); /* Ensure that try_complete does not call scsi_done */ cmdinfo->state |= COMMAND_ABORTED; -- cgit v1.2.3 From ce39fe6fa115d9fea0112c907773a400b98d2463 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Sat, 13 Sep 2014 12:26:50 +0200 Subject: uas: Log error codes when logging errors Signed-off-by: Hans de Goede Signed-off-by: Greg Kroah-Hartman --- drivers/usb/storage/uas.c | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) (limited to 'drivers/usb/storage') diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index 816f56658628..dfa11912708d 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -750,7 +750,8 @@ static int uas_eh_bus_reset_handler(struct scsi_cmnd *cmnd) usb_unlock_device(udev); if (err) { - shost_printk(KERN_INFO, sdev->host, "%s FAILED\n", __func__); + shost_printk(KERN_INFO, sdev->host, "%s FAILED err %d\n", + __func__, err); return FAILED; } @@ -1024,13 +1025,16 @@ static int uas_post_reset(struct usb_interface *intf) struct Scsi_Host *shost = usb_get_intfdata(intf); struct uas_dev_info *devinfo = (struct uas_dev_info *)shost->hostdata; unsigned long flags; + int err; if (devinfo->shutdown) return 0; - if (uas_configure_endpoints(devinfo) != 0) { + err = uas_configure_endpoints(devinfo); + if (err) { shost_printk(KERN_ERR, shost, - "%s: alloc streams error after reset", __func__); + "%s: alloc streams error %d after reset", + __func__, err); return 1; } @@ -1066,10 +1070,13 @@ static int uas_reset_resume(struct usb_interface *intf) struct Scsi_Host *shost = usb_get_intfdata(intf); struct uas_dev_info *devinfo = (struct uas_dev_info *)shost->hostdata; unsigned long flags; + int err; - if (uas_configure_endpoints(devinfo) != 0) { + err = uas_configure_endpoints(devinfo); + if (err) { shost_printk(KERN_ERR, shost, - "%s: alloc streams error after reset", __func__); + "%s: alloc streams error %d after reset", + __func__, err); return -EIO; } -- cgit v1.2.3 From fac1f48584c1b6c745412cf8c5dbdc1725aad8f2 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Sat, 13 Sep 2014 12:26:51 +0200 Subject: uas: Add response iu handling If something goes wrong in our communication with an uas device we may get a response iu in reaction to a cmnd, rather then a status iu. In this case propagate an error upwards, rather then logging a bogus iu message. Signed-off-by: Hans de Goede Signed-off-by: Greg Kroah-Hartman --- drivers/usb/storage/uas.c | 10 ++++++++++ 1 file changed, 10 insertions(+) (limited to 'drivers/usb/storage') diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index dfa11912708d..b27fe21d866d 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -328,6 +328,16 @@ static void uas_stat_cmplt(struct urb *urb) } uas_xfer_data(urb, cmnd, SUBMIT_DATA_OUT_URB); break; + case IU_ID_RESPONSE: + uas_log_cmd_state(cmnd, "unexpected response iu", + ((struct response_iu *)iu)->response_code); + /* Error, cancel data transfers */ + data_in_urb = usb_get_urb(cmdinfo->data_in_urb); + data_out_urb = usb_get_urb(cmdinfo->data_out_urb); + cmdinfo->state &= ~COMMAND_INFLIGHT; + cmnd->result = DID_ERROR << 16; + uas_try_complete(cmnd, __func__); + break; default: uas_log_cmd_state(cmnd, "bogus IU", iu->iu_id); } -- cgit v1.2.3 From 7adce46784a0992eb7276416798db6746ba8f667 Mon Sep 17 00:00:00 2001 From: Andy Shevchenko Date: Fri, 5 Sep 2014 17:56:25 +0300 Subject: USB: storage: use %*ph specifier to dump small buffers Instead of dereference each byte let's use %*ph specifier in the printk() calls. Signed-off-by: Andy Shevchenko Signed-off-by: Greg Kroah-Hartman --- drivers/usb/storage/alauda.c | 11 ++++------- drivers/usb/storage/sddr09.c | 3 +-- 2 files changed, 5 insertions(+), 9 deletions(-) (limited to 'drivers/usb/storage') diff --git a/drivers/usb/storage/alauda.c b/drivers/usb/storage/alauda.c index 6636a583da12..62c2d9daa7d6 100644 --- a/drivers/usb/storage/alauda.c +++ b/drivers/usb/storage/alauda.c @@ -415,14 +415,11 @@ static int alauda_init_media(struct us_data *us) if (alauda_get_media_signature(us, data) != USB_STOR_XFER_GOOD) return USB_STOR_TRANSPORT_ERROR; - usb_stor_dbg(us, "Media signature: %02X %02X %02X %02X\n", - data[0], data[1], data[2], data[3]); + usb_stor_dbg(us, "Media signature: %4ph\n", data); media_info = alauda_card_find_id(data[1]); if (media_info == NULL) { - printk(KERN_WARNING - "alauda_init_media: Unrecognised media signature: " - "%02X %02X %02X %02X\n", - data[0], data[1], data[2], data[3]); + pr_warn("alauda_init_media: Unrecognised media signature: %4ph\n", + data); return USB_STOR_TRANSPORT_ERROR; } @@ -513,7 +510,7 @@ static int alauda_check_status2(struct us_data *us) if (rc != USB_STOR_XFER_GOOD) return rc; - usb_stor_dbg(us, "%02X %02X %02X\n", data[0], data[1], data[2]); + usb_stor_dbg(us, "%3ph\n", data); if (data[0] & ALAUDA_STATUS_ERROR) return USB_STOR_XFER_ERROR; diff --git a/drivers/usb/storage/sddr09.c b/drivers/usb/storage/sddr09.c index 38a4504ce450..3847053d732c 100644 --- a/drivers/usb/storage/sddr09.c +++ b/drivers/usb/storage/sddr09.c @@ -1155,8 +1155,7 @@ sddr09_get_cardinfo(struct us_data *us, unsigned char flags) { return NULL; } - sprintf(blurbtxt, "sddr09: Found Flash card, ID = %02X %02X %02X %02X", - deviceID[0], deviceID[1], deviceID[2], deviceID[3]); + sprintf(blurbtxt, "sddr09: Found Flash card, ID = %4ph", deviceID); /* Byte 0 is the manufacturer */ sprintf(blurbtxt + strlen(blurbtxt), -- cgit v1.2.3 From a6cd244b921973e4ff38f238496cc094142bd444 Mon Sep 17 00:00:00 2001 From: Mickael Maison Date: Thu, 18 Sep 2014 11:25:04 +0200 Subject: usb: Fixed a few typos Fixed typos in comments of various drivers/usb files Signed-off-by: Mickael Maison Acked-by: Felipe Balbi Signed-off-by: Greg Kroah-Hartman --- drivers/usb/core/hcd.c | 2 +- drivers/usb/musb/musb_regs.h | 2 +- drivers/usb/storage/realtek_cr.c | 4 ++-- drivers/usb/storage/scsiglue.c | 2 +- drivers/usb/storage/usb.c | 2 +- 5 files changed, 6 insertions(+), 6 deletions(-) (limited to 'drivers/usb/storage') diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c index 487abcfcccd8..56a06612a5e2 100644 --- a/drivers/usb/core/hcd.c +++ b/drivers/usb/core/hcd.c @@ -1272,7 +1272,7 @@ EXPORT_SYMBOL_GPL(usb_hcd_unlink_urb_from_ep); * The usb core itself is however optimized for host controllers that can dma * using regular system memory - like pci devices doing bus mastering. * - * To support host controllers with limited dma capabilites we provide dma + * To support host controllers with limited dma capabilities we provide dma * bounce buffers. This feature can be enabled using the HCD_LOCAL_MEM flag. * For this to work properly the host controller code must first use the * function dma_declare_coherent_memory() to point out which memory area diff --git a/drivers/usb/musb/musb_regs.h b/drivers/usb/musb/musb_regs.h index b9bcda5e3945..37122a480bc1 100644 --- a/drivers/usb/musb/musb_regs.h +++ b/drivers/usb/musb/musb_regs.h @@ -577,7 +577,7 @@ static inline u16 musb_read_hwvers(void __iomem *mbase) { /* * This register is invisible on Blackfin, actually the MUSB - * RTL version of Blackfin is 1.9, so just harcode its value. + * RTL version of Blackfin is 1.9, so just hardcode its value. */ return MUSB_HWVERS_1900; } diff --git a/drivers/usb/storage/realtek_cr.c b/drivers/usb/storage/realtek_cr.c index 281be56d5648..8591d89a38e6 100644 --- a/drivers/usb/storage/realtek_cr.c +++ b/drivers/usb/storage/realtek_cr.c @@ -115,7 +115,7 @@ struct rts51x_chip { enum RTS51X_STAT state; int support_auto_delink; #endif - /* used to back up the protocal choosen in probe1 phase */ + /* used to back up the protocol chosen in probe1 phase */ proto_cmnd proto_handler_backup; }; @@ -925,7 +925,7 @@ static int realtek_cr_autosuspend_setup(struct us_data *us) (unsigned long)chip); fw5895_init(us); - /* enable autosuspend funciton of the usb device */ + /* enable autosuspend function of the usb device */ usb_enable_autosuspend(us->pusb_dev); return 0; diff --git a/drivers/usb/storage/scsiglue.c b/drivers/usb/storage/scsiglue.c index 866b5df36ed1..0e400f382f3a 100644 --- a/drivers/usb/storage/scsiglue.c +++ b/drivers/usb/storage/scsiglue.c @@ -241,7 +241,7 @@ static int slave_configure(struct scsi_device *sdev) /* Some USB cardreaders have trouble reading an sdcard's last * sector in a larger then 1 sector read, since the performance - * impact is negible we set this flag for all USB disks */ + * impact is negligible we set this flag for all USB disks */ sdev->last_sector_bug = 1; /* Enable last-sector hacks for single-target devices using diff --git a/drivers/usb/storage/usb.c b/drivers/usb/storage/usb.c index f60e7d463636..ef9058138dff 100644 --- a/drivers/usb/storage/usb.c +++ b/drivers/usb/storage/usb.c @@ -98,7 +98,7 @@ MODULE_PARM_DESC(quirks, "supplemental list of device IDs and their quirks"); /* The vendor name should be kept at eight characters or less, and * the product name should be kept at 16 characters or less. If a device * has the US_FL_FIX_INQUIRY flag, then the vendor and product names - * normally generated by a device thorugh the INQUIRY response will be + * normally generated by a device through the INQUIRY response will be * taken from this list, and this is the reason for the above size * restriction. However, if the flag is not present, then you * are free to use as many characters as you like. -- cgit v1.2.3 From bda9893c50fb56253d3c206c14e3f933e5f68b3c Mon Sep 17 00:00:00 2001 From: Mark Knibbs Date: Sun, 21 Sep 2014 19:59:42 +0100 Subject: storage: Replace magic number with define in usb_stor_euscsi_init() usb_stor_euscsi_init() calls usb_stor_control_msg() with timeout argument 5000. USB_CTRL_SET_TIMEOUT is defined to be 5000 in usb.h, so would it make sense to use that instead? Patch below if it would. Signed-off-by: Mark Knibbs Signed-off-by: Greg Kroah-Hartman --- drivers/usb/storage/initializers.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/usb/storage') diff --git a/drivers/usb/storage/initializers.c b/drivers/usb/storage/initializers.c index 5a8b5ff1e45b..4bc2fc98636e 100644 --- a/drivers/usb/storage/initializers.c +++ b/drivers/usb/storage/initializers.c @@ -52,7 +52,7 @@ int usb_stor_euscsi_init(struct us_data *us) us->iobuf[0] = 0x1; result = usb_stor_control_msg(us, us->send_ctrl_pipe, 0x0C, USB_RECIP_INTERFACE | USB_TYPE_VENDOR, - 0x01, 0x0, us->iobuf, 0x1, 5000); + 0x01, 0x0, us->iobuf, 0x1, USB_CTRL_SET_TIMEOUT); usb_stor_dbg(us, "-- result is %d\n", result); return 0; -- cgit v1.2.3 From 57cde01a7b8111cdd43b6a261763aad1ead8161c Mon Sep 17 00:00:00 2001 From: Mark Knibbs Date: Tue, 23 Sep 2014 11:20:17 +0100 Subject: storage: Add quirks for Castlewood and Double-H USB-SCSI converters Castlewood Systems supplied various models of USB-SCSI converter with their ORB external removable-media drive. The ORB Windows and Macintosh drivers support six USB IDs: 084B:A001 [VID 084B is Castlewood Systems] 04E6:0002 (*) ORB USB Smart Cable P/N 88205-001 (generic SCM ID) 2027:A001 Double-H Technology DH-2000SC 1822:0001 (*) Ariston iConnect/iSCSI 07AF:0004 (*) Microtech XpressSCSI (25-pin) 07AF:0005 (*) Microtech XpressSCSI (50-pin) *: quirk already in unusual-devs.h [Apparently the official VID for Double-H Technology is 0x07EB = 2027 decimal. That's another hex/decimal mix-up with these SCM-based products (in addition to the Ariston and Entrega ones). Perhaps the USB-IF informed companies of their allocated VID in decimal, but they assumed it was hex? It seems all Entrega products used VID 0x1645, not just the USB-SCSI converter.] Double-H Technology Co., Ltd. produced a USB-SCSI converter, model DH-2000SC, which is probably the one supported by the ORB drivers. Perhaps the Castlewood-bundled product had a different label or PID though? Castlewood mentioned Conmate as being one type of USB-SCSI converter. Conmate and Double-H seem related somehow; both company addresses in the same road, and at one point the Conmate web site mentioned DH-2000H4, DH-200D4/DH-2000C4 as models of USB hub (DH short for Double-H presumably). Conmate did show a USB-SCSI converter model CM-660 on their web site at one point. My guess is that was identical to the DH-2000SC. Mention of the Double-H product: http://web.archive.org/web/20010221010141/http://www.doubleh.com.tw/dh-2000sc.htm The only picture I could find is at http://jp.acesuppliers.com/catalog/j64/component/page03.html The casing design looks the same as my ORB USB Smart Cable which has ID 04E6:0002. Anyway, that's enough rambling. Here's the patch. storage: Add quirks for Castlewood and Double-H USB-SCSI converters Add quirks for two SCM-based USB-SCSI converters which were bundled with some Castlewood ORB removable drives. Without the quirk only the (single) drive with SCSI ID 0 can be accessed. Signed-off-by: Mark Knibbs Signed-off-by: Greg Kroah-Hartman --- drivers/usb/storage/unusual_devs.h | 14 ++++++++++++++ 1 file changed, 14 insertions(+) (limited to 'drivers/usb/storage') diff --git a/drivers/usb/storage/unusual_devs.h b/drivers/usb/storage/unusual_devs.h index 4a5c68a47e46..b065398c5566 100644 --- a/drivers/usb/storage/unusual_devs.h +++ b/drivers/usb/storage/unusual_devs.h @@ -1099,6 +1099,13 @@ UNUSUAL_DEV( 0x0840, 0x0085, 0x0001, 0x0001, USB_SC_DEVICE, USB_PR_DEVICE, NULL, US_FL_FIX_CAPACITY), +/* Supplied with some Castlewood ORB removable drives */ +UNUSUAL_DEV( 0x084b, 0xa001, 0x0000, 0x9999, + "Castlewood Systems", + "USB to SCSI cable", + USB_SC_DEVICE, USB_PR_DEVICE, usb_stor_euscsi_init, + US_FL_SCM_MULT_TARG ), + /* Entry and supporting patch by Theodore Kilgore . * Flag will support Bulk devices which use a standards-violating 32-byte * Command Block Wrapper. Here, the "DC2MEGA" cameras (several brands) with @@ -2063,6 +2070,13 @@ UNUSUAL_DEV( 0x1e74, 0x4621, 0x0000, 0x0000, USB_SC_DEVICE, USB_PR_DEVICE, NULL, US_FL_BULK_IGNORE_TAG | US_FL_MAX_SECTORS_64 ), +/* Supplied with some Castlewood ORB removable drives */ +UNUSUAL_DEV( 0x2027, 0xa001, 0x0000, 0x9999, + "Double-H Technology", + "USB to SCSI Intelligent Cable", + USB_SC_DEVICE, USB_PR_DEVICE, usb_stor_euscsi_init, + US_FL_SCM_MULT_TARG ), + UNUSUAL_DEV( 0x2116, 0x0320, 0x0001, 0x0001, "ST", "2A", -- cgit v1.2.3 From 3512e7bfea6a459cad84712a021d856bd78cd7e4 Mon Sep 17 00:00:00 2001 From: Mark Knibbs Date: Tue, 23 Sep 2014 12:43:02 +0100 Subject: storage: Add quirk for another SCM-based USB-SCSI converter There is apparently another SCM USB-SCSI converter with ID 04E6:000F. It is listed along with 04E6:000B in the Windows INF file for the Startech ICUSBSCSI2 as "eUSB SCSI Adapter (Bus Powered)". The quirk allows devices with SCSI ID other than 0 to be accessed. Also make a couple of existing SCM product IDs lower case to be consistent with other entries. Signed-off-by: Mark Knibbs Signed-off-by: Greg Kroah-Hartman --- drivers/usb/storage/unusual_devs.h | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) (limited to 'drivers/usb/storage') diff --git a/drivers/usb/storage/unusual_devs.h b/drivers/usb/storage/unusual_devs.h index b065398c5566..11c7a9676441 100644 --- a/drivers/usb/storage/unusual_devs.h +++ b/drivers/usb/storage/unusual_devs.h @@ -492,18 +492,24 @@ UNUSUAL_DEV( 0x04e6, 0x000a, 0x0200, 0x0200, "eUSB CompactFlash Adapter", USB_SC_8020, USB_PR_CB, NULL, 0), -UNUSUAL_DEV( 0x04e6, 0x000B, 0x0100, 0x0100, +UNUSUAL_DEV( 0x04e6, 0x000b, 0x0100, 0x0100, "Shuttle", "eUSCSI Bridge", USB_SC_SCSI, USB_PR_BULK, usb_stor_euscsi_init, US_FL_SCM_MULT_TARG ), -UNUSUAL_DEV( 0x04e6, 0x000C, 0x0100, 0x0100, +UNUSUAL_DEV( 0x04e6, 0x000c, 0x0100, 0x0100, "Shuttle", "eUSCSI Bridge", USB_SC_SCSI, USB_PR_BULK, usb_stor_euscsi_init, US_FL_SCM_MULT_TARG ), +UNUSUAL_DEV( 0x04e6, 0x000f, 0x0000, 0x9999, + "SCM Microsystems", + "eUSB SCSI Adapter (Bus Powered)", + USB_SC_DEVICE, USB_PR_DEVICE, usb_stor_euscsi_init, + US_FL_SCM_MULT_TARG ), + UNUSUAL_DEV( 0x04e6, 0x0101, 0x0200, 0x0200, "Shuttle", "CD-RW Device", -- cgit v1.2.3 From 2d75b9cbb1418f20ad1e688dd8312a029ef2e6b5 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Fri, 3 Oct 2014 12:08:56 +0200 Subject: uas: Reduce number of function arguments for uas_alloc_foo functions The stream_id and pipe are already present in uas_cmd_info resp uas_dev_info, so there is no need to pass a copy along. Signed-off-by: Hans de Goede Reviewed-by: Christoph Hellwig Signed-off-by: Greg Kroah-Hartman --- drivers/usb/storage/uas.c | 29 ++++++++++++++--------------- 1 file changed, 14 insertions(+), 15 deletions(-) (limited to 'drivers/usb/storage') diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index b27fe21d866d..d1dbe8833b4a 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -412,20 +412,22 @@ static void uas_cmd_cmplt(struct urb *urb) } static struct urb *uas_alloc_data_urb(struct uas_dev_info *devinfo, gfp_t gfp, - unsigned int pipe, u16 stream_id, struct scsi_cmnd *cmnd, enum dma_data_direction dir) { struct usb_device *udev = devinfo->udev; + struct uas_cmd_info *cmdinfo = (void *)&cmnd->SCp; struct urb *urb = usb_alloc_urb(0, gfp); struct scsi_data_buffer *sdb = (dir == DMA_FROM_DEVICE) ? scsi_in(cmnd) : scsi_out(cmnd); + unsigned int pipe = (dir == DMA_FROM_DEVICE) + ? devinfo->data_in_pipe : devinfo->data_out_pipe; if (!urb) goto out; usb_fill_bulk_urb(urb, udev, pipe, NULL, sdb->length, uas_data_cmplt, cmnd); - urb->stream_id = stream_id; + urb->stream_id = cmdinfo->stream; urb->num_sgs = udev->bus->sg_tablesize ? sdb->table.nents : 0; urb->sg = sdb->table.sgl; out: @@ -433,9 +435,10 @@ static struct urb *uas_alloc_data_urb(struct uas_dev_info *devinfo, gfp_t gfp, } static struct urb *uas_alloc_sense_urb(struct uas_dev_info *devinfo, gfp_t gfp, - struct Scsi_Host *shost, u16 stream_id) + struct scsi_cmnd *cmnd) { struct usb_device *udev = devinfo->udev; + struct uas_cmd_info *cmdinfo = (void *)&cmnd->SCp; struct urb *urb = usb_alloc_urb(0, gfp); struct sense_iu *iu; @@ -447,8 +450,8 @@ static struct urb *uas_alloc_sense_urb(struct uas_dev_info *devinfo, gfp_t gfp, goto free; usb_fill_bulk_urb(urb, udev, devinfo->status_pipe, iu, sizeof(*iu), - uas_stat_cmplt, shost); - urb->stream_id = stream_id; + uas_stat_cmplt, cmnd->device->host); + urb->stream_id = cmdinfo->stream; urb->transfer_flags |= URB_FREE_BUFFER; out: return urb; @@ -500,15 +503,13 @@ static struct urb *uas_alloc_cmd_urb(struct uas_dev_info *devinfo, gfp_t gfp, * daft to me. */ -static struct urb *uas_submit_sense_urb(struct scsi_cmnd *cmnd, - gfp_t gfp, unsigned int stream) +static struct urb *uas_submit_sense_urb(struct scsi_cmnd *cmnd, gfp_t gfp) { - struct Scsi_Host *shost = cmnd->device->host; - struct uas_dev_info *devinfo = (struct uas_dev_info *)shost->hostdata; + struct uas_dev_info *devinfo = cmnd->device->hostdata; struct urb *urb; int err; - urb = uas_alloc_sense_urb(devinfo, gfp, shost, stream); + urb = uas_alloc_sense_urb(devinfo, gfp, cmnd); if (!urb) return NULL; usb_anchor_urb(urb, &devinfo->sense_urbs); @@ -531,7 +532,7 @@ static int uas_submit_urbs(struct scsi_cmnd *cmnd, lockdep_assert_held(&devinfo->lock); if (cmdinfo->state & SUBMIT_STATUS_URB) { - urb = uas_submit_sense_urb(cmnd, gfp, cmdinfo->stream); + urb = uas_submit_sense_urb(cmnd, gfp); if (!urb) return SCSI_MLQUEUE_DEVICE_BUSY; cmdinfo->state &= ~SUBMIT_STATUS_URB; @@ -539,8 +540,7 @@ static int uas_submit_urbs(struct scsi_cmnd *cmnd, if (cmdinfo->state & ALLOC_DATA_IN_URB) { cmdinfo->data_in_urb = uas_alloc_data_urb(devinfo, gfp, - devinfo->data_in_pipe, cmdinfo->stream, - cmnd, DMA_FROM_DEVICE); + cmnd, DMA_FROM_DEVICE); if (!cmdinfo->data_in_urb) return SCSI_MLQUEUE_DEVICE_BUSY; cmdinfo->state &= ~ALLOC_DATA_IN_URB; @@ -560,8 +560,7 @@ static int uas_submit_urbs(struct scsi_cmnd *cmnd, if (cmdinfo->state & ALLOC_DATA_OUT_URB) { cmdinfo->data_out_urb = uas_alloc_data_urb(devinfo, gfp, - devinfo->data_out_pipe, cmdinfo->stream, - cmnd, DMA_TO_DEVICE); + cmnd, DMA_TO_DEVICE); if (!cmdinfo->data_out_urb) return SCSI_MLQUEUE_DEVICE_BUSY; cmdinfo->state &= ~ALLOC_DATA_OUT_URB; -- cgit v1.2.3