[PATCH v2] ohci-pci: add qemu quirk
On a loaded virtualization host (dozen guests booting at the same time)
it may happen that the ohci controller emulation doesn't manage to do
timely frame processing, with the result that the io watchdog fires and
considers the controller being dead, even though it's only the emulation
being unusual slow due to the load peak.
So, add a quirk for qemu and don't use the watchdog in case we figure we
are running on emulated ohci. The virtual ohci controller masquerades
as apple ohci controller, but we can identify it by subsystem id.
Signed-off-by: Gerd Hoffmann <kra...@redhat.com>
---
drivers/usb/host/ohci-hcd.c | 3 ++-
drivers/usb/host/ohci-pci.c | 16
drivers/usb/host/ohci.h | 1 +
3 files changed, 19 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/host/ohci-hcd.c b/drivers/usb/host/ohci-hcd.c
index b6daf2e..5f81a2b 100644
--- a/drivers/usb/host/ohci-hcd.c
+++ b/drivers/usb/host/ohci-hcd.c
@@ -231,7 +231,8 @@ static int ohci_urb_enqueue (
/* Start up the I/O watchdog timer, if it's not running */
if (!timer_pending(>io_watchdog) &&
- list_empty(>eds_in_use)) {
+ list_empty(>eds_in_use) &&
+ !(ohci->flags & OHCI_QUIRK_QEMU)) {
ohci->prev_frame_no = ohci_frame_no(ohci);
mod_timer(>io_watchdog,
jiffies + IO_WATCHDOG_DELAY);
diff --git a/drivers/usb/host/ohci-pci.c b/drivers/usb/host/ohci-pci.c
index bb15096..a84aebe 100644
--- a/drivers/usb/host/ohci-pci.c
+++ b/drivers/usb/host/ohci-pci.c
@@ -164,6 +164,15 @@ static int ohci_quirk_amd700(struct usb_hcd *hcd)
return 0;
}
+static int ohci_quirk_qemu(struct usb_hcd *hcd)
+{
+ struct ohci_hcd *ohci = hcd_to_ohci(hcd);
+
+ ohci->flags |= OHCI_QUIRK_QEMU;
+ ohci_dbg(ohci, "enabled qemu quirk\n");
+ return 0;
+}
+
/* List of quirks for OHCI */
static const struct pci_device_id ohci_pci_quirks[] = {
{
@@ -214,6 +223,13 @@ static int ohci_quirk_amd700(struct usb_hcd *hcd)
PCI_DEVICE(PCI_VENDOR_ID_ATI, 0x4399),
.driver_data = (unsigned long)ohci_quirk_amd700,
},
+ {
+ .vendor = PCI_VENDOR_ID_APPLE,
+ .device = 0x003f,
+ .subvendor = PCI_SUBVENDOR_ID_REDHAT_QUMRANET,
+ .subdevice = PCI_SUBDEVICE_ID_QEMU,
+ .driver_data= (unsigned long)ohci_quirk_qemu,
+ },
/* FIXME for some of the early AMD 760 southbridges, OHCI
* won't work at all. blacklist them.
diff --git a/drivers/usb/host/ohci.h b/drivers/usb/host/ohci.h
index 37f1725..a51b189 100644
--- a/drivers/usb/host/ohci.h
+++ b/drivers/usb/host/ohci.h
@@ -418,6 +418,7 @@ struct ohci_hcd {
#defineOHCI_QUIRK_AMD_PLL 0x200 /* AMD PLL
quirk*/
#defineOHCI_QUIRK_AMD_PREFETCH 0x400 /* pre-fetch
for ISO transfer */
#defineOHCI_QUIRK_GLOBAL_SUSPEND 0x800 /* must suspend
ports */
+#defineOHCI_QUIRK_QEMU 0x1000 /* relax timing
expectations */
// there are also chip quirks/bugs in init logic
--
1.8.3.1
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] ohci-pci: add qemu quirk
On a loaded virtualization host (dozen guests booting at the same time)
it may happen that the ohci controller emulation doesn't manage to do
timely frame processing, with the result that the io watchdog fires and
considers the controller being dead, even though it's only the emulation
being unusual slow due to the load peak.
So, add a quirk for qemu and don't use the watchdog in case we figure we
are running on emulated ohci. The virtual ohci controller masquerades
as apple ohci controller, but we can identify it by subsystem id.
Signed-off-by: Gerd Hoffmann <kra...@redhat.com>
---
drivers/usb/host/ohci-hcd.c | 3 ++-
drivers/usb/host/ohci-pci.c | 16
drivers/usb/host/ohci.h | 1 +
3 files changed, 19 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/host/ohci-hcd.c b/drivers/usb/host/ohci-hcd.c
index b6daf2e..b75ae28 100644
--- a/drivers/usb/host/ohci-hcd.c
+++ b/drivers/usb/host/ohci-hcd.c
@@ -231,7 +231,8 @@ static int ohci_urb_enqueue (
/* Start up the I/O watchdog timer, if it's not running */
if (!timer_pending(>io_watchdog) &&
- list_empty(>eds_in_use)) {
+ list_empty(>eds_in_use) &&
+ !(ohci->flags & OHCI_QUIRK_QEMU)) {
ohci->prev_frame_no = ohci_frame_no(ohci);
mod_timer(>io_watchdog,
jiffies + IO_WATCHDOG_DELAY);
diff --git a/drivers/usb/host/ohci-pci.c b/drivers/usb/host/ohci-pci.c
index bb15096..a84aebe 100644
--- a/drivers/usb/host/ohci-pci.c
+++ b/drivers/usb/host/ohci-pci.c
@@ -164,6 +164,15 @@ static int ohci_quirk_amd700(struct usb_hcd *hcd)
return 0;
}
+static int ohci_quirk_qemu(struct usb_hcd *hcd)
+{
+ struct ohci_hcd *ohci = hcd_to_ohci(hcd);
+
+ ohci->flags |= OHCI_QUIRK_QEMU;
+ ohci_dbg(ohci, "enabled qemu quirk\n");
+ return 0;
+}
+
/* List of quirks for OHCI */
static const struct pci_device_id ohci_pci_quirks[] = {
{
@@ -214,6 +223,13 @@ static int ohci_quirk_amd700(struct usb_hcd *hcd)
PCI_DEVICE(PCI_VENDOR_ID_ATI, 0x4399),
.driver_data = (unsigned long)ohci_quirk_amd700,
},
+ {
+ .vendor = PCI_VENDOR_ID_APPLE,
+ .device = 0x003f,
+ .subvendor = PCI_SUBVENDOR_ID_REDHAT_QUMRANET,
+ .subdevice = PCI_SUBDEVICE_ID_QEMU,
+ .driver_data= (unsigned long)ohci_quirk_qemu,
+ },
/* FIXME for some of the early AMD 760 southbridges, OHCI
* won't work at all. blacklist them.
diff --git a/drivers/usb/host/ohci.h b/drivers/usb/host/ohci.h
index 37f1725..a51b189 100644
--- a/drivers/usb/host/ohci.h
+++ b/drivers/usb/host/ohci.h
@@ -418,6 +418,7 @@ struct ohci_hcd {
#defineOHCI_QUIRK_AMD_PLL 0x200 /* AMD PLL
quirk*/
#defineOHCI_QUIRK_AMD_PREFETCH 0x400 /* pre-fetch
for ISO transfer */
#defineOHCI_QUIRK_GLOBAL_SUSPEND 0x800 /* must suspend
ports */
+#defineOHCI_QUIRK_QEMU 0x1000 /* relax timing
expectations */
// there are also chip quirks/bugs in init logic
--
1.8.3.1
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: MAINTAINERS: Oliver Neukum is the new uas maintainer
On Do, 2016-07-14 at 14:26 +0200, Hans de Goede wrote: > Oliver Neukum is taking over uas maintainership from me and > Gerd Hoffmann. > > Cc: Oliver Neukum <oneu...@suse.com> > Cc: Gerd Hoffmann <kra...@redhat.com> > Signed-off-by: Hans de Goede <hdego...@redhat.com> Acked-by: Gerd Hoffmann <kra...@redhat.com> -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 32/68] uas: Use proper packet size when submitting reponse urbs
On Fr, 2013-11-15 at 16:06 +0100, Hans de Goede wrote: task management commands expect a response_iu rather then a sense_iu, and these have different sizes. Make the urb we submit to get the reply the right size. I think that doesn't work for uas in usb2 (no streams) mode. status urbs are not linked to requests (via tag / stream id) in that case, so you can't know in advance which status urb gets filles with a sense_ui and which with a response_ui by the device. cheers, Gerd -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] xhci: fix usb3 streams
Gerd, Hans, any objections to this updated patch? The warning is fixed
with it.
The patch probably still needs to address the case where the ring
expansion fails because we can't insert the new segments into the radix
tree. The patch should probably allocate the segments, attempt to add
them to the radix tree, and fail without modifying the link TRBs of the
ring. I'd have to look more deeply into the code to see what exactly
should be done there.
I would like that issue fixed before I merge these patches, so if you
want to take a stab at fixing it, please do.
Sarah Sharp
88
xhci maintains a radix tree for each stream endpoint because it must
be able to map a trb address to the stream ring. Each ring segment
must be added to the ring for this to work. Currently xhci sticks
only the first segment of each stream ring into the radix tree.
Result is that things work initially, but as soon as the first segment
is full xhci can't map the trb address from the completion event to the
stream ring any more - BOOM. You'll find this message in the logs:
ERROR Transfer event for disabled endpoint or incorrect stream ring
This patch adds a helper function to update the radix tree, and a
function to remove ring segments from the tree. Both functions loop
over the segment list and handles all segments instead of just the
first.
[Note: Sarah changed this patch to add radix_tree_maybe_preload() and
radix_tree_preload_end() calls around the radix tree insert, since we
can now insert entries in interrupt context. There are now two helper
functions to make the code cleaner, and those functions are moved to
make them static.]
Signed-off-by: Gerd Hoffmann kra...@redhat.com
Signed-off-by: Hans de Goede hdego...@redhat.com
Signed-off-by: Sarah Sharp sarah.a.sh...@linux.intel.com
---
drivers/usb/host/xhci-mem.c | 132 +---
drivers/usb/host/xhci.h | 1 +
2 files changed, 90 insertions(+), 43 deletions(-)
diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index 83bcd13..8b1ba5b 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -149,14 +149,95 @@ static void xhci_link_rings(struct xhci_hcd *xhci, struct
xhci_ring *ring,
}
}
+/*
+ * We need a radix tree for mapping physical addresses of TRBs to which stream
+ * ID they belong to. We need to do this because the host controller won't
tell
+ * us which stream ring the TRB came from. We could store the stream ID in an
+ * event data TRB, but that doesn't help us for the cancellation case, since
the
+ * endpoint may stop before it reaches that event data TRB.
+ *
+ * The radix tree maps the upper portion of the TRB DMA address to a ring
+ * segment that has the same upper portion of DMA addresses. For example, say
I
+ * have segments of size 1KB, that are always 64-byte aligned. A segment may
+ * start at 0x10c91000 and end at 0x10c913f0. If I use the upper 10 bits, the
+ * key to the stream ID is 0x43244. I can use the DMA address of the TRB to
+ * pass the radix tree a key to get the right stream ID:
+ *
+ * 0x10c90fff 10 = 0x43243
+ * 0x10c912c0 10 = 0x43244
+ * 0x10c91400 10 = 0x43245
+ *
+ * Obviously, only those TRBs with DMA addresses that are within the segment
+ * will make the radix tree return the stream ID for that ring.
+ *
+ * Caveats for the radix tree:
+ *
+ * The radix tree uses an unsigned long as a key pair. On 32-bit systems, an
+ * unsigned long will be 32-bits; on a 64-bit system an unsigned long will be
+ * 64-bits. Since we only request 32-bit DMA addresses, we can use that as the
+ * key on 32-bit or 64-bit systems (it would also be fine if we asked for
64-bit
+ * PCI DMA addresses on a 64-bit system). There might be a problem on 32-bit
+ * extended systems (where the DMA address can be bigger than 32-bits),
+ * if we allow the PCI dma mask to be bigger than 32-bits. So don't do that.
+ */
+static int xhci_update_stream_mapping(struct xhci_ring *ring, gfp_t mem_flags)
+{
+ struct xhci_segment *seg;
+ unsigned long key;
+ int ret;
+
+ if (WARN_ON_ONCE(ring-trb_address_map == NULL))
+ return 0;
+
+ seg = ring-first_seg;
+ do {
+ key = (unsigned long)(seg-dma TRB_SEGMENT_SHIFT);
+ /* Skip any segments that were already added. */
+ if (radix_tree_lookup(ring-trb_address_map, key))
+ continue;
+
+ ret = radix_tree_maybe_preload(mem_flags);
+ if (ret)
+ return ret;
+ ret = radix_tree_insert(ring-trb_address_map,
+ key, ring);
+ radix_tree_preload_end();
+ if (ret)
+ return ret;
+ seg = seg-next;
+ } while (seg != ring-first_seg);
+
+ return 0;
+}
+
+static void
Re: [PATCH v3 3/6] uas: make work list per-device
On Di, 2013-09-17 at 13:30 -0700, Christoph Hellwig wrote: On Fri, Sep 13, 2013 at 01:27:12PM +0200, Gerd Hoffmann wrote: Simplifies locking, we'll protect the list with the device spin lock. Also plugs races which can happen when two devices operate on the global list. While being at it rename the list head from list to work, preparing for the addition of a second list. Why do you even the list? The list was already there when I took over maintainance ... What would a ordered per-device workqueue not provide? Had no reason to look into replacing the list with something else so far. Why do you suggest using a workqueue instead? Note that the list is not used in a typical request workflow. Only in case queuing an usb urb failed the request is linked into the list and the worker will try to submit the usb urb again. cheers, Gerd -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 1/6] xhci: fix usb3 streams
xhci maintains a radix tree for each stream endpoint because it must
be able to map a trb address to the stream ring. Each ring segment
must be added to the ring for this to work. Currently xhci sticks
only the first segment of each stream ring into the radix tree.
Result is that things work initially, but as soon as the first segment
is full xhci can't map the trb address from the completion event to the
stream ring any more - BOOM. You'll find this message in the logs:
ERROR Transfer event for disabled endpoint or incorrect stream ring
This patch adds a helper function to update the radix tree. It can
both insert and remove ring segments. It loops over the segment list
and handles all segments instead of just the first. It is called
whenever an update is needed: When allocating a ring, when expanding
a ring and when releasing a ring.
Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
drivers/usb/host/xhci-mem.c | 53 ++---
drivers/usb/host/xhci.h | 2 ++
2 files changed, 42 insertions(+), 13 deletions(-)
diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index 6f8c2fd..de8b006 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -154,8 +154,11 @@ void xhci_ring_free(struct xhci_hcd *xhci, struct
xhci_ring *ring)
if (!ring)
return;
- if (ring-first_seg)
+ if (ring-first_seg) {
+ if (ring-type == TYPE_STREAM)
+ xhci_update_stream_ring(ring, false);
xhci_free_segments_for_ring(xhci, ring-first_seg);
+ }
kfree(ring);
}
@@ -351,6 +354,11 @@ int xhci_ring_expansion(struct xhci_hcd *xhci, struct
xhci_ring *ring,
xhci_dbg(xhci, ring expansion succeed, now has %d segments\n,
ring-num_segs);
+ if (ring-type == TYPE_STREAM) {
+ ret = xhci_update_stream_ring(ring, true);
+ WARN_ON(ret); /* FIXME */
+ }
+
return 0;
}
@@ -601,6 +609,35 @@ static int xhci_test_radix_tree(struct xhci_hcd *xhci,
* extended systems (where the DMA address can be bigger than 32-bits),
* if we allow the PCI dma mask to be bigger than 32-bits. So don't do that.
*/
+
+int xhci_update_stream_ring(struct xhci_ring *ring, bool insert)
+{
+ struct xhci_segment *seg;
+ unsigned long key;
+ bool present;
+ int ret;
+
+ if (WARN_ON_ONCE(ring-trb_address_map == NULL))
+ return 0;
+
+ seg = ring-first_seg;
+ do {
+ key = (unsigned long)(seg-dma TRB_SEGMENT_SHIFT);
+ present = radix_tree_lookup(ring-trb_address_map, key) != NULL;
+ if (!present insert) {
+ ret = radix_tree_insert(ring-trb_address_map,
+ key, ring);
+ if (ret)
+ return ret;
+ }
+ if (present !insert)
+ radix_tree_delete(ring-trb_address_map, key);
+ seg = seg-next;
+ } while (seg != ring-first_seg);
+
+ return 0;
+}
+
struct xhci_stream_info *xhci_alloc_stream_info(struct xhci_hcd *xhci,
unsigned int num_stream_ctxs,
unsigned int num_streams, gfp_t mem_flags)
@@ -608,7 +645,6 @@ struct xhci_stream_info *xhci_alloc_stream_info(struct
xhci_hcd *xhci,
struct xhci_stream_info *stream_info;
u32 cur_stream;
struct xhci_ring *cur_ring;
- unsigned long key;
u64 addr;
int ret;
@@ -663,6 +699,7 @@ struct xhci_stream_info *xhci_alloc_stream_info(struct
xhci_hcd *xhci,
if (!cur_ring)
goto cleanup_rings;
cur_ring-stream_id = cur_stream;
+ cur_ring-trb_address_map = stream_info-trb_address_map;
/* Set deq ptr, cycle bit, and stream context type */
addr = cur_ring-first_seg-dma |
SCT_FOR_CTX(SCT_PRI_TR) |
@@ -672,10 +709,7 @@ struct xhci_stream_info *xhci_alloc_stream_info(struct
xhci_hcd *xhci,
xhci_dbg(xhci, Setting stream %d ring ptr to 0x%08llx\n,
cur_stream, (unsigned long long) addr);
- key = (unsigned long)
- (cur_ring-first_seg-dma TRB_SEGMENT_SHIFT);
- ret = radix_tree_insert(stream_info-trb_address_map,
- key, cur_ring);
+ ret = xhci_update_stream_ring(cur_ring, true);
if (ret) {
xhci_ring_free(xhci, cur_ring);
stream_info-stream_rings[cur_stream] = NULL;
@@ -702,9 +736,6 @@ cleanup_rings:
for (cur_stream = 1; cur_stream num_streams; cur_stream++) {
cur_ring = stream_info-stream_rings[cur_stream];
if (cur_ring) {
- addr
[PATCH v3 0/6] make uas fly
Hi, This patch series enables uas support in the linux kernel. First patch fixes usb3 streams support in xhci, patches 2-5 improve uas error handling and patch 6 removes BROKEN from the uas kernel config. New in v3: Fix the race pointed out by Oliver, on both existing code and the patch. Added patch which removes the BUG_ON()s cheers, Gerd Gerd Hoffmann (6): xhci: fix usb3 streams uas: properly reinitialize in uas_eh_bus_reset_handler uas: make work list per-device uas: add dead request list uas: replace BUG_ON() + WARN_ON() with WARN_ON_ONCE() uas: remove BROKEN drivers/usb/host/xhci-mem.c | 53 +++ drivers/usb/host/xhci.h | 2 + drivers/usb/storage/Kconfig | 2 +- drivers/usb/storage/uas.c | 158 +--- 4 files changed, 133 insertions(+), 82 deletions(-) -- 1.8.3.1 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 6/6] uas: remove BROKEN
xhci streams support is fixed, unblock usb attached scsi. Signed-off-by: Gerd Hoffmann kra...@redhat.com --- drivers/usb/storage/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/storage/Kconfig b/drivers/usb/storage/Kconfig index 8470e1b..4761a28 100644 --- a/drivers/usb/storage/Kconfig +++ b/drivers/usb/storage/Kconfig @@ -202,7 +202,7 @@ config USB_STORAGE_ENE_UB6250 config USB_UAS tristate USB Attached SCSI - depends on SCSI BROKEN + depends on SCSI help The USB Attached SCSI protocol is supported by some USB storage devices. It permits higher performance by supporting -- 1.8.3.1 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 5/6] uas: replace BUG_ON() + WARN_ON() with WARN_ON_ONCE()
Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
drivers/usb/storage/uas.c | 19 ++-
1 file changed, 10 insertions(+), 9 deletions(-)
diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index f049038..046eedf 100644
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -154,7 +154,7 @@ static void uas_abort_work(struct uas_dev_info *devinfo)
struct scsi_cmnd *cmnd = container_of(scp, struct scsi_cmnd,
SCp);
uas_log_cmd_state(cmnd, __func__);
- WARN_ON(cmdinfo-state COMMAND_ABORTED);
+ WARN_ON_ONCE(cmdinfo-state COMMAND_ABORTED);
cmdinfo-state |= COMMAND_ABORTED;
cmdinfo-state = ~IS_IN_WORK_LIST;
list_del(cmdinfo-work);
@@ -169,7 +169,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(!spin_is_locked(devinfo-lock));
+ WARN_ON_ONCE(!spin_is_locked(devinfo-lock));
list_add_tail(cmdinfo-work, devinfo-work_list);
cmdinfo-state |= IS_IN_WORK_LIST;
schedule_work(devinfo-work);
@@ -187,7 +187,7 @@ 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(!(cmdinfo-state COMMAND_ABORTED));
+ WARN_ON_ONCE(!(cmdinfo-state COMMAND_ABORTED));
/* all urbs are killed, clear inflight bits */
cmdinfo-state = ~(COMMAND_INFLIGHT |
DATA_IN_URB_INFLIGHT |
@@ -271,13 +271,13 @@ 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(!spin_is_locked(devinfo-lock));
+ WARN_ON_ONCE(!spin_is_locked(devinfo-lock));
if (cmdinfo-state (COMMAND_INFLIGHT |
DATA_IN_URB_INFLIGHT |
DATA_OUT_URB_INFLIGHT |
UNLINK_DATA_URBS))
return -EBUSY;
- BUG_ON(cmdinfo-state COMMAND_COMPLETED);
+ 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);
@@ -398,8 +398,9 @@ static void uas_data_cmplt(struct urb *urb)
sdb = scsi_out(cmnd);
cmdinfo-state = ~DATA_OUT_URB_INFLIGHT;
}
- BUG_ON(sdb == NULL);
- if (urb-status) {
+ if (sdb == NULL) {
+ WARN_ON_ONCE(1);
+ } else if (urb-status) {
/* error: no data transfered */
sdb-resid = sdb-length;
} else {
@@ -573,7 +574,7 @@ static int uas_submit_urbs(struct scsi_cmnd *cmnd,
struct uas_cmd_info *cmdinfo = (void *)cmnd-SCp;
int err;
- WARN_ON(!spin_is_locked(devinfo-lock));
+ WARN_ON_ONCE(!spin_is_locked(devinfo-lock));
if (cmdinfo-state SUBMIT_STATUS_URB) {
err = uas_submit_sense_urb(cmnd-device-host, gfp,
cmdinfo-stream);
@@ -771,7 +772,7 @@ static int uas_eh_abort_handler(struct scsi_cmnd *cmnd)
uas_log_cmd_state(cmnd, __func__);
spin_lock_irqsave(devinfo-lock, flags);
- WARN_ON(cmdinfo-state COMMAND_ABORTED);
+ WARN_ON_ONCE(cmdinfo-state COMMAND_ABORTED);
cmdinfo-state |= COMMAND_ABORTED;
list_add_tail(cmdinfo-dead, devinfo-dead_list);
if (cmdinfo-state IS_IN_WORK_LIST) {
--
1.8.3.1
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 4/6] uas: add dead request list
This patch adds a new list where all requests which are canceled are
added to, so we don't loose them. Then, after killing all inflight
urbs on bus reset (and disconnect) we'll walk over the list and clean
them up.
Without this we can end up with aborted requests lingering around in
case of status pipe transfer errors.
Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
drivers/usb/storage/uas.c | 50 +++
1 file changed, 42 insertions(+), 8 deletions(-)
diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index 3cf5a5f..f049038 100644
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -53,6 +53,7 @@ struct uas_dev_info {
spinlock_t lock;
struct work_struct work;
struct list_head work_list;
+ struct list_head dead_list;
};
enum {
@@ -80,6 +81,7 @@ struct uas_cmd_info {
struct urb *data_in_urb;
struct urb *data_out_urb;
struct list_head work;
+ struct list_head dead;
};
/* I hate forward declarations, but I actually have a loop */
@@ -89,6 +91,7 @@ static void uas_do_work(struct work_struct *work);
static int uas_try_complete(struct scsi_cmnd *cmnd, const char *caller);
static void uas_configure_endpoints(struct uas_dev_info *devinfo);
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_unlink_data_urbs(struct uas_dev_info *devinfo,
struct uas_cmd_info *cmdinfo)
@@ -150,16 +153,12 @@ static void uas_abort_work(struct uas_dev_info *devinfo)
struct scsi_pointer *scp = (void *)cmdinfo;
struct scsi_cmnd *cmnd = container_of(scp, struct scsi_cmnd,
SCp);
+ uas_log_cmd_state(cmnd, __func__);
+ WARN_ON(cmdinfo-state COMMAND_ABORTED);
cmdinfo-state |= COMMAND_ABORTED;
cmdinfo-state = ~IS_IN_WORK_LIST;
- if (devinfo-resetting) {
- /* uas_stat_cmplt() will not do that
-* when a device reset is in
-* progress */
- cmdinfo-state = ~COMMAND_INFLIGHT;
- }
- uas_try_complete(cmnd, __func__);
list_del(cmdinfo-work);
+ list_add_tail(cmdinfo-dead, devinfo-dead_list);
}
spin_unlock_irqrestore(devinfo-lock, flags);
}
@@ -176,6 +175,28 @@ static void uas_add_work(struct uas_cmd_info *cmdinfo)
schedule_work(devinfo-work);
}
+static void uas_zap_dead(struct uas_dev_info *devinfo)
+{
+ 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-dead_list, dead) {
+ struct scsi_pointer *scp = (void *)cmdinfo;
+ struct scsi_cmnd *cmnd = container_of(scp, struct scsi_cmnd,
+ SCp);
+ uas_log_cmd_state(cmnd, __func__);
+ WARN_ON(!(cmdinfo-state COMMAND_ABORTED));
+ /* all urbs are killed, clear inflight bits */
+ cmdinfo-state = ~(COMMAND_INFLIGHT |
+ DATA_IN_URB_INFLIGHT |
+ DATA_OUT_URB_INFLIGHT);
+ uas_try_complete(cmnd, __func__);
+ }
+ spin_unlock_irqrestore(devinfo-lock, flags);
+}
+
static void uas_sense(struct urb *urb, struct scsi_cmnd *cmnd)
{
struct sense_iu *sense_iu = urb-transfer_buffer;
@@ -263,6 +284,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);
cmnd-result = DID_ABORT 16;
+ list_del(cmdinfo-dead);
}
cmnd-scsi_done(cmnd);
return 0;
@@ -292,7 +314,13 @@ static void uas_stat_cmplt(struct urb *urb)
u16 tag;
if (urb-status) {
- dev_err(urb-dev-dev, URB BAD STATUS %d\n, urb-status);
+ if (urb-status == -ENOENT) {
+ dev_err(urb-dev-dev, stat urb: killed, stream %d\n,
+ urb-stream_id);
+ } else {
+ dev_err(urb-dev-dev, stat urb: status %d\n,
+ urb-status);
+ }
usb_free_urb(urb);
return;
}
@@ -743,7 +771,9 @@ static int uas_eh_abort_handler(struct scsi_cmnd *cmnd)
uas_log_cmd_state(cmnd, __func__);
spin_lock_irqsave(devinfo-lock, flags);
+ WARN_ON(cmdinfo-state COMMAND_ABORTED);
cmdinfo-state |= COMMAND_ABORTED;
+ list_add_tail(cmdinfo-dead, devinfo-dead_list);
if (cmdinfo-state
[PATCH v3 3/6] uas: make work list per-device
Simplifies locking, we'll protect the list with the device spin lock.
Also plugs races which can happen when two devices operate on the
global list.
While being at it rename the list head from list to work, preparing
for the addition of a second list.
Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
drivers/usb/storage/uas.c | 106 +++---
1 file changed, 44 insertions(+), 62 deletions(-)
diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index fc08ee9..3cf5a5f 100644
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -51,6 +51,8 @@ struct uas_dev_info {
unsigned uas_sense_old:1;
struct scsi_cmnd *cmnd;
spinlock_t lock;
+ struct work_struct work;
+ struct list_head work_list;
};
enum {
@@ -77,7 +79,7 @@ struct uas_cmd_info {
struct urb *cmd_urb;
struct urb *data_in_urb;
struct urb *data_out_urb;
- struct list_head list;
+ struct list_head work;
};
/* I hate forward declarations, but I actually have a loop */
@@ -88,10 +90,6 @@ static int uas_try_complete(struct scsi_cmnd *cmnd, const
char *caller);
static void uas_configure_endpoints(struct uas_dev_info *devinfo);
static void uas_free_streams(struct uas_dev_info *devinfo);
-static DECLARE_WORK(uas_work, uas_do_work);
-static DEFINE_SPINLOCK(uas_work_lock);
-static LIST_HEAD(uas_work_list);
-
static void uas_unlink_data_urbs(struct uas_dev_info *devinfo,
struct uas_cmd_info *cmdinfo)
{
@@ -118,75 +116,66 @@ static void uas_unlink_data_urbs(struct uas_dev_info
*devinfo,
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 uas_cmd_info *temp;
- struct list_head list;
unsigned long flags;
int err;
- spin_lock_irq(uas_work_lock);
- list_replace_init(uas_work_list, list);
- spin_unlock_irq(uas_work_lock);
-
- list_for_each_entry_safe(cmdinfo, temp, list, list) {
+ spin_lock_irqsave(devinfo-lock, flags);
+ list_for_each_entry_safe(cmdinfo, temp, devinfo-work_list, work) {
struct scsi_pointer *scp = (void *)cmdinfo;
- struct scsi_cmnd *cmnd = container_of(scp,
- struct scsi_cmnd, SCp);
- struct uas_dev_info *devinfo = (void *)cmnd-device-hostdata;
- spin_lock_irqsave(devinfo-lock, flags);
+ struct scsi_cmnd *cmnd = container_of(scp, struct scsi_cmnd,
+ SCp);
err = uas_submit_urbs(cmnd, cmnd-device-hostdata, GFP_ATOMIC);
- if (!err)
+ if (!err) {
cmdinfo-state = ~IS_IN_WORK_LIST;
- spin_unlock_irqrestore(devinfo-lock, flags);
- if (err) {
- list_del(cmdinfo-list);
- spin_lock_irq(uas_work_lock);
- list_add_tail(cmdinfo-list, uas_work_list);
- spin_unlock_irq(uas_work_lock);
- schedule_work(uas_work);
+ list_del(cmdinfo-work);
+ } else {
+ schedule_work(devinfo-work);
}
}
+ spin_unlock_irqrestore(devinfo-lock, flags);
}
static void uas_abort_work(struct uas_dev_info *devinfo)
{
struct uas_cmd_info *cmdinfo;
struct uas_cmd_info *temp;
- struct list_head list;
unsigned long flags;
- spin_lock_irq(uas_work_lock);
- list_replace_init(uas_work_list, list);
- spin_unlock_irq(uas_work_lock);
-
spin_lock_irqsave(devinfo-lock, flags);
- list_for_each_entry_safe(cmdinfo, temp, list, list) {
+ list_for_each_entry_safe(cmdinfo, temp, devinfo-work_list, work) {
struct scsi_pointer *scp = (void *)cmdinfo;
- struct scsi_cmnd *cmnd = container_of(scp,
- struct scsi_cmnd, SCp);
- struct uas_dev_info *di = (void *)cmnd-device-hostdata;
-
- if (di == devinfo) {
- cmdinfo-state |= COMMAND_ABORTED;
- cmdinfo-state = ~IS_IN_WORK_LIST;
- if (devinfo-resetting) {
- /* uas_stat_cmplt() will not do that
-* when a device reset is in
-* progress */
- cmdinfo-state = ~COMMAND_INFLIGHT;
- }
- uas_try_complete(cmnd, __func__);
- } else {
- /* not our uas device, relink into list */
- list_del(cmdinfo-list);
- spin_lock_irq
[PATCH v3 2/6] uas: properly reinitialize in uas_eh_bus_reset_handler
Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
drivers/usb/storage/uas.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index d966b59..fc08ee9 100644
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -85,6 +85,8 @@ static int uas_submit_urbs(struct scsi_cmnd *cmnd,
struct uas_dev_info *devinfo, gfp_t gfp);
static void uas_do_work(struct work_struct *work);
static int uas_try_complete(struct scsi_cmnd *cmnd, const char *caller);
+static void uas_configure_endpoints(struct uas_dev_info *devinfo);
+static void uas_free_streams(struct uas_dev_info *devinfo);
static DECLARE_WORK(uas_work, uas_do_work);
static DEFINE_SPINLOCK(uas_work_lock);
@@ -800,7 +802,10 @@ static int uas_eh_bus_reset_handler(struct scsi_cmnd *cmnd)
usb_kill_anchored_urbs(devinfo-cmd_urbs);
usb_kill_anchored_urbs(devinfo-sense_urbs);
usb_kill_anchored_urbs(devinfo-data_urbs);
+ uas_free_streams(devinfo);
err = usb_reset_device(udev);
+ if (!err)
+ uas_configure_endpoints(devinfo);
devinfo-resetting = 0;
if (err) {
--
1.8.3.1
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 0/5] make uas fly
Hi, This patch series enables uas support in the linux kernel. First patch fixes usb3 streams support in xhci, patches 2-4 improve uas error handling and patch 5 removes BROKEN from the uas kernel config. v2 changes: - use WARN_ON_ONCE() instead of BUG_ON() - make checkpatch happy (codestyle fixes). cheers, Gerd Gerd Hoffmann (5): xhci: fix usb3 streams uas: properly reinitialize in uas_eh_bus_reset_handler uas: rename work list lock + list field uas: add dead request list uas: remove BROKEN drivers/usb/host/xhci-mem.c | 53 ++- drivers/usb/host/xhci.h | 2 + drivers/usb/storage/Kconfig | 2 +- drivers/usb/storage/uas.c | 123 4 files changed, 134 insertions(+), 46 deletions(-) -- 1.8.3.1 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/5] xhci: fix usb3 streams
xhci maintains a radix tree for each stream endpoint because it must
be able to map a trb address to the stream ring. Each ring segment
must be added to the ring for this to work. Currently xhci sticks
only the first segment of each stream ring into the radix tree.
Result is that things work initially, but as soon as the first segment
is full xhci can't map the trb address from the completion event to the
stream ring any more - BOOM. You'll find this message in the logs:
ERROR Transfer event for disabled endpoint or incorrect stream ring
This patch adds a helper function to update the radix tree. It can
both insert and remove ring segments. It loops over the segment list
and handles all segments instead of just the first. It is called
whenever an update is needed: When allocating a ring, when expanding
a ring and when releasing a ring.
Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
drivers/usb/host/xhci-mem.c | 53 ++---
drivers/usb/host/xhci.h | 2 ++
2 files changed, 42 insertions(+), 13 deletions(-)
diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index 6f8c2fd..de8b006 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -154,8 +154,11 @@ void xhci_ring_free(struct xhci_hcd *xhci, struct
xhci_ring *ring)
if (!ring)
return;
- if (ring-first_seg)
+ if (ring-first_seg) {
+ if (ring-type == TYPE_STREAM)
+ xhci_update_stream_ring(ring, false);
xhci_free_segments_for_ring(xhci, ring-first_seg);
+ }
kfree(ring);
}
@@ -351,6 +354,11 @@ int xhci_ring_expansion(struct xhci_hcd *xhci, struct
xhci_ring *ring,
xhci_dbg(xhci, ring expansion succeed, now has %d segments\n,
ring-num_segs);
+ if (ring-type == TYPE_STREAM) {
+ ret = xhci_update_stream_ring(ring, true);
+ WARN_ON(ret); /* FIXME */
+ }
+
return 0;
}
@@ -601,6 +609,35 @@ static int xhci_test_radix_tree(struct xhci_hcd *xhci,
* extended systems (where the DMA address can be bigger than 32-bits),
* if we allow the PCI dma mask to be bigger than 32-bits. So don't do that.
*/
+
+int xhci_update_stream_ring(struct xhci_ring *ring, bool insert)
+{
+ struct xhci_segment *seg;
+ unsigned long key;
+ bool present;
+ int ret;
+
+ if (WARN_ON_ONCE(ring-trb_address_map == NULL))
+ return 0;
+
+ seg = ring-first_seg;
+ do {
+ key = (unsigned long)(seg-dma TRB_SEGMENT_SHIFT);
+ present = radix_tree_lookup(ring-trb_address_map, key) != NULL;
+ if (!present insert) {
+ ret = radix_tree_insert(ring-trb_address_map,
+ key, ring);
+ if (ret)
+ return ret;
+ }
+ if (present !insert)
+ radix_tree_delete(ring-trb_address_map, key);
+ seg = seg-next;
+ } while (seg != ring-first_seg);
+
+ return 0;
+}
+
struct xhci_stream_info *xhci_alloc_stream_info(struct xhci_hcd *xhci,
unsigned int num_stream_ctxs,
unsigned int num_streams, gfp_t mem_flags)
@@ -608,7 +645,6 @@ struct xhci_stream_info *xhci_alloc_stream_info(struct
xhci_hcd *xhci,
struct xhci_stream_info *stream_info;
u32 cur_stream;
struct xhci_ring *cur_ring;
- unsigned long key;
u64 addr;
int ret;
@@ -663,6 +699,7 @@ struct xhci_stream_info *xhci_alloc_stream_info(struct
xhci_hcd *xhci,
if (!cur_ring)
goto cleanup_rings;
cur_ring-stream_id = cur_stream;
+ cur_ring-trb_address_map = stream_info-trb_address_map;
/* Set deq ptr, cycle bit, and stream context type */
addr = cur_ring-first_seg-dma |
SCT_FOR_CTX(SCT_PRI_TR) |
@@ -672,10 +709,7 @@ struct xhci_stream_info *xhci_alloc_stream_info(struct
xhci_hcd *xhci,
xhci_dbg(xhci, Setting stream %d ring ptr to 0x%08llx\n,
cur_stream, (unsigned long long) addr);
- key = (unsigned long)
- (cur_ring-first_seg-dma TRB_SEGMENT_SHIFT);
- ret = radix_tree_insert(stream_info-trb_address_map,
- key, cur_ring);
+ ret = xhci_update_stream_ring(cur_ring, true);
if (ret) {
xhci_ring_free(xhci, cur_ring);
stream_info-stream_rings[cur_stream] = NULL;
@@ -702,9 +736,6 @@ cleanup_rings:
for (cur_stream = 1; cur_stream num_streams; cur_stream++) {
cur_ring = stream_info-stream_rings[cur_stream];
if (cur_ring) {
- addr
[PATCH 3/5] uas: rename work list lock + list field
This patch prepares for the addition of another list and renames the
work list lock and the list_head field in struct uas_cmd_info.
Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
drivers/usb/storage/uas.c | 50 +++
1 file changed, 25 insertions(+), 25 deletions(-)
diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index fc08ee9..db09bda 100644
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -77,7 +77,7 @@ struct uas_cmd_info {
struct urb *cmd_urb;
struct urb *data_in_urb;
struct urb *data_out_urb;
- struct list_head list;
+ struct list_head work;
};
/* I hate forward declarations, but I actually have a loop */
@@ -89,7 +89,7 @@ static void uas_configure_endpoints(struct uas_dev_info
*devinfo);
static void uas_free_streams(struct uas_dev_info *devinfo);
static DECLARE_WORK(uas_work, uas_do_work);
-static DEFINE_SPINLOCK(uas_work_lock);
+static DEFINE_SPINLOCK(uas_lists_lock);
static LIST_HEAD(uas_work_list);
static void uas_unlink_data_urbs(struct uas_dev_info *devinfo,
@@ -124,11 +124,11 @@ static void uas_do_work(struct work_struct *work)
unsigned long flags;
int err;
- spin_lock_irq(uas_work_lock);
+ spin_lock_irq(uas_lists_lock);
list_replace_init(uas_work_list, list);
- spin_unlock_irq(uas_work_lock);
+ spin_unlock_irq(uas_lists_lock);
- list_for_each_entry_safe(cmdinfo, temp, list, list) {
+ list_for_each_entry_safe(cmdinfo, temp, list, work) {
struct scsi_pointer *scp = (void *)cmdinfo;
struct scsi_cmnd *cmnd = container_of(scp,
struct scsi_cmnd, SCp);
@@ -139,10 +139,10 @@ static void uas_do_work(struct work_struct *work)
cmdinfo-state = ~IS_IN_WORK_LIST;
spin_unlock_irqrestore(devinfo-lock, flags);
if (err) {
- list_del(cmdinfo-list);
- spin_lock_irq(uas_work_lock);
- list_add_tail(cmdinfo-list, uas_work_list);
- spin_unlock_irq(uas_work_lock);
+ list_del(cmdinfo-work);
+ spin_lock_irq(uas_lists_lock);
+ list_add_tail(cmdinfo-work, uas_work_list);
+ spin_unlock_irq(uas_lists_lock);
schedule_work(uas_work);
}
}
@@ -155,12 +155,12 @@ static void uas_abort_work(struct uas_dev_info *devinfo)
struct list_head list;
unsigned long flags;
- spin_lock_irq(uas_work_lock);
+ spin_lock_irq(uas_lists_lock);
list_replace_init(uas_work_list, list);
- spin_unlock_irq(uas_work_lock);
+ spin_unlock_irq(uas_lists_lock);
spin_lock_irqsave(devinfo-lock, flags);
- list_for_each_entry_safe(cmdinfo, temp, list, list) {
+ list_for_each_entry_safe(cmdinfo, temp, list, work) {
struct scsi_pointer *scp = (void *)cmdinfo;
struct scsi_cmnd *cmnd = container_of(scp,
struct scsi_cmnd, SCp);
@@ -178,10 +178,10 @@ static void uas_abort_work(struct uas_dev_info *devinfo)
uas_try_complete(cmnd, __func__);
} else {
/* not our uas device, relink into list */
- list_del(cmdinfo-list);
- spin_lock_irq(uas_work_lock);
- list_add_tail(cmdinfo-list, uas_work_list);
- spin_unlock_irq(uas_work_lock);
+ list_del(cmdinfo-work);
+ spin_lock_irq(uas_lists_lock);
+ list_add_tail(cmdinfo-work, uas_work_list);
+ spin_unlock_irq(uas_lists_lock);
}
}
spin_unlock_irqrestore(devinfo-lock, flags);
@@ -288,10 +288,10 @@ static void uas_xfer_data(struct urb *urb, struct
scsi_cmnd *cmnd,
cmdinfo-state |= direction | SUBMIT_STATUS_URB;
err = uas_submit_urbs(cmnd, cmnd-device-hostdata, GFP_ATOMIC);
if (err) {
- spin_lock(uas_work_lock);
- list_add_tail(cmdinfo-list, uas_work_list);
+ spin_lock(uas_lists_lock);
+ list_add_tail(cmdinfo-work, uas_work_list);
cmdinfo-state |= IS_IN_WORK_LIST;
- spin_unlock(uas_work_lock);
+ spin_unlock(uas_lists_lock);
schedule_work(uas_work);
}
}
@@ -694,10 +694,10 @@ static int uas_queuecommand_lck(struct scsi_cmnd *cmnd,
spin_unlock_irqrestore(devinfo-lock, flags);
return SCSI_MLQUEUE_DEVICE_BUSY;
}
- spin_lock(uas_work_lock);
- list_add_tail(cmdinfo-list, uas_work_list);
+ spin_lock
[PATCH 5/5] uas: remove BROKEN
xhci streams support is fixed, unblock usb attached scsi. Signed-off-by: Gerd Hoffmann kra...@redhat.com --- drivers/usb/storage/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/storage/Kconfig b/drivers/usb/storage/Kconfig index 8470e1b..4761a28 100644 --- a/drivers/usb/storage/Kconfig +++ b/drivers/usb/storage/Kconfig @@ -202,7 +202,7 @@ config USB_STORAGE_ENE_UB6250 config USB_UAS tristate USB Attached SCSI - depends on SCSI BROKEN + depends on SCSI help The USB Attached SCSI protocol is supported by some USB storage devices. It permits higher performance by supporting -- 1.8.3.1 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/5] uas: add dead request list
This patch adds a new list where all requests which are canceled are
added to, so we don't loose them. Then, after killing all inflight
urbs on bus reset (and disconnect) we'll walk over the list and clean
them up.
Without this we can end up with aborted requests lingering around in
case of status pipe transfer errors.
Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
drivers/usb/storage/uas.c | 70 +--
1 file changed, 62 insertions(+), 8 deletions(-)
diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index db09bda..2b3ca29 100644
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -78,6 +78,7 @@ struct uas_cmd_info {
struct urb *data_in_urb;
struct urb *data_out_urb;
struct list_head work;
+ struct list_head dead;
};
/* I hate forward declarations, but I actually have a loop */
@@ -87,10 +88,12 @@ static void uas_do_work(struct work_struct *work);
static int uas_try_complete(struct scsi_cmnd *cmnd, const char *caller);
static void uas_configure_endpoints(struct uas_dev_info *devinfo);
static void uas_free_streams(struct uas_dev_info *devinfo);
+static void uas_log_cmd_state(struct scsi_cmnd *cmnd, const char *caller);
static DECLARE_WORK(uas_work, uas_do_work);
static DEFINE_SPINLOCK(uas_lists_lock);
static LIST_HEAD(uas_work_list);
+static LIST_HEAD(uas_dead_list);
static void uas_unlink_data_urbs(struct uas_dev_info *devinfo,
struct uas_cmd_info *cmdinfo)
@@ -167,15 +170,13 @@ static void uas_abort_work(struct uas_dev_info *devinfo)
struct uas_dev_info *di = (void *)cmnd-device-hostdata;
if (di == devinfo) {
+ uas_log_cmd_state(cmnd, __func__);
+ BUG_ON(cmdinfo-state COMMAND_ABORTED);
cmdinfo-state |= COMMAND_ABORTED;
+ spin_lock_irq(uas_lists_lock);
+ list_add_tail(cmdinfo-dead, uas_dead_list);
+ spin_unlock_irq(uas_lists_lock);
cmdinfo-state = ~IS_IN_WORK_LIST;
- if (devinfo-resetting) {
- /* uas_stat_cmplt() will not do that
-* when a device reset is in
-* progress */
- cmdinfo-state = ~COMMAND_INFLIGHT;
- }
- uas_try_complete(cmnd, __func__);
} else {
/* not our uas device, relink into list */
list_del(cmdinfo-work);
@@ -187,6 +188,43 @@ static void uas_abort_work(struct uas_dev_info *devinfo)
spin_unlock_irqrestore(devinfo-lock, flags);
}
+static void uas_zap_dead(struct uas_dev_info *devinfo)
+{
+ struct uas_cmd_info *cmdinfo;
+ struct uas_cmd_info *temp;
+ struct list_head list;
+ unsigned long flags;
+
+ spin_lock_irq(uas_lists_lock);
+ list_replace_init(uas_dead_list, list);
+ spin_unlock_irq(uas_lists_lock);
+
+ spin_lock_irqsave(devinfo-lock, flags);
+ list_for_each_entry_safe(cmdinfo, temp, list, dead) {
+ struct scsi_pointer *scp = (void *)cmdinfo;
+ struct scsi_cmnd *cmnd = container_of(scp,
+ struct scsi_cmnd, SCp);
+ struct uas_dev_info *di = (void *)cmnd-device-hostdata;
+
+ if (di == devinfo) {
+ uas_log_cmd_state(cmnd, __func__);
+ BUG_ON(!(cmdinfo-state COMMAND_ABORTED));
+ /* all urbs are killed, clear inflight bits */
+ cmdinfo-state = ~(COMMAND_INFLIGHT |
+ DATA_IN_URB_INFLIGHT |
+ DATA_OUT_URB_INFLIGHT);
+ uas_try_complete(cmnd, __func__);
+ } else {
+ /* not our uas device, relink into list */
+ list_del(cmdinfo-dead);
+ spin_lock_irq(uas_lists_lock);
+ list_add_tail(cmdinfo-dead, uas_dead_list);
+ spin_unlock_irq(uas_lists_lock);
+ }
+ }
+ spin_unlock_irqrestore(devinfo-lock, flags);
+}
+
static void uas_sense(struct urb *urb, struct scsi_cmnd *cmnd)
{
struct sense_iu *sense_iu = urb-transfer_buffer;
@@ -274,6 +312,9 @@ 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);
cmnd-result = DID_ABORT 16;
+ spin_lock_irq(uas_lists_lock);
+ list_del(cmdinfo-dead);
+ spin_unlock_irq(uas_lists_lock);
}
cmnd-scsi_done(cmnd);
return 0;
@@ -307,7 +348,13 @@ static
[PATCH 2/5] uas: properly reinitialize in uas_eh_bus_reset_handler
Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
drivers/usb/storage/uas.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index d966b59..fc08ee9 100644
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -85,6 +85,8 @@ static int uas_submit_urbs(struct scsi_cmnd *cmnd,
struct uas_dev_info *devinfo, gfp_t gfp);
static void uas_do_work(struct work_struct *work);
static int uas_try_complete(struct scsi_cmnd *cmnd, const char *caller);
+static void uas_configure_endpoints(struct uas_dev_info *devinfo);
+static void uas_free_streams(struct uas_dev_info *devinfo);
static DECLARE_WORK(uas_work, uas_do_work);
static DEFINE_SPINLOCK(uas_work_lock);
@@ -800,7 +802,10 @@ static int uas_eh_bus_reset_handler(struct scsi_cmnd *cmnd)
usb_kill_anchored_urbs(devinfo-cmd_urbs);
usb_kill_anchored_urbs(devinfo-sense_urbs);
usb_kill_anchored_urbs(devinfo-data_urbs);
+ uas_free_streams(devinfo);
err = usb_reset_device(udev);
+ if (!err)
+ uas_configure_endpoints(devinfo);
devinfo-resetting = 0;
if (err) {
--
1.8.3.1
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/5] make uas fly
Hi, This patch series enables uas support in the linux kernel. First patch fixes usb3 streams support in xhci, patches 2-4 improve uas error handling and patch 5 removes BROKEN from the uas kernel config. cheers, Gerd Gerd Hoffmann (5): xhci: fix usb3 streams uas: properly reinitialize in uas_eh_bus_reset_handler uas: rename work list lock + list field uas: add dead request list uas: remove BROKEN drivers/usb/host/xhci-mem.c | 51 +- drivers/usb/host/xhci.h | 2 + drivers/usb/storage/Kconfig | 2 +- drivers/usb/storage/uas.c | 123 4 files changed, 132 insertions(+), 46 deletions(-) -- 1.8.3.1 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 5/5] uas: remove BROKEN
xhci streams support is fixed, unblock usb attached scsi. Signed-off-by: Gerd Hoffmann kra...@redhat.com --- drivers/usb/storage/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/storage/Kconfig b/drivers/usb/storage/Kconfig index 8470e1b..4761a28 100644 --- a/drivers/usb/storage/Kconfig +++ b/drivers/usb/storage/Kconfig @@ -202,7 +202,7 @@ config USB_STORAGE_ENE_UB6250 config USB_UAS tristate USB Attached SCSI - depends on SCSI BROKEN + depends on SCSI help The USB Attached SCSI protocol is supported by some USB storage devices. It permits higher performance by supporting -- 1.8.3.1 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/5] uas: properly reinitialize in uas_eh_bus_reset_handler
Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
drivers/usb/storage/uas.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index d966b59..f89202f 100644
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -85,6 +85,8 @@ static int uas_submit_urbs(struct scsi_cmnd *cmnd,
struct uas_dev_info *devinfo, gfp_t gfp);
static void uas_do_work(struct work_struct *work);
static int uas_try_complete(struct scsi_cmnd *cmnd, const char *caller);
+static void uas_configure_endpoints(struct uas_dev_info *devinfo);
+static void uas_free_streams(struct uas_dev_info *devinfo);
static DECLARE_WORK(uas_work, uas_do_work);
static DEFINE_SPINLOCK(uas_work_lock);
@@ -800,7 +802,11 @@ static int uas_eh_bus_reset_handler(struct scsi_cmnd *cmnd)
usb_kill_anchored_urbs(devinfo-cmd_urbs);
usb_kill_anchored_urbs(devinfo-sense_urbs);
usb_kill_anchored_urbs(devinfo-data_urbs);
+ uas_free_streams(devinfo);
err = usb_reset_device(udev);
+ if (!err) {
+ uas_configure_endpoints(devinfo);
+ }
devinfo-resetting = 0;
if (err) {
--
1.8.3.1
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/5] uas: add dead request list
This patch adds a new list where all requests which are canceled are
added to, so we don't loose them. Then, after killing all inflight
urbs on bus reset (and disconnect) we'll walk over the list and clean
them up.
Without this we can end up with aborted requests lingering around in
case of status pipe transfer errors.
Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
drivers/usb/storage/uas.c | 69 +--
1 file changed, 61 insertions(+), 8 deletions(-)
diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index a63972a..9dfb8f9 100644
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -78,6 +78,7 @@ struct uas_cmd_info {
struct urb *data_in_urb;
struct urb *data_out_urb;
struct list_head work;
+ struct list_head dead;
};
/* I hate forward declarations, but I actually have a loop */
@@ -87,10 +88,12 @@ static void uas_do_work(struct work_struct *work);
static int uas_try_complete(struct scsi_cmnd *cmnd, const char *caller);
static void uas_configure_endpoints(struct uas_dev_info *devinfo);
static void uas_free_streams(struct uas_dev_info *devinfo);
+static void uas_log_cmd_state(struct scsi_cmnd *cmnd, const char *caller);
static DECLARE_WORK(uas_work, uas_do_work);
static DEFINE_SPINLOCK(uas_lists_lock);
static LIST_HEAD(uas_work_list);
+static LIST_HEAD(uas_dead_list);
static void uas_unlink_data_urbs(struct uas_dev_info *devinfo,
struct uas_cmd_info *cmdinfo)
@@ -167,15 +170,13 @@ static void uas_abort_work(struct uas_dev_info *devinfo)
struct uas_dev_info *di = (void *)cmnd-device-hostdata;
if (di == devinfo) {
+ uas_log_cmd_state(cmnd, __func__);
+ BUG_ON(cmdinfo-state COMMAND_ABORTED);
cmdinfo-state |= COMMAND_ABORTED;
+ spin_lock_irq(uas_lists_lock);
+ list_add_tail(cmdinfo-dead, uas_dead_list);
+ spin_unlock_irq(uas_lists_lock);
cmdinfo-state = ~IS_IN_WORK_LIST;
- if (devinfo-resetting) {
- /* uas_stat_cmplt() will not do that
-* when a device reset is in
-* progress */
- cmdinfo-state = ~COMMAND_INFLIGHT;
- }
- uas_try_complete(cmnd, __func__);
} else {
/* not our uas device, relink into list */
list_del(cmdinfo-work);
@@ -187,6 +188,43 @@ static void uas_abort_work(struct uas_dev_info *devinfo)
spin_unlock_irqrestore(devinfo-lock, flags);
}
+static void uas_zap_dead(struct uas_dev_info *devinfo)
+{
+ struct uas_cmd_info *cmdinfo;
+ struct uas_cmd_info *temp;
+ struct list_head list;
+ unsigned long flags;
+
+ spin_lock_irq(uas_lists_lock);
+ list_replace_init(uas_dead_list, list);
+ spin_unlock_irq(uas_lists_lock);
+
+ spin_lock_irqsave(devinfo-lock, flags);
+ list_for_each_entry_safe(cmdinfo, temp, list, dead) {
+ struct scsi_pointer *scp = (void *)cmdinfo;
+ struct scsi_cmnd *cmnd = container_of(scp,
+ struct scsi_cmnd, SCp);
+ struct uas_dev_info *di = (void *)cmnd-device-hostdata;
+
+ if (di == devinfo) {
+ uas_log_cmd_state(cmnd, __func__);
+ BUG_ON(!(cmdinfo-state COMMAND_ABORTED));
+ /* all urbs are killed, clear inflight bits */
+ cmdinfo-state = ~(COMMAND_INFLIGHT |
+ DATA_IN_URB_INFLIGHT |
+ DATA_OUT_URB_INFLIGHT);
+ uas_try_complete(cmnd, __func__);
+ } else {
+ /* not our uas device, relink into list */
+ list_del(cmdinfo-dead);
+ spin_lock_irq(uas_lists_lock);
+ list_add_tail(cmdinfo-dead, uas_dead_list);
+ spin_unlock_irq(uas_lists_lock);
+ }
+ }
+ spin_unlock_irqrestore(devinfo-lock, flags);
+}
+
static void uas_sense(struct urb *urb, struct scsi_cmnd *cmnd)
{
struct sense_iu *sense_iu = urb-transfer_buffer;
@@ -274,6 +312,9 @@ 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);
cmnd-result = DID_ABORT 16;
+ spin_lock_irq(uas_lists_lock);
+ list_del(cmdinfo-dead);
+ spin_unlock_irq(uas_lists_lock);
}
cmnd-scsi_done(cmnd);
return 0;
@@ -307,7 +348,12 @@ static
[PATCH 1/5] xhci: fix usb3 streams
xhci maintains a radix tree for each stream endpoint because it must
be able to map a trb address to the stream ring. Each ring segment
must be added to the ring for this to work. Currently xhci sticks
only the first segment of each stream ring into the radix tree.
Result is that things work initially, but as soon as the first segment
is full xhci can't map the trb address from the completion event to the
stream ring any more - BOOM. You'll find this message in the logs:
ERROR Transfer event for disabled endpoint or incorrect stream ring
This patch adds a helper function to update the radix tree. It can
both insert and remove ring segments. It loops over the segment list
and handles all segments instead of just the first. It is called
whenever an update is needed: When allocating a ring, when expanding
a ring and when releasing a ring.
Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
drivers/usb/host/xhci-mem.c | 51 +
drivers/usb/host/xhci.h | 2 ++
2 files changed, 40 insertions(+), 13 deletions(-)
diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index 6f8c2fd..c7fd88f 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -154,8 +154,11 @@ void xhci_ring_free(struct xhci_hcd *xhci, struct
xhci_ring *ring)
if (!ring)
return;
- if (ring-first_seg)
+ if (ring-first_seg) {
+ if (ring-type == TYPE_STREAM)
+ xhci_update_stream_ring(ring, false);
xhci_free_segments_for_ring(xhci, ring-first_seg);
+ }
kfree(ring);
}
@@ -351,6 +354,11 @@ int xhci_ring_expansion(struct xhci_hcd *xhci, struct
xhci_ring *ring,
xhci_dbg(xhci, ring expansion succeed, now has %d segments\n,
ring-num_segs);
+ if (ring-type == TYPE_STREAM) {
+ ret = xhci_update_stream_ring(ring, true);
+ WARN_ON(ret); /* FIXME */
+ }
+
return 0;
}
@@ -601,6 +609,33 @@ static int xhci_test_radix_tree(struct xhci_hcd *xhci,
* extended systems (where the DMA address can be bigger than 32-bits),
* if we allow the PCI dma mask to be bigger than 32-bits. So don't do that.
*/
+
+int xhci_update_stream_ring(struct xhci_ring *ring, bool insert)
+{
+ struct xhci_segment *seg;
+ unsigned long key;
+ bool present;
+ int ret;
+
+ BUG_ON(ring-trb_address_map == NULL);
+ seg = ring-first_seg;
+ do {
+ key = (unsigned long)(seg-dma TRB_SEGMENT_SHIFT);
+ present = radix_tree_lookup(ring-trb_address_map, key) != NULL;
+ if (!present insert) {
+ ret = radix_tree_insert(ring-trb_address_map, key,
ring);
+ if (ret)
+ return ret;
+ }
+ if (present !insert) {
+ radix_tree_delete(ring-trb_address_map, key);
+ }
+ seg = seg-next;
+ } while (seg != ring-first_seg);
+
+ return 0;
+}
+
struct xhci_stream_info *xhci_alloc_stream_info(struct xhci_hcd *xhci,
unsigned int num_stream_ctxs,
unsigned int num_streams, gfp_t mem_flags)
@@ -608,7 +643,6 @@ struct xhci_stream_info *xhci_alloc_stream_info(struct
xhci_hcd *xhci,
struct xhci_stream_info *stream_info;
u32 cur_stream;
struct xhci_ring *cur_ring;
- unsigned long key;
u64 addr;
int ret;
@@ -663,6 +697,7 @@ struct xhci_stream_info *xhci_alloc_stream_info(struct
xhci_hcd *xhci,
if (!cur_ring)
goto cleanup_rings;
cur_ring-stream_id = cur_stream;
+ cur_ring-trb_address_map = stream_info-trb_address_map;
/* Set deq ptr, cycle bit, and stream context type */
addr = cur_ring-first_seg-dma |
SCT_FOR_CTX(SCT_PRI_TR) |
@@ -672,10 +707,7 @@ struct xhci_stream_info *xhci_alloc_stream_info(struct
xhci_hcd *xhci,
xhci_dbg(xhci, Setting stream %d ring ptr to 0x%08llx\n,
cur_stream, (unsigned long long) addr);
- key = (unsigned long)
- (cur_ring-first_seg-dma TRB_SEGMENT_SHIFT);
- ret = radix_tree_insert(stream_info-trb_address_map,
- key, cur_ring);
+ ret = xhci_update_stream_ring(cur_ring, true);
if (ret) {
xhci_ring_free(xhci, cur_ring);
stream_info-stream_rings[cur_stream] = NULL;
@@ -702,9 +734,6 @@ cleanup_rings:
for (cur_stream = 1; cur_stream num_streams; cur_stream++) {
cur_ring = stream_info-stream_rings[cur_stream];
if (cur_ring) {
- addr = cur_ring-first_seg-dma;
- radix_tree_delete
[PATCH 3/5] uas: rename work list lock + list field
This patch prepares for the addition of another list and renames the
work list lock and the list_head field in struct uas_cmd_info.
Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
drivers/usb/storage/uas.c | 50 +++
1 file changed, 25 insertions(+), 25 deletions(-)
diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index f89202f..a63972a 100644
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -77,7 +77,7 @@ struct uas_cmd_info {
struct urb *cmd_urb;
struct urb *data_in_urb;
struct urb *data_out_urb;
- struct list_head list;
+ struct list_head work;
};
/* I hate forward declarations, but I actually have a loop */
@@ -89,7 +89,7 @@ static void uas_configure_endpoints(struct uas_dev_info
*devinfo);
static void uas_free_streams(struct uas_dev_info *devinfo);
static DECLARE_WORK(uas_work, uas_do_work);
-static DEFINE_SPINLOCK(uas_work_lock);
+static DEFINE_SPINLOCK(uas_lists_lock);
static LIST_HEAD(uas_work_list);
static void uas_unlink_data_urbs(struct uas_dev_info *devinfo,
@@ -124,11 +124,11 @@ static void uas_do_work(struct work_struct *work)
unsigned long flags;
int err;
- spin_lock_irq(uas_work_lock);
+ spin_lock_irq(uas_lists_lock);
list_replace_init(uas_work_list, list);
- spin_unlock_irq(uas_work_lock);
+ spin_unlock_irq(uas_lists_lock);
- list_for_each_entry_safe(cmdinfo, temp, list, list) {
+ list_for_each_entry_safe(cmdinfo, temp, list, work) {
struct scsi_pointer *scp = (void *)cmdinfo;
struct scsi_cmnd *cmnd = container_of(scp,
struct scsi_cmnd, SCp);
@@ -139,10 +139,10 @@ static void uas_do_work(struct work_struct *work)
cmdinfo-state = ~IS_IN_WORK_LIST;
spin_unlock_irqrestore(devinfo-lock, flags);
if (err) {
- list_del(cmdinfo-list);
- spin_lock_irq(uas_work_lock);
- list_add_tail(cmdinfo-list, uas_work_list);
- spin_unlock_irq(uas_work_lock);
+ list_del(cmdinfo-work);
+ spin_lock_irq(uas_lists_lock);
+ list_add_tail(cmdinfo-work, uas_work_list);
+ spin_unlock_irq(uas_lists_lock);
schedule_work(uas_work);
}
}
@@ -155,12 +155,12 @@ static void uas_abort_work(struct uas_dev_info *devinfo)
struct list_head list;
unsigned long flags;
- spin_lock_irq(uas_work_lock);
+ spin_lock_irq(uas_lists_lock);
list_replace_init(uas_work_list, list);
- spin_unlock_irq(uas_work_lock);
+ spin_unlock_irq(uas_lists_lock);
spin_lock_irqsave(devinfo-lock, flags);
- list_for_each_entry_safe(cmdinfo, temp, list, list) {
+ list_for_each_entry_safe(cmdinfo, temp, list, work) {
struct scsi_pointer *scp = (void *)cmdinfo;
struct scsi_cmnd *cmnd = container_of(scp,
struct scsi_cmnd, SCp);
@@ -178,10 +178,10 @@ static void uas_abort_work(struct uas_dev_info *devinfo)
uas_try_complete(cmnd, __func__);
} else {
/* not our uas device, relink into list */
- list_del(cmdinfo-list);
- spin_lock_irq(uas_work_lock);
- list_add_tail(cmdinfo-list, uas_work_list);
- spin_unlock_irq(uas_work_lock);
+ list_del(cmdinfo-work);
+ spin_lock_irq(uas_lists_lock);
+ list_add_tail(cmdinfo-work, uas_work_list);
+ spin_unlock_irq(uas_lists_lock);
}
}
spin_unlock_irqrestore(devinfo-lock, flags);
@@ -288,10 +288,10 @@ static void uas_xfer_data(struct urb *urb, struct
scsi_cmnd *cmnd,
cmdinfo-state |= direction | SUBMIT_STATUS_URB;
err = uas_submit_urbs(cmnd, cmnd-device-hostdata, GFP_ATOMIC);
if (err) {
- spin_lock(uas_work_lock);
- list_add_tail(cmdinfo-list, uas_work_list);
+ spin_lock(uas_lists_lock);
+ list_add_tail(cmdinfo-work, uas_work_list);
cmdinfo-state |= IS_IN_WORK_LIST;
- spin_unlock(uas_work_lock);
+ spin_unlock(uas_lists_lock);
schedule_work(uas_work);
}
}
@@ -694,10 +694,10 @@ static int uas_queuecommand_lck(struct scsi_cmnd *cmnd,
spin_unlock_irqrestore(devinfo-lock, flags);
return SCSI_MLQUEUE_DEVICE_BUSY;
}
- spin_lock(uas_work_lock);
- list_add_tail(cmdinfo-list, uas_work_list);
+ spin_lock
Re: [RfC PATCH] xhci: fix usb3 streams
On Fr, 2013-08-30 at 09:48 -0700, Sarah Sharp wrote: Hi Gerd, Thanks for catching this! I have a UAS device now, but won't have time to test it for a week or so. Can you please Cc me on patches to the xHCI driver in the future? Otherwise it gets lost in the other linux-usb mailing list traffic. Hooked up scripts/get_maintainer.pl in my git config now, that should take care of this in the future. + ret = xhci_update_stream_ring(cur_ring, 1); Could you use true instead of 1 here? Fixed here and in the other two places, resent with some uas patches on top. cheers, Gerd -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RfC PATCH] xhci: fix usb3 streams
xhci maintains a radix tree for each stream endpoint because it must
be able to map a trb address to the stream ring. Each ring segment
must be added to the ring for this to work. Currently xhci sticks
only the first segment of each stream ring into the radix tree.
Result is that things work initially, but as soon as the first segment
is full xhci can't map the trb address from the completion event to the
stream ring any more - BOOM. You'll find this message in the logs:
ERROR Transfer event for disabled endpoint or incorrect stream ring
This patch adds a helper function to update the radix tree. It can
both insert and remove ring segments. It loops over the segment list
and handles all segments instead of just the first. It is called
whenever an update is needed: When allocating a ring, when expanding
a ring and when releasing a ring.
It's RfC because (a) I wanna stress test this and the uas driver a bit
more before submitting for real and (b) because there is still a FIXME
to fix. Reviews are welcome nevertheless. Also sending to make people
aware I'm looking into this atm.
Latest bits are available from:
git://git.kraxel.org/linux uas
Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
drivers/usb/host/xhci-mem.c | 51 +
drivers/usb/host/xhci.h | 2 ++
2 files changed, 40 insertions(+), 13 deletions(-)
diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index 6f8c2fd..44cdb1d 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -154,8 +154,11 @@ void xhci_ring_free(struct xhci_hcd *xhci, struct
xhci_ring *ring)
if (!ring)
return;
- if (ring-first_seg)
+ if (ring-first_seg) {
+ if (ring-type == TYPE_STREAM)
+ xhci_update_stream_ring(ring, 0);
xhci_free_segments_for_ring(xhci, ring-first_seg);
+ }
kfree(ring);
}
@@ -351,6 +354,11 @@ int xhci_ring_expansion(struct xhci_hcd *xhci, struct
xhci_ring *ring,
xhci_dbg(xhci, ring expansion succeed, now has %d segments\n,
ring-num_segs);
+ if (ring-type == TYPE_STREAM) {
+ ret = xhci_update_stream_ring(ring, 1);
+ WARN_ON(ret); /* FIXME */
+ }
+
return 0;
}
@@ -601,6 +609,33 @@ static int xhci_test_radix_tree(struct xhci_hcd *xhci,
* extended systems (where the DMA address can be bigger than 32-bits),
* if we allow the PCI dma mask to be bigger than 32-bits. So don't do that.
*/
+
+int xhci_update_stream_ring(struct xhci_ring *ring, bool insert)
+{
+ struct xhci_segment *seg;
+ unsigned long key;
+ bool present;
+ int ret;
+
+ BUG_ON(ring-trb_address_map == NULL);
+ seg = ring-first_seg;
+ do {
+ key = (unsigned long)(seg-dma TRB_SEGMENT_SHIFT);
+ present = radix_tree_lookup(ring-trb_address_map, key) != NULL;
+ if (!present insert) {
+ ret = radix_tree_insert(ring-trb_address_map, key,
ring);
+ if (ret)
+ return ret;
+ }
+ if (present !insert) {
+ radix_tree_delete(ring-trb_address_map, key);
+ }
+ seg = seg-next;
+ } while (seg != ring-first_seg);
+
+ return 0;
+}
+
struct xhci_stream_info *xhci_alloc_stream_info(struct xhci_hcd *xhci,
unsigned int num_stream_ctxs,
unsigned int num_streams, gfp_t mem_flags)
@@ -608,7 +643,6 @@ struct xhci_stream_info *xhci_alloc_stream_info(struct
xhci_hcd *xhci,
struct xhci_stream_info *stream_info;
u32 cur_stream;
struct xhci_ring *cur_ring;
- unsigned long key;
u64 addr;
int ret;
@@ -663,6 +697,7 @@ struct xhci_stream_info *xhci_alloc_stream_info(struct
xhci_hcd *xhci,
if (!cur_ring)
goto cleanup_rings;
cur_ring-stream_id = cur_stream;
+ cur_ring-trb_address_map = stream_info-trb_address_map;
/* Set deq ptr, cycle bit, and stream context type */
addr = cur_ring-first_seg-dma |
SCT_FOR_CTX(SCT_PRI_TR) |
@@ -672,10 +707,7 @@ struct xhci_stream_info *xhci_alloc_stream_info(struct
xhci_hcd *xhci,
xhci_dbg(xhci, Setting stream %d ring ptr to 0x%08llx\n,
cur_stream, (unsigned long long) addr);
- key = (unsigned long)
- (cur_ring-first_seg-dma TRB_SEGMENT_SHIFT);
- ret = radix_tree_insert(stream_info-trb_address_map,
- key, cur_ring);
+ ret = xhci_update_stream_ring(cur_ring, 1);
if (ret) {
xhci_ring_free(xhci, cur_ring);
stream_info-stream_rings[cur_stream] = NULL
Re: USB2.0 disk format failure in windows guest
Hi, Fixing this will require qemu to copy the beginning and ending parts of these non-aligned qTDs into separate bounce buffers so that the URB length can be divisible by 512. Worth trying: http://www.kraxel.org/cgit/qemu/log/?h=usb.80 It puts the qemu usb passthrough code upside down. All xfers will go through a bounce buffer, requests are submitted via libusbx. That should fix it. Of course there is the risk of regressions in other areas as it is all new code. Also make sure you have libusbx-devel installed, otherwise qemu will fallback to the old code which uses usbfs ioctls directly. cheers, Gerd -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Fwd: PROBLEM: Permanent kernel panic in USB hub driver - 3.5.0-22
Hi, However, I think the root cause is the same as in previous cores, so it is still would be worth to analyze them. Here is a new picture of the panic: https://dl.dropbox.com/u/8276110/3.7.4%20panic.jpg Do you have the UAS driver compiled in? I see some functions that could only be called after the UAS driver allocates a streams context (i.e. xhci_stream_id_to_ring). It doesn't seem to be related to the Set Address timeout crash that was your previous issue. The crash could be a uas driver bug, please pick up fixes from http://www.kraxel.org/cgit/linux/log/?h=uas (greg's usb-next tree will do too). That will not make the uas driver work due to the radix tree bug, but the uas driver's error handling should be solid enougth that xhci driver bugs don't make uas crash the box. cheers, Gerd -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: xhci streams bug
On 01/31/13 15:34, Gerd Hoffmann wrote: [ all still in qemu, will cross-checking on real hardware ] Done now. Getting the same behavior with the TI demo board on a nec xhci controller (express card). cheers, Gerd -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
xhci streams bug
Hi, Started hacking streams support into qemu, trapped into this one: [ 218.807129] xhci_hcd :00:0f.0: ERROR Transfer event for disabled endpoint or incorrect stream ring [ 218.808087] xhci_hcd :00:0f.0: @3c32d560 38342000 0100 01078001 Triggers after xhci emulation stepping over the first link trb for a stream ring. I think it's because xhci doesn't manage the trb_address_map radix tree correctly. I can only find a single radix_tree_insert() call in the code, and that one is for the initial segment. But nobody seems to update the radix tree when linking the next segment ... cheers, Gerd -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: xhci streams bug
Hi, I think it's because xhci doesn't manage the trb_address_map radix tree correctly. I can only find a single radix_tree_insert() call in the code, and that one is for the initial segment. But nobody seems to update the radix tree when linking the next segment ... There seems to be a bit more fishy, a device reset doesn't bring the device back online. [ 117.169453] scsi3 : uas [ 117.171072] usbcore: registered new interface driver uas [ 117.175060] scsi 3:0:0:0: Direct-Access QEMU QEMU HARDDISK 1.3. PQ: 0 ANSI: 5 [ 117.195589] sd 3:0:0:0: Attached scsi generic sg1 type 0 [ 117.206834] sd 3:0:0:0: [sdb] 2097152 512-byte logical blocks: (1.07 GB/1.00 GiB) [ 117.223331] sd 3:0:0:0: [sdb] Write Protect is off [ 117.236356] sd 3:0:0:0: [sdb] Write cache: enabled, read cache: enabled, doesn't support DPO or FUA [ 117.251144] sdb: sdb1 [ 117.266808] sd 3:0:0:0: [sdb] Attached SCSI disk All fine so far. [ 117.324571] xhci_hcd :00:0f.0: ERROR Transfer event for disabled endpoint or incorrect stream ring [ 117.325543] xhci_hcd :00:0f.0: @3c348550 3c8a8800 0d60 01058000 Hitting stream ring link bug, status pipe stops working. [ 177.760380] sd 3:0:0:0: [sdb] uas_eh_abort_handler 88003c8ef600 tag 0, inflight: CMD [ 180.769264] scsi host3: uas_eh_task_mgmt: ABORT TASK timed out [ 180.778724] sd 3:0:0:0: [sdb] uas_eh_abort_handler 8800350f6d00 tag 1, inflight: CMD [ 183.780182] scsi host3: uas_eh_task_mgmt: ABORT TASK timed out [ 183.790096] sd 3:0:0:0: uas_eh_abort_handler 88002e859100 tag 2, inflight: CMD [ 186.796318] scsi host3: uas_eh_task_mgmt: ABORT TASK timed out [ 186.799973] sd 3:0:0:0: uas_eh_device_reset_handler [ 189.805352] scsi host3: uas_eh_task_mgmt: LOGICAL UNIT RESET timed out scsi / uas tries to recover via task management, which fails. Probably due to the status pipe being hosed. Could also be a bug though in qemu's tmf code though. [ 189.815757] usb 7-3: URB BAD STATUS -2 [ 189.819638] usb 7-3: URB BAD STATUS -2 [ 189.822628] usb 7-3: URB BAD STATUS -2 [ 189.826979] usb 7-3: URB BAD STATUS -2 [ 189.829789] usb 7-3: URB BAD STATUS -2 [ 189.830699] usb 7-3: URB BAD STATUS -2 uas canceled inflight urbs here via usb_kill_anchored_urbs() [ 189.936982] usb 7-3: reset SuperSpeed USB device number 2 using xhci_hcd [ 189.956674] usb 7-3: Parent hub missing LPM exit latency info. Power management will be impacted. [ 189.958721] xhci_hcd :00:0f.0: xHCI xhci_drop_endpoint called with disabled ep 88003d043700 [ 189.964337] xhci_hcd :00:0f.0: xHCI xhci_drop_endpoint called with disabled ep 88003d043740 [ 189.968832] xhci_hcd :00:0f.0: xHCI xhci_drop_endpoint called with disabled ep 88003d043780 [ 189.970601] xhci_hcd :00:0f.0: xHCI xhci_drop_endpoint called with disabled ep 88003d0437c0 uas resets device. [ 189.974161] scsi host3: uas_eh_bus_reset_handler success uas thinks we are fine again ... [ 189.978617] scsi host3: sense urb submission failure ... but we are not, sense pipe still broken. [ 199.979340] sd 3:0:0:0: uas_eh_abort_handler 88002e859100 tag 2, inflight: s-st a-cmd s-cmd [ 199.991331] sd 3:0:0:0: abort completed [ 199.995088] sd 3:0:0:0: Device offlined - not ready after error recovery scsi layer decides to take the device offline as the request (test unit ready probably) didn't work. [ 199.999020] sd 3:0:0:0: Device offlined - not ready after error recovery [ 200.001558] sd 3:0:0:0: Device offlined - not ready after error recovery [ 200.003413] sd 3:0:0:0: [sdb] Unhandled error code [ 200.006862] sd 3:0:0:0: [sdb] [ 200.007532] Result: hostbyte=DID_TIME_OUT driverbyte=DRIVER_OK [ 200.011701] sd 3:0:0:0: [sdb] CDB: [ 200.013864] Read(10): 28 00 00 00 08 00 00 00 08 00 [ 200.015128] end_request: I/O error, dev sdb, sector 2048 [ 200.016244] Buffer I/O error on device sdb1, logical block 0 [ 200.017412] sd 3:0:0:0: [sdb] Unhandled error code [ 200.018384] sd 3:0:0:0: [sdb] [ 200.019023] Result: hostbyte=DID_TIME_OUT driverbyte=DRIVER_OK [ 200.020161] sd 3:0:0:0: [sdb] CDB: [ 200.020846] Read(10): 28 00 00 00 01 68 00 00 08 00 [ 200.023422] end_request: I/O error, dev sdb, sector 360 [ 200.024636] Buffer I/O error on device sdb, logical block 45 scsi layer finally throws an I/O error. But, hey, at least the machine is still fine, uas didn't crash in the process ... [ all still in qemu, will cross-checking on real hardware ] cheers, Gerd -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] usb-uas: update MAINTAINERS entry
Hi, Problem is that uas is pretty much the only device using streams, so uas will be the one who triggers any stream bugs in xhci. I have no idea how solid xhci streams support is at the moment. The xHCI streams support isn't well tested, because the UAS devices I had were so buggy that I couldn't fully test them, and the UAS driver didn't (doesn't still?) properly support cancellation or device reset. There have been a number of improvements fixes there, it should do alot better now. Also, I do know there are a couple of streams work-arounds that need to be created for the Intel Panther Point xHCI host controller, so I would suggest testing with an NEC/Renesas xHCI host instead. I will get around to implementing them, but other bugs have taken a higher priority. Hmm. I have a nec xhci for that, but streams not working on Intel pretty much blocks removing the BROKEN tag from uas :( cheers, Gerd -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: UAS questions
On 01/27/13 13:54, Vlad Silman wrote: Hello, I have a few questions regarding Linux UAS host-side and device-side drivers. I've seen that Linux UAS host driver supports the task management commands as defined by T10 UAS spec, such as ABORT TASK and LOGICAL UNIT RESET. I'm trying to work with Thermaltake BlacX dock that doesn't support these commands and it doesn't work with Linux UAS host driver. uas should work nevertheless. Under normal circumstances the device management functions should not be used. And even if they don't work, at some point in error handling the scsi layer will invoke the bus_reset handler which doesn't try to use task management functions any more and just resets the device to recover ... Be sure to run the latest bits (greg's usb tree or http://www.kraxel.org/cgit/linux/log/?h=uas), there have been uas fixes recently which are not merged upstream yet. Might also be bugs in the xhci stream handling are tripping you up. cheers, Gerd -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] usb-uas: set max_lun and max_channel
256 luns is what the sam-4 address method 0 can handle and what the qemu uas emulation supports. So pick that for now. Signed-off-by: Gerd Hoffmann kra...@redhat.com --- drivers/usb/storage/uas.c |2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index 29c5dab..0b72257 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -991,6 +991,8 @@ static int uas_probe(struct usb_interface *intf, const struct usb_device_id *id) shost-max_cmd_len = 16 + 252; shost-max_id = 1; + shost-max_lun = 256; + shost-max_channel = 1; shost-sg_tablesize = udev-bus-sg_tablesize; devinfo-intf = intf; -- 1.7.9.7 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] usb-uas: update MAINTAINERS entry
Cc: Sarah Sharp sarah.a.sh...@linux.intel.com Signed-off-by: Gerd Hoffmann kra...@redhat.com --- MAINTAINERS |3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/MAINTAINERS b/MAINTAINERS index 8ae709e..c5b37de 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -7911,9 +7911,10 @@ F: drivers/net/wireless/ath/ar5523/ USB ATTACHED SCSI M: Matthew Wilcox wi...@linux.intel.com M: Sarah Sharp sarah.a.sh...@linux.intel.com +M: Gerd Hoffmann kra...@redhat.com L: linux-usb@vger.kernel.org L: linux-s...@vger.kernel.org -S: Supported +S: Maintained F: drivers/usb/storage/uas.c USB CDC ETHERNET DRIVER -- 1.7.9.7 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] usb-uas: set max_lun and max_channel
256 luns is what the sam-4 address method 0 can handle and what the qemu uas emulation supports. So pick that for now. [ v2: unlike the other two max_* fields max_channel isn't max+1 ] Signed-off-by: Gerd Hoffmann kra...@redhat.com --- drivers/usb/storage/uas.c |2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index 29c5dab..2d40a11 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -991,6 +991,8 @@ static int uas_probe(struct usb_interface *intf, const struct usb_device_id *id) shost-max_cmd_len = 16 + 252; shost-max_id = 1; + shost-max_lun = 256; + shost-max_channel = 0; shost-sg_tablesize = udev-bus-sg_tablesize; devinfo-intf = intf; -- 1.7.9.7 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] usb-uas: update MAINTAINERS entry
Hi, diff --git a/MAINTAINERS b/MAINTAINERS index 8ae709e..c5b37de 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -7911,9 +7911,10 @@ F:drivers/net/wireless/ath/ar5523/ USB ATTACHED SCSI M: Matthew Wilcox wi...@linux.intel.com M: Sarah Sharp sarah.a.sh...@linux.intel.com +M: Gerd Hoffmann kra...@redhat.com Should Matthew be removed from this? Dunno, Sarah? Also, any word on when I can remove the CONFIG_BROKEN marking on this driver? With the patches in -next uas itself should be reasonable solid. Problem is that uas is pretty much the only device using streams, so uas will be the one who triggers any stream bugs in xhci. I have no idea how solid xhci streams support is at the moment. Sarah, is there some way to avoid using streams? The UAS specs seems to imply using streams is mandatory when connected to a USB-3 port, is that correct? Is there some way to force usb3 devices into usb2 mode even when plugged into a usb3 port? I'd like to have a no_streams module option if possible ... cheers, Gerd -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] usb-uas: update MAINTAINERS entry
Hi, Sarah, is there some way to avoid using streams? The UAS specs seems to imply using streams is mandatory when connected to a USB-3 port, is that correct? Is there some way to force usb3 devices into usb2 mode even when plugged into a usb3 port? I'd like to have a no_streams module option if possible ... Well, I think we want to use streams, that's the whole advantage of UAS over the old spec. Sure, but being able to turn them off for trouble-shooting purposes would still be useful IMO. I wasn't aware that the bugs were in the xhci driver, I thought they were in the uas driver, but I could be totally wrong. Oh, uas had bugs too, pretty serious ones included, no question. Oh, and any hints on what device on the market today actually follows the UAS spec so I can buy one for testing? /me asked the same a while ago, here is the reply quoting sarah I would suggest getting a TI UAS evaluation board. They seem to be the most stable UAS device out there: http://www.ti.com/tool/tusb9261demo I have one of their boards from a year or so ago, but I suspect there's a new revision by now. I got the sample from Kevin Main km...@ti.com, and I suspect he might give you one for free as well. Another option might be to use the Linux UAS gadget stack with a OMAP5 board with the Synopsis Designware 3 USB 3.0 device controller. You could talk with Sebastian Andrzej Siewior bige...@linutronix.de, since he has been doing a lot of work on the UAS gadget driver lately. /quote cheers, Gerd -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/6] uas: improve abort handler
Two changes. First we check whenever the request is linked in the work
list and if so take it out. Second check whenever the command is
actually in flight before asking the device to cancel it via task
management, and in case it isn't just zap the data urbs and finish it.
Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
drivers/usb/storage/uas.c | 19 +--
1 files changed, 17 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index 05f1f2b..5416f2a 100644
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -715,8 +715,23 @@ static int uas_eh_abort_handler(struct scsi_cmnd *cmnd)
uas_log_cmd_state(cmnd, __func__);
spin_lock_irqsave(devinfo-lock, flags);
cmdinfo-state |= COMMAND_ABORTED;
- spin_unlock_irqrestore(devinfo-lock, flags);
- ret = uas_eh_task_mgmt(cmnd, ABORT TASK, TMF_ABORT_TASK);
+ if (cmdinfo-state IS_IN_WORK_LIST) {
+ spin_lock(uas_work_lock);
+ list_del(cmdinfo-list);
+ cmdinfo-state = ~IS_IN_WORK_LIST;
+ spin_unlock(uas_work_lock);
+ }
+ if (cmdinfo-state COMMAND_INFLIGHT) {
+ spin_unlock_irqrestore(devinfo-lock, flags);
+ ret = uas_eh_task_mgmt(cmnd, ABORT TASK, TMF_ABORT_TASK);
+ } else {
+ spin_unlock_irqrestore(devinfo-lock, flags);
+ uas_unlink_data_urbs(devinfo, cmdinfo);
+ spin_lock_irqsave(devinfo-lock, flags);
+ uas_try_complete(cmnd, __func__);
+ spin_unlock_irqrestore(devinfo-lock, flags);
+ ret = SUCCESS;
+ }
return ret;
}
--
1.7.1
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 5/6] uas: improve device reset
Add new function to unlink and abort requests from the work
list, call it on bus reset and disconnect where we kill all
in-flight urbs. Also reorder calls in disconnect to first
cancel transfers, then remove the scsi hba.
Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
drivers/usb/storage/uas.c | 45 -
1 files changed, 44 insertions(+), 1 deletions(-)
diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index 5416f2a..547f96a 100644
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -84,6 +84,7 @@ struct uas_cmd_info {
static int uas_submit_urbs(struct scsi_cmnd *cmnd,
struct uas_dev_info *devinfo, gfp_t gfp);
static void uas_do_work(struct work_struct *work);
+static int uas_try_complete(struct scsi_cmnd *cmnd, const char *caller);
static DECLARE_WORK(uas_work, uas_do_work);
static DEFINE_SPINLOCK(uas_work_lock);
@@ -145,6 +146,45 @@ static void uas_do_work(struct work_struct *work)
}
}
+static void uas_abort_work(struct uas_dev_info *devinfo)
+{
+ struct uas_cmd_info *cmdinfo;
+ struct uas_cmd_info *temp;
+ struct list_head list;
+ unsigned long flags;
+
+ spin_lock_irq(uas_work_lock);
+ list_replace_init(uas_work_list, list);
+ spin_unlock_irq(uas_work_lock);
+
+ spin_lock_irqsave(devinfo-lock, flags);
+ list_for_each_entry_safe(cmdinfo, temp, list, list) {
+ struct scsi_pointer *scp = (void *)cmdinfo;
+ struct scsi_cmnd *cmnd = container_of(scp,
+ struct scsi_cmnd, SCp);
+ struct uas_dev_info *di = (void *)cmnd-device-hostdata;
+
+ if (di == devinfo) {
+ cmdinfo-state |= COMMAND_ABORTED;
+ cmdinfo-state = ~IS_IN_WORK_LIST;
+ if (devinfo-resetting) {
+ /* uas_stat_cmplt() will not do that
+* when a device reset is in
+* progress */
+ cmdinfo-state = ~COMMAND_INFLIGHT;
+ }
+ uas_try_complete(cmnd, __func__);
+ } else {
+ /* not our uas device, relink into list */
+ list_del(cmdinfo-list);
+ spin_lock_irq(uas_work_lock);
+ list_add_tail(cmdinfo-list, uas_work_list);
+ spin_unlock_irq(uas_work_lock);
+ }
+ }
+ spin_unlock_irqrestore(devinfo-lock, flags);
+}
+
static void uas_sense(struct urb *urb, struct scsi_cmnd *cmnd)
{
struct sense_iu *sense_iu = urb-transfer_buffer;
@@ -750,6 +790,7 @@ static int uas_eh_bus_reset_handler(struct scsi_cmnd *cmnd)
int err;
devinfo-resetting = 1;
+ uas_abort_work(devinfo);
usb_kill_anchored_urbs(devinfo-cmd_urbs);
usb_kill_anchored_urbs(devinfo-sense_urbs);
usb_kill_anchored_urbs(devinfo-data_urbs);
@@ -995,10 +1036,12 @@ static void uas_disconnect(struct usb_interface *intf)
struct Scsi_Host *shost = usb_get_intfdata(intf);
struct uas_dev_info *devinfo = (void *)shost-hostdata[0];
- scsi_remove_host(shost);
+ devinfo-resetting = 1;
+ uas_abort_work(devinfo);
usb_kill_anchored_urbs(devinfo-cmd_urbs);
usb_kill_anchored_urbs(devinfo-sense_urbs);
usb_kill_anchored_urbs(devinfo-data_urbs);
+ scsi_remove_host(shost);
uas_free_streams(devinfo);
kfree(devinfo);
}
--
1.7.1
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/6] uas: add UNLINK_DATA_URBS flag
uas_unlink_data_urbs uses this to make sure the the scsi command is
not released while looking at it. This will be needed when we start
calling uas_unlink_data_urbs in the request cancel code paths.
Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
drivers/usb/storage/uas.c | 24 +---
1 files changed, 21 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index 8b58e5e..a972e53 100644
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -66,6 +66,7 @@ enum {
DATA_OUT_URB_INFLIGHT = (1 10),
COMMAND_COMPLETED = (1 11),
COMMAND_ABORTED = (1 12),
+ UNLINK_DATA_URBS= (1 13),
};
/* Overrides scsi_pointer */
@@ -90,10 +91,25 @@ static LIST_HEAD(uas_work_list);
static void uas_unlink_data_urbs(struct uas_dev_info *devinfo,
struct uas_cmd_info *cmdinfo)
{
+ unsigned long flags;
+
+ /*
+* The UNLINK_DATA_URBS flag makes sure uas_try_complete
+* (called by urb completion) doesn't release cmdinfo
+* underneath us.
+*/
+ spin_lock_irqsave(devinfo-lock, flags);
+ cmdinfo-state |= UNLINK_DATA_URBS;
+ spin_unlock_irqrestore(devinfo-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, flags);
+ cmdinfo-state = ~UNLINK_DATA_URBS;
+ spin_unlock_irqrestore(devinfo-lock, flags);
}
static void uas_do_work(struct work_struct *work)
@@ -177,7 +193,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 %p tag %d, inflight:
- %s%s%s%s%s%s%s%s%s%s%s%s\n,
+ %s%s%s%s%s%s%s%s%s%s%s%s%s\n,
caller, cmnd, cmnd-request-tag,
(ci-state SUBMIT_STATUS_URB) ? s-st : ,
(ci-state ALLOC_DATA_IN_URB) ? a-in : ,
@@ -190,7 +206,8 @@ static void uas_log_cmd_state(struct scsi_cmnd *cmnd, const
char *caller)
(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 COMMAND_ABORTED) ? abort : ,
+ (ci-state UNLINK_DATA_URBS) ? unlink: );
}
static int uas_try_complete(struct scsi_cmnd *cmnd, const char *caller)
@@ -201,7 +218,8 @@ static int uas_try_complete(struct scsi_cmnd *cmnd, const
char *caller)
WARN_ON(!spin_is_locked(devinfo-lock));
if (cmdinfo-state (COMMAND_INFLIGHT |
DATA_IN_URB_INFLIGHT |
- DATA_OUT_URB_INFLIGHT))
+ DATA_OUT_URB_INFLIGHT |
+ UNLINK_DATA_URBS))
return -EBUSY;
BUG_ON(cmdinfo-state COMMAND_COMPLETED);
cmdinfo-state |= COMMAND_COMPLETED;
--
1.7.1
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/6] uas: new function to cancel data urbs
Add uas_unlink_data_urbs function to cancel in-flight data urbs.
Moves existing code into a separate function.
[ v2: also drop the locking, just call usb_unlink_urb no matter what,
which is safe because the usb core guarantees the completion
callback is called only once ]
Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
drivers/usb/storage/uas.c | 22 --
1 files changed, 12 insertions(+), 10 deletions(-)
diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index 98b98ee..8b58e5e 100644
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -87,6 +87,15 @@ static DECLARE_WORK(uas_work, uas_do_work);
static DEFINE_SPINLOCK(uas_work_lock);
static LIST_HEAD(uas_work_list);
+static void uas_unlink_data_urbs(struct uas_dev_info *devinfo,
+struct uas_cmd_info *cmdinfo)
+{
+ if (cmdinfo-data_in_urb)
+ usb_unlink_urb(cmdinfo-data_in_urb);
+ if (cmdinfo-data_out_urb)
+ usb_unlink_urb(cmdinfo-data_out_urb);
+}
+
static void uas_do_work(struct work_struct *work)
{
struct uas_cmd_info *cmdinfo;
@@ -274,16 +283,9 @@ static void uas_stat_cmplt(struct urb *urb)
uas_sense(urb, cmnd);
if (cmnd-result != 0) {
/* cancel data transfers on error */
- if (cmdinfo-state DATA_IN_URB_INFLIGHT) {
- spin_unlock_irqrestore(devinfo-lock, flags);
- usb_unlink_urb(cmdinfo-data_in_urb);
- spin_lock_irqsave(devinfo-lock, flags);
- }
- if (cmdinfo-state DATA_OUT_URB_INFLIGHT) {
- spin_unlock_irqrestore(devinfo-lock, flags);
- usb_unlink_urb(cmdinfo-data_out_urb);
- spin_lock_irqsave(devinfo-lock, flags);
- }
+ spin_unlock_irqrestore(devinfo-lock, flags);
+ uas_unlink_data_urbs(devinfo, cmdinfo);
+ spin_lock_irqsave(devinfo-lock, flags);
}
cmdinfo-state = ~COMMAND_INFLIGHT;
uas_try_complete(cmnd, __func__);
--
1.7.1
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 0/6] uas: error handling fixes
Hi, New version of the uas error handling patch series. See also https://bugzilla.kernel.org/show_bug.cgi?id=51031 Gained two new commits to also handle bus reset and disconnect better. Also available from git://git.kraxel.org/linux uas [ git tree includes an additional debug commit ] cheers, Gerd Gerd Hoffmann (6): uas: new function to cancel data urbs uas: add UNLINK_DATA_URBS flag uas: add IS_IN_WORK_LIST flag uas: improve abort handler uas: improve device reset uas: fail any request submitted while resetting the device. drivers/usb/storage/uas.c | 122 +++-- 1 files changed, 106 insertions(+), 16 deletions(-) -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/6] uas: add IS_IN_WORK_LIST flag
Keep track whenever the request is linked into the work list or not.
Needed for request abort.
Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
drivers/usb/storage/uas.c | 10 --
1 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index a972e53..05f1f2b 100644
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -67,6 +67,7 @@ enum {
COMMAND_COMPLETED = (1 11),
COMMAND_ABORTED = (1 12),
UNLINK_DATA_URBS= (1 13),
+ IS_IN_WORK_LIST = (1 14),
};
/* Overrides scsi_pointer */
@@ -131,6 +132,8 @@ static void uas_do_work(struct work_struct *work)
struct uas_dev_info *devinfo = (void *)cmnd-device-hostdata;
spin_lock_irqsave(devinfo-lock, flags);
err = uas_submit_urbs(cmnd, cmnd-device-hostdata, GFP_ATOMIC);
+ if (!err)
+ cmdinfo-state = ~IS_IN_WORK_LIST;
spin_unlock_irqrestore(devinfo-lock, flags);
if (err) {
list_del(cmdinfo-list);
@@ -193,7 +196,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 %p tag %d, inflight:
- %s%s%s%s%s%s%s%s%s%s%s%s%s\n,
+ %s%s%s%s%s%s%s%s%s%s%s%s%s%s\n,
caller, cmnd, cmnd-request-tag,
(ci-state SUBMIT_STATUS_URB) ? s-st : ,
(ci-state ALLOC_DATA_IN_URB) ? a-in : ,
@@ -207,7 +210,8 @@ 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 UNLINK_DATA_URBS) ? unlink: ,
+ (ci-state IS_IN_WORK_LIST) ? work : );
}
static int uas_try_complete(struct scsi_cmnd *cmnd, const char *caller)
@@ -244,6 +248,7 @@ static void uas_xfer_data(struct urb *urb, struct scsi_cmnd
*cmnd,
if (err) {
spin_lock(uas_work_lock);
list_add_tail(cmdinfo-list, uas_work_list);
+ cmdinfo-state |= IS_IN_WORK_LIST;
spin_unlock(uas_work_lock);
schedule_work(uas_work);
}
@@ -643,6 +648,7 @@ static int uas_queuecommand_lck(struct scsi_cmnd *cmnd,
}
spin_lock(uas_work_lock);
list_add_tail(cmdinfo-list, uas_work_list);
+ cmdinfo-state |= IS_IN_WORK_LIST;
spin_unlock(uas_work_lock);
schedule_work(uas_work);
}
--
1.7.1
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/4] uas: new function to cancel data urbs
Add uas_unlink_data_urbs function to cancel in-flight data urbs.
Moves existing code into a separate function.
Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
drivers/usb/storage/uas.c | 32 ++--
1 files changed, 22 insertions(+), 10 deletions(-)
diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index 98b98ee..c348afa 100644
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -87,6 +87,25 @@ static DECLARE_WORK(uas_work, uas_do_work);
static DEFINE_SPINLOCK(uas_work_lock);
static LIST_HEAD(uas_work_list);
+static void uas_unlink_data_urbs(struct uas_dev_info *devinfo,
+struct uas_cmd_info *cmdinfo)
+{
+ unsigned long flags;
+
+ spin_lock_irqsave(devinfo-lock, flags);
+ if (cmdinfo-state DATA_IN_URB_INFLIGHT) {
+ spin_unlock_irqrestore(devinfo-lock, flags);
+ usb_unlink_urb(cmdinfo-data_in_urb);
+ spin_lock_irqsave(devinfo-lock, flags);
+ }
+ if (cmdinfo-state DATA_OUT_URB_INFLIGHT) {
+ spin_unlock_irqrestore(devinfo-lock, flags);
+ usb_unlink_urb(cmdinfo-data_out_urb);
+ spin_lock_irqsave(devinfo-lock, flags);
+ }
+ spin_unlock_irqrestore(devinfo-lock, flags);
+}
+
static void uas_do_work(struct work_struct *work)
{
struct uas_cmd_info *cmdinfo;
@@ -274,16 +293,9 @@ static void uas_stat_cmplt(struct urb *urb)
uas_sense(urb, cmnd);
if (cmnd-result != 0) {
/* cancel data transfers on error */
- if (cmdinfo-state DATA_IN_URB_INFLIGHT) {
- spin_unlock_irqrestore(devinfo-lock, flags);
- usb_unlink_urb(cmdinfo-data_in_urb);
- spin_lock_irqsave(devinfo-lock, flags);
- }
- if (cmdinfo-state DATA_OUT_URB_INFLIGHT) {
- spin_unlock_irqrestore(devinfo-lock, flags);
- usb_unlink_urb(cmdinfo-data_out_urb);
- spin_lock_irqsave(devinfo-lock, flags);
- }
+ spin_unlock_irqrestore(devinfo-lock, flags);
+ uas_unlink_data_urbs(devinfo, cmdinfo);
+ spin_lock_irqsave(devinfo-lock, flags);
}
cmdinfo-state = ~COMMAND_INFLIGHT;
uas_try_complete(cmnd, __func__);
--
1.7.1
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/4] uas: add UNLINK_DATA_URBS flag
uas_unlink_data_urbs uses this to make sure the the scsi command is
not released while looking at it. This will be needed when we start
calling uas_unlink_data_urbs in the request cancel code paths.
Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
drivers/usb/storage/uas.c | 16 +---
1 files changed, 13 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index c348afa..1ebe974 100644
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -66,6 +66,7 @@ enum {
DATA_OUT_URB_INFLIGHT = (1 10),
COMMAND_COMPLETED = (1 11),
COMMAND_ABORTED = (1 12),
+ UNLINK_DATA_URBS= (1 13),
};
/* Overrides scsi_pointer */
@@ -92,7 +93,13 @@ static void uas_unlink_data_urbs(struct uas_dev_info
*devinfo,
{
unsigned long flags;
+ /*
+* The UNLINK_DATA_URBS flag makes sure uas_try_complete
+* (called by urb completion) doesn't release cmdinfo
+* underneath us.
+*/
spin_lock_irqsave(devinfo-lock, flags);
+ cmdinfo-state |= UNLINK_DATA_URBS;
if (cmdinfo-state DATA_IN_URB_INFLIGHT) {
spin_unlock_irqrestore(devinfo-lock, flags);
usb_unlink_urb(cmdinfo-data_in_urb);
@@ -103,6 +110,7 @@ static void uas_unlink_data_urbs(struct uas_dev_info
*devinfo,
usb_unlink_urb(cmdinfo-data_out_urb);
spin_lock_irqsave(devinfo-lock, flags);
}
+ cmdinfo-state = ~UNLINK_DATA_URBS;
spin_unlock_irqrestore(devinfo-lock, flags);
}
@@ -187,7 +195,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 %p tag %d, inflight:
- %s%s%s%s%s%s%s%s%s%s%s%s\n,
+ %s%s%s%s%s%s%s%s%s%s%s%s%s\n,
caller, cmnd, cmnd-request-tag,
(ci-state SUBMIT_STATUS_URB) ? s-st : ,
(ci-state ALLOC_DATA_IN_URB) ? a-in : ,
@@ -200,7 +208,8 @@ static void uas_log_cmd_state(struct scsi_cmnd *cmnd, const
char *caller)
(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 COMMAND_ABORTED) ? abort : ,
+ (ci-state UNLINK_DATA_URBS) ? unlink: );
}
static int uas_try_complete(struct scsi_cmnd *cmnd, const char *caller)
@@ -211,7 +220,8 @@ static int uas_try_complete(struct scsi_cmnd *cmnd, const
char *caller)
WARN_ON(!spin_is_locked(devinfo-lock));
if (cmdinfo-state (COMMAND_INFLIGHT |
DATA_IN_URB_INFLIGHT |
- DATA_OUT_URB_INFLIGHT))
+ DATA_OUT_URB_INFLIGHT |
+ UNLINK_DATA_URBS))
return -EBUSY;
BUG_ON(cmdinfo-state COMMAND_COMPLETED);
cmdinfo-state |= COMMAND_COMPLETED;
--
1.7.1
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/4] uas: improve abort handler
Two changes. First we check whenever the request is linked in the work
list and if so take it out. Second check whenever the command is
actually in flight before asking the device to cancel it via task
management, and in case it isn't just zap the data urbs and finish it.
Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
drivers/usb/storage/uas.c | 18 --
1 files changed, 16 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index dd23b61..5f498db 100644
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -717,8 +717,22 @@ static int uas_eh_abort_handler(struct scsi_cmnd *cmnd)
uas_log_cmd_state(cmnd, __func__);
spin_lock_irqsave(devinfo-lock, flags);
cmdinfo-state |= COMMAND_ABORTED;
- spin_unlock_irqrestore(devinfo-lock, flags);
- ret = uas_eh_task_mgmt(cmnd, ABORT TASK, TMF_ABORT_TASK);
+ if (cmdinfo-state IS_IN_WORK_LIST) {
+ spin_lock_irq(uas_work_lock);
+ list_del(cmdinfo-list);
+ cmdinfo-state = ~IS_IN_WORK_LIST;
+ spin_unlock_irq(uas_work_lock);
+ }
+ if (cmdinfo-state COMMAND_INFLIGHT) {
+ spin_unlock_irqrestore(devinfo-lock, flags);
+ ret = uas_eh_task_mgmt(cmnd, ABORT TASK, TMF_ABORT_TASK);
+ } else {
+ spin_unlock_irqrestore(devinfo-lock, flags);
+ uas_unlink_data_urbs(devinfo, cmdinfo);
+ spin_lock_irqsave(devinfo-lock, flags);
+ uas_try_complete(cmnd, __func__);
+ spin_unlock_irqrestore(devinfo-lock, flags);
+ }
return ret;
}
--
1.7.1
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/4] uas: error handling fixes
Hi, Trying to address https://bugzilla.kernel.org/show_bug.cgi?id=51031 cheers, Gerd Gerd Hoffmann (4): uas: new function to cancel data urbs uas: add UNLINK_DATA_URBS flag uas: add IS_IN_WORK_LIST flag uas: improve abort handler drivers/usb/storage/uas.c | 72 +++- 1 files changed, 57 insertions(+), 15 deletions(-) -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/4] uas: add IS_IN_WORK_LIST flag
Keep track whenever the request is linked into the work list or not.
Needed for request abort.
Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
drivers/usb/storage/uas.c | 10 --
1 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index 1ebe974..dd23b61 100644
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -67,6 +67,7 @@ enum {
COMMAND_COMPLETED = (1 11),
COMMAND_ABORTED = (1 12),
UNLINK_DATA_URBS= (1 13),
+ IS_IN_WORK_LIST = (1 14),
};
/* Overrides scsi_pointer */
@@ -133,6 +134,8 @@ static void uas_do_work(struct work_struct *work)
struct uas_dev_info *devinfo = (void *)cmnd-device-hostdata;
spin_lock_irqsave(devinfo-lock, flags);
err = uas_submit_urbs(cmnd, cmnd-device-hostdata, GFP_ATOMIC);
+ if (!err)
+ cmdinfo-state = ~IS_IN_WORK_LIST;
spin_unlock_irqrestore(devinfo-lock, flags);
if (err) {
list_del(cmdinfo-list);
@@ -195,7 +198,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 %p tag %d, inflight:
- %s%s%s%s%s%s%s%s%s%s%s%s%s\n,
+ %s%s%s%s%s%s%s%s%s%s%s%s%s%s\n,
caller, cmnd, cmnd-request-tag,
(ci-state SUBMIT_STATUS_URB) ? s-st : ,
(ci-state ALLOC_DATA_IN_URB) ? a-in : ,
@@ -209,7 +212,8 @@ 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 UNLINK_DATA_URBS) ? unlink: ,
+ (ci-state IS_IN_WORK_LIST) ? work : );
}
static int uas_try_complete(struct scsi_cmnd *cmnd, const char *caller)
@@ -246,6 +250,7 @@ static void uas_xfer_data(struct urb *urb, struct scsi_cmnd
*cmnd,
if (err) {
spin_lock(uas_work_lock);
list_add_tail(cmdinfo-list, uas_work_list);
+ cmdinfo-state |= IS_IN_WORK_LIST;
spin_unlock(uas_work_lock);
schedule_work(uas_work);
}
@@ -645,6 +650,7 @@ static int uas_queuecommand_lck(struct scsi_cmnd *cmnd,
}
spin_lock(uas_work_lock);
list_add_tail(cmdinfo-list, uas_work_list);
+ cmdinfo-state |= IS_IN_WORK_LIST;
spin_unlock(uas_work_lock);
schedule_work(uas_work);
}
--
1.7.1
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/4] uas: improve abort handler
On 11/29/12 14:29, Oliver Neukum wrote:
On Thursday 29 November 2012 14:06:15 Gerd Hoffmann wrote:
diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index dd23b61..5f498db 100644
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -717,8 +717,22 @@ static int uas_eh_abort_handler(struct scsi_cmnd *cmnd)
uas_log_cmd_state(cmnd, __func__);
spin_lock_irqsave(devinfo-lock, flags);
cmdinfo-state |= COMMAND_ABORTED;
- spin_unlock_irqrestore(devinfo-lock, flags);
- ret = uas_eh_task_mgmt(cmnd, ABORT TASK, TMF_ABORT_TASK);
+ if (cmdinfo-state IS_IN_WORK_LIST) {
+ spin_lock_irq(uas_work_lock);
a) it makes no sense to take the _irq version while you hold an _irqsave
Will fix.
b) are you sure this sequence of locks is safe deadlockwise?
Yes. No other lock is acquired anywhere while holding uas_work_lock.
cheers,
Gerd
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/4] uas: new function to cancel data urbs
+static void uas_unlink_data_urbs(struct uas_dev_info *devinfo,
+ struct uas_cmd_info *cmdinfo)
+{
+ unsigned long flags;
+
+ spin_lock_irqsave(devinfo-lock, flags);
+ if (cmdinfo-state DATA_IN_URB_INFLIGHT) {
+ spin_unlock_irqrestore(devinfo-lock, flags);
urb_unlink_urb may call the completion callback which in turn grabs the
lock to update cmdinfo-state, so we must drop it to avoid deadlocks.
But what is the point of taking it at all if the result of the check may be
reversed
when you act upon it?
Good question. I'm doing all cmdinfo-state access under lock to avoid
races, but I can see how this is kida pointless here. Guess I can just
call urb_unlink_urb no matter what as the usb core guarantees the
completion handler is called only once.
Is it safe to call urb_unlink_urb twice on the same urb? Or must I take
care to not do that?
cheers,
Gerd
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: xhci Portsc register issue
Hi,
But I'm not sure, since I've never attempted to memory map PCI registers
from userspace. I was under the impression that you just can't do that.
You can mmap /sys/bus/pci/devices(${device}/resource0. Just hacked up a
tool which dumps the capability registers this way:
http://www.kraxel.org/cgit/usb-tools/tree/usb-print-caps.c
cheers,
Gerd
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] uas: fix locking
Forgot to unlock in the uas_eh_task_mgmt error paths.
Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
drivers/usb/storage/uas.c |2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index 1578909..4218701 100644
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -649,12 +649,14 @@ static int uas_eh_task_mgmt(struct scsi_cmnd *cmnd,
shost_printk(KERN_INFO, shost,
%s: %s: submit sense urb failed\n,
__func__, fname);
+ 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);
+ spin_unlock_irqrestore(devinfo-lock, flags);
return FAILED;
}
spin_unlock_irqrestore(devinfo-lock, flags);
--
1.7.1
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] uas: fix gcc warning
Streamline control flow so it is easier for gcc to follow which paths
can be taken and which can't.
Fixes warning: 'cmdinfo' may be used uninitialized in this function
Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
drivers/usb/storage/uas.c | 18 --
1 files changed, 8 insertions(+), 10 deletions(-)
diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index 4218701..98b98ee 100644
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -249,16 +249,18 @@ static void uas_stat_cmplt(struct urb *urb)
cmnd = devinfo-cmnd;
else
cmnd = scsi_host_find_tag(shost, tag - 1);
+
if (!cmnd) {
- if (iu-iu_id != IU_ID_RESPONSE) {
- usb_free_urb(urb);
- spin_unlock_irqrestore(devinfo-lock, flags);
- return;
+ if (iu-iu_id == IU_ID_RESPONSE) {
+ /* store results for uas_eh_task_mgmt() */
+ memcpy(devinfo-response, iu,
sizeof(devinfo-response));
}
- } else {
- cmdinfo = (void *)cmnd-SCp;
+ usb_free_urb(urb);
+ spin_unlock_irqrestore(devinfo-lock, flags);
+ return;
}
+ cmdinfo = (void *)cmnd-SCp;
switch (iu-iu_id) {
case IU_ID_STATUS:
if (devinfo-cmnd == cmnd)
@@ -292,10 +294,6 @@ static void uas_stat_cmplt(struct urb *urb)
case IU_ID_WRITE_READY:
uas_xfer_data(urb, cmnd, SUBMIT_DATA_OUT_URB);
break;
- case IU_ID_RESPONSE:
- /* store results for uas_eh_task_mgmt() */
- memcpy(devinfo-response, iu, sizeof(devinfo-response));
- break;
default:
scmd_printk(KERN_ERR, cmnd,
Bogus IU (%d) received on status pipe\n, iu-iu_id);
--
1.7.1
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 0/5] uas: bug fixes
Hi, Resending whole uas bug fix patch series. Patches #3 + #5 got updated, addressing review comments. The fixed patches have been on the list already, but not yet the whole series as updated v2. cheers, Gerd The following changes since commit 56d27adcb536b7430d5f8a6240df8ad261eb00bd: Merge git://git.kernel.org/pub/scm/linux/kernel/git/cmetcalf/linux-tile (2012-09-24 16:17:17 -0700) are available in the git repository at: git://git.kraxel.org/linux uas Gerd Hoffmann (5): uas: keep track of command urbs uas: fix task management uas: remove aborted field, replace with status bit. uas: fix abort uas: add locking drivers/usb/storage/uas.c | 89 + 1 files changed, 66 insertions(+), 23 deletions(-) -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 2/5] uas: fix task management
Allocate one tag for task management functions and
use it properly.
Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
drivers/usb/storage/uas.c |6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index ab66365..1d326c5 100644
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -611,7 +611,7 @@ static int uas_eh_task_mgmt(struct scsi_cmnd *cmnd,
{
struct Scsi_Host *shost = cmnd-device-host;
struct uas_dev_info *devinfo = (void *)shost-hostdata[0];
- u16 tag = ; /* FIXME */
+ u16 tag = devinfo-qdepth - 1;
memset(devinfo-response, 0, sizeof(devinfo-response));
if (uas_submit_sense_urb(shost, GFP_NOIO, tag)) {
@@ -701,7 +701,7 @@ static int uas_slave_configure(struct scsi_device *sdev)
{
struct uas_dev_info *devinfo = sdev-hostdata;
scsi_set_tag_type(sdev, MSG_ORDERED_TAG);
- scsi_activate_tcq(sdev, devinfo-qdepth - 2);
+ scsi_activate_tcq(sdev, devinfo-qdepth - 3);
return 0;
}
@@ -880,7 +880,7 @@ static int uas_probe(struct usb_interface *intf, const
struct usb_device_id *id)
init_usb_anchor(devinfo-data_urbs);
uas_configure_endpoints(devinfo);
- result = scsi_init_shared_tag_map(shost, devinfo-qdepth - 2);
+ result = scsi_init_shared_tag_map(shost, devinfo-qdepth - 3);
if (result)
goto free;
--
1.7.1
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 3/5] uas: remove aborted field, replace with status bit.
Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
drivers/usb/storage/uas.c | 12 ++--
1 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index 1d326c5..42976ec 100644
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -64,13 +64,13 @@ enum {
DATA_IN_URB_INFLIGHT= (1 9),
DATA_OUT_URB_INFLIGHT = (1 10),
COMMAND_COMPLETED = (1 11),
+ COMMAND_ABORTED = (1 12),
};
/* Overrides scsi_pointer */
struct uas_cmd_info {
unsigned int state;
unsigned int stream;
- unsigned int aborted;
struct urb *cmd_urb;
struct urb *data_in_urb;
struct urb *data_out_urb;
@@ -163,7 +163,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 %p tag %d, inflight:
- %s%s%s%s%s%s%s%s%s%s%s\n,
+ %s%s%s%s%s%s%s%s%s%s%s%s\n,
caller, cmnd, cmnd-request-tag,
(ci-state SUBMIT_STATUS_URB) ? s-st : ,
(ci-state ALLOC_DATA_IN_URB) ? a-in : ,
@@ -175,7 +175,8 @@ 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_COMPLETED) ? done : ,
+ (ci-state COMMAND_ABORTED) ? abort : );
}
static int uas_try_complete(struct scsi_cmnd *cmnd, const char *caller)
@@ -302,7 +303,7 @@ static void uas_data_cmplt(struct urb *urb)
} else {
sdb-resid = sdb-length - urb-actual_length;
}
- if (cmdinfo-aborted) {
+ if (cmdinfo-state COMMAND_ABORTED) {
return;
}
uas_try_complete(cmnd, __func__);
@@ -570,7 +571,6 @@ static int uas_queuecommand_lck(struct scsi_cmnd *cmnd,
cmdinfo-state = SUBMIT_STATUS_URB |
ALLOC_CMD_URB | SUBMIT_CMD_URB;
- cmdinfo-aborted = 0;
switch (cmnd-sc_data_direction) {
case DMA_FROM_DEVICE:
@@ -652,7 +652,7 @@ static int uas_eh_abort_handler(struct scsi_cmnd *cmnd)
int ret;
uas_log_cmd_state(cmnd, __func__);
- cmdinfo-aborted = 1;
+ cmdinfo-state |= COMMAND_ABORTED;
ret = uas_eh_task_mgmt(cmnd, ABORT TASK, TMF_ABORT_TASK);
if (cmdinfo-state DATA_IN_URB_INFLIGHT)
usb_kill_urb(cmdinfo-data_in_urb);
--
1.7.1
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 1/5] uas: keep track of command urbs
Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
drivers/usb/storage/uas.c | 11 ++-
1 files changed, 10 insertions(+), 1 deletions(-)
diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index 638cd64..ab66365 100644
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -41,6 +41,7 @@ struct sense_iu_old {
struct uas_dev_info {
struct usb_interface *intf;
struct usb_device *udev;
+ struct usb_anchor cmd_urbs;
struct usb_anchor sense_urbs;
struct usb_anchor data_urbs;
int qdepth, resetting;
@@ -431,6 +432,7 @@ static int uas_submit_task_urb(struct scsi_cmnd *cmnd,
gfp_t gfp,
err = usb_submit_urb(urb, gfp);
if (err)
goto err;
+ usb_anchor_urb(urb, devinfo-cmd_urbs);
return 0;
@@ -521,18 +523,22 @@ static int uas_submit_urbs(struct scsi_cmnd *cmnd,
if (cmdinfo-state ALLOC_CMD_URB) {
cmdinfo-cmd_urb = uas_alloc_cmd_urb(devinfo, gfp, cmnd,
- cmdinfo-stream);
+cmdinfo-stream);
if (!cmdinfo-cmd_urb)
return SCSI_MLQUEUE_DEVICE_BUSY;
cmdinfo-state = ~ALLOC_CMD_URB;
}
if (cmdinfo-state SUBMIT_CMD_URB) {
+ usb_get_urb(cmdinfo-cmd_urb);
if (usb_submit_urb(cmdinfo-cmd_urb, gfp)) {
scmd_printk(KERN_INFO, cmnd,
cmd urb submission failure\n);
return SCSI_MLQUEUE_DEVICE_BUSY;
}
+ usb_anchor_urb(cmdinfo-cmd_urb, devinfo-cmd_urbs);
+ usb_put_urb(cmdinfo-cmd_urb);
+ cmdinfo-cmd_urb = NULL;
cmdinfo-state = ~SUBMIT_CMD_URB;
cmdinfo-state |= COMMAND_INFLIGHT;
}
@@ -670,6 +676,7 @@ static int uas_eh_bus_reset_handler(struct scsi_cmnd *cmnd)
int err;
devinfo-resetting = 1;
+ usb_kill_anchored_urbs(devinfo-cmd_urbs);
usb_kill_anchored_urbs(devinfo-sense_urbs);
usb_kill_anchored_urbs(devinfo-data_urbs);
err = usb_reset_device(udev);
@@ -868,6 +875,7 @@ static int uas_probe(struct usb_interface *intf, const
struct usb_device_id *id)
devinfo-intf = intf;
devinfo-udev = udev;
devinfo-resetting = 0;
+ init_usb_anchor(devinfo-cmd_urbs);
init_usb_anchor(devinfo-sense_urbs);
init_usb_anchor(devinfo-data_urbs);
uas_configure_endpoints(devinfo);
@@ -913,6 +921,7 @@ static void uas_disconnect(struct usb_interface *intf)
struct uas_dev_info *devinfo = (void *)shost-hostdata[0];
scsi_remove_host(shost);
+ usb_kill_anchored_urbs(devinfo-cmd_urbs);
usb_kill_anchored_urbs(devinfo-sense_urbs);
usb_kill_anchored_urbs(devinfo-data_urbs);
uas_free_streams(devinfo);
--
1.7.1
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 4/5] uas: fix abort
Properly report aborted commands.
Also don't access cmdinfo after kicking task management,
it may not be valid any more once it returns.
Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
drivers/usb/storage/uas.c | 11 ---
1 files changed, 4 insertions(+), 7 deletions(-)
diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index 42976ec..df1d72e 100644
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -191,6 +191,10 @@ static int uas_try_complete(struct scsi_cmnd *cmnd, const
char *caller)
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);
+ cmnd-result = DID_ABORT 16;
+ }
cmnd-scsi_done(cmnd);
return 0;
}
@@ -303,9 +307,6 @@ static void uas_data_cmplt(struct urb *urb)
} else {
sdb-resid = sdb-length - urb-actual_length;
}
- if (cmdinfo-state COMMAND_ABORTED) {
- return;
- }
uas_try_complete(cmnd, __func__);
}
@@ -654,10 +655,6 @@ static int uas_eh_abort_handler(struct scsi_cmnd *cmnd)
uas_log_cmd_state(cmnd, __func__);
cmdinfo-state |= COMMAND_ABORTED;
ret = uas_eh_task_mgmt(cmnd, ABORT TASK, TMF_ABORT_TASK);
- if (cmdinfo-state DATA_IN_URB_INFLIGHT)
- usb_kill_urb(cmdinfo-data_in_urb);
- if (cmdinfo-state DATA_OUT_URB_INFLIGHT)
- usb_kill_urb(cmdinfo-data_out_urb);
return ret;
}
--
1.7.1
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 5/5] uas: add locking
Add spinlock to protect uas data structures.
[ v2: s/GFP_NOIO/GFP_ATOMIC/, better don't sleep when holding a spinlock ]
Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
drivers/usb/storage/uas.c | 51 ++--
1 files changed, 44 insertions(+), 7 deletions(-)
diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index df1d72e..1578909 100644
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -50,6 +50,7 @@ struct uas_dev_info {
unsigned use_streams:1;
unsigned uas_sense_old:1;
struct scsi_cmnd *cmnd;
+ spinlock_t lock;
};
enum {
@@ -91,6 +92,7 @@ static void uas_do_work(struct work_struct *work)
struct uas_cmd_info *cmdinfo;
struct uas_cmd_info *temp;
struct list_head list;
+ unsigned long flags;
int err;
spin_lock_irq(uas_work_lock);
@@ -101,7 +103,10 @@ static void uas_do_work(struct work_struct *work)
struct scsi_pointer *scp = (void *)cmdinfo;
struct scsi_cmnd *cmnd = container_of(scp,
struct scsi_cmnd, SCp);
- err = uas_submit_urbs(cmnd, cmnd-device-hostdata, GFP_NOIO);
+ struct uas_dev_info *devinfo = (void *)cmnd-device-hostdata;
+ spin_lock_irqsave(devinfo-lock, flags);
+ err = uas_submit_urbs(cmnd, cmnd-device-hostdata, GFP_ATOMIC);
+ spin_unlock_irqrestore(devinfo-lock, flags);
if (err) {
list_del(cmdinfo-list);
spin_lock_irq(uas_work_lock);
@@ -182,7 +187,9 @@ static void uas_log_cmd_state(struct scsi_cmnd *cmnd, const
char *caller)
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(!spin_is_locked(devinfo-lock));
if (cmdinfo-state (COMMAND_INFLIGHT |
DATA_IN_URB_INFLIGHT |
DATA_OUT_URB_INFLIGHT))
@@ -222,6 +229,7 @@ static void uas_stat_cmplt(struct urb *urb)
struct uas_dev_info *devinfo = (void *)shost-hostdata[0];
struct scsi_cmnd *cmnd;
struct uas_cmd_info *cmdinfo;
+ unsigned long flags;
u16 tag;
if (urb-status) {
@@ -235,6 +243,7 @@ static void uas_stat_cmplt(struct urb *urb)
return;
}
+ spin_lock_irqsave(devinfo-lock, flags);
tag = be16_to_cpup(iu-tag) - 1;
if (tag == 0)
cmnd = devinfo-cmnd;
@@ -243,6 +252,7 @@ static void uas_stat_cmplt(struct urb *urb)
if (!cmnd) {
if (iu-iu_id != IU_ID_RESPONSE) {
usb_free_urb(urb);
+ spin_unlock_irqrestore(devinfo-lock, flags);
return;
}
} else {
@@ -262,10 +272,16 @@ static void uas_stat_cmplt(struct urb *urb)
uas_sense(urb, cmnd);
if (cmnd-result != 0) {
/* cancel data transfers on error */
- if (cmdinfo-state DATA_IN_URB_INFLIGHT)
+ if (cmdinfo-state DATA_IN_URB_INFLIGHT) {
+ spin_unlock_irqrestore(devinfo-lock, flags);
usb_unlink_urb(cmdinfo-data_in_urb);
- if (cmdinfo-state DATA_OUT_URB_INFLIGHT)
+ spin_lock_irqsave(devinfo-lock, flags);
+ }
+ if (cmdinfo-state DATA_OUT_URB_INFLIGHT) {
+ spin_unlock_irqrestore(devinfo-lock, flags);
usb_unlink_urb(cmdinfo-data_out_urb);
+ spin_lock_irqsave(devinfo-lock, flags);
+ }
}
cmdinfo-state = ~COMMAND_INFLIGHT;
uas_try_complete(cmnd, __func__);
@@ -285,14 +301,18 @@ static void uas_stat_cmplt(struct urb *urb)
Bogus IU (%d) received on status pipe\n, iu-iu_id);
}
usb_free_urb(urb);
+ spin_unlock_irqrestore(devinfo-lock, flags);
}
static void uas_data_cmplt(struct urb *urb)
{
struct scsi_cmnd *cmnd = urb-context;
struct uas_cmd_info *cmdinfo = (void *)cmnd-SCp;
+ struct uas_dev_info *devinfo = (void *)cmnd-device-hostdata;
struct scsi_data_buffer *sdb = NULL;
+ 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;
@@ -308,6 +328,7 @@ static void uas_data_cmplt(struct urb *urb)
sdb-resid = sdb-length - urb-actual_length;
}
uas_try_complete(cmnd, __func__);
+ spin_unlock_irqrestore
Re: USB tree closed for 3.7
On 09/25/12 15:45, Greg KH wrote: Hi all, If you haven't sent me any pending patches for 3.7, it's a bit too late as I've now closed the usb-next tree for any new stuff for 3.7, unless it's bug fixes. If you have sent me stuff and I've missed it, please let me know as I think my queue is now empty. [ sent to the list only, guess I better Cc you in the future ] http://marc.info/?l=linux-usbm=134856283907957w=2 cheers, Gerd -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] uas: remove aborted field, replace with status bit.
Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
drivers/usb/storage/uas.c | 12 ++--
1 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index 1d326c5..42976ec 100644
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -64,13 +64,13 @@ enum {
DATA_IN_URB_INFLIGHT= (1 9),
DATA_OUT_URB_INFLIGHT = (1 10),
COMMAND_COMPLETED = (1 11),
+ COMMAND_ABORTED = (1 12),
};
/* Overrides scsi_pointer */
struct uas_cmd_info {
unsigned int state;
unsigned int stream;
- unsigned int aborted;
struct urb *cmd_urb;
struct urb *data_in_urb;
struct urb *data_out_urb;
@@ -163,7 +163,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 %p tag %d, inflight:
- %s%s%s%s%s%s%s%s%s%s%s\n,
+ %s%s%s%s%s%s%s%s%s%s%s%s\n,
caller, cmnd, cmnd-request-tag,
(ci-state SUBMIT_STATUS_URB) ? s-st : ,
(ci-state ALLOC_DATA_IN_URB) ? a-in : ,
@@ -175,7 +175,8 @@ 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_COMPLETED) ? done : ,
+ (ci-state COMMAND_ABORTED) ? abort : );
}
static int uas_try_complete(struct scsi_cmnd *cmnd, const char *caller)
@@ -302,7 +303,7 @@ static void uas_data_cmplt(struct urb *urb)
} else {
sdb-resid = sdb-length - urb-actual_length;
}
- if (cmdinfo-aborted) {
+ if (cmdinfo-state COMMAND_ABORTED) {
return;
}
uas_try_complete(cmnd, __func__);
@@ -570,7 +571,6 @@ static int uas_queuecommand_lck(struct scsi_cmnd *cmnd,
cmdinfo-state = SUBMIT_STATUS_URB |
ALLOC_CMD_URB | SUBMIT_CMD_URB;
- cmdinfo-aborted = 0;
switch (cmnd-sc_data_direction) {
case DMA_FROM_DEVICE:
@@ -652,7 +652,7 @@ static int uas_eh_abort_handler(struct scsi_cmnd *cmnd)
int ret;
uas_log_cmd_state(cmnd, __func__);
- cmdinfo-aborted = 1;
+ cmdinfo-state |= COMMAND_ABORTED;
ret = uas_eh_task_mgmt(cmnd, ABORT TASK, TMF_ABORT_TASK);
if (cmdinfo-state DATA_IN_URB_INFLIGHT)
usb_kill_urb(cmdinfo-data_in_urb);
--
1.7.1
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] uas: remove aborted field, replace with status bit.
Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
drivers/usb/storage/uas.c | 12 ++--
1 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index 1d326c5..42976ec 100644
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -64,13 +64,13 @@ enum {
DATA_IN_URB_INFLIGHT= (1 9),
DATA_OUT_URB_INFLIGHT = (1 10),
COMMAND_COMPLETED = (1 11),
+ COMMAND_ABORTED = (1 12),
};
/* Overrides scsi_pointer */
struct uas_cmd_info {
unsigned int state;
unsigned int stream;
- unsigned int aborted;
struct urb *cmd_urb;
struct urb *data_in_urb;
struct urb *data_out_urb;
@@ -163,7 +163,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 %p tag %d, inflight:
- %s%s%s%s%s%s%s%s%s%s%s\n,
+ %s%s%s%s%s%s%s%s%s%s%s%s\n,
caller, cmnd, cmnd-request-tag,
(ci-state SUBMIT_STATUS_URB) ? s-st : ,
(ci-state ALLOC_DATA_IN_URB) ? a-in : ,
@@ -175,7 +175,8 @@ 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_COMPLETED) ? done : ,
+ (ci-state COMMAND_ABORTED) ? abort : );
}
static int uas_try_complete(struct scsi_cmnd *cmnd, const char *caller)
@@ -302,7 +303,7 @@ static void uas_data_cmplt(struct urb *urb)
} else {
sdb-resid = sdb-length - urb-actual_length;
}
- if (cmdinfo-aborted) {
+ if (cmdinfo-state COMMAND_ABORTED) {
return;
}
uas_try_complete(cmnd, __func__);
@@ -570,7 +571,6 @@ static int uas_queuecommand_lck(struct scsi_cmnd *cmnd,
cmdinfo-state = SUBMIT_STATUS_URB |
ALLOC_CMD_URB | SUBMIT_CMD_URB;
- cmdinfo-aborted = 0;
switch (cmnd-sc_data_direction) {
case DMA_FROM_DEVICE:
@@ -652,7 +652,7 @@ static int uas_eh_abort_handler(struct scsi_cmnd *cmnd)
int ret;
uas_log_cmd_state(cmnd, __func__);
- cmdinfo-aborted = 1;
+ cmdinfo-state |= COMMAND_ABORTED;
ret = uas_eh_task_mgmt(cmnd, ABORT TASK, TMF_ABORT_TASK);
if (cmdinfo-state DATA_IN_URB_INFLIGHT)
usb_kill_urb(cmdinfo-data_in_urb);
--
1.7.1
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/5] uas: remove aborted field, replace with status bit.
---
drivers/usb/storage/uas.c | 12 ++--
1 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index 1d326c5..42976ec 100644
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -64,13 +64,13 @@ enum {
DATA_IN_URB_INFLIGHT= (1 9),
DATA_OUT_URB_INFLIGHT = (1 10),
COMMAND_COMPLETED = (1 11),
+ COMMAND_ABORTED = (1 12),
};
/* Overrides scsi_pointer */
struct uas_cmd_info {
unsigned int state;
unsigned int stream;
- unsigned int aborted;
struct urb *cmd_urb;
struct urb *data_in_urb;
struct urb *data_out_urb;
@@ -163,7 +163,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 %p tag %d, inflight:
- %s%s%s%s%s%s%s%s%s%s%s\n,
+ %s%s%s%s%s%s%s%s%s%s%s%s\n,
caller, cmnd, cmnd-request-tag,
(ci-state SUBMIT_STATUS_URB) ? s-st : ,
(ci-state ALLOC_DATA_IN_URB) ? a-in : ,
@@ -175,7 +175,8 @@ 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_COMPLETED) ? done : ,
+ (ci-state COMMAND_ABORTED) ? abort : );
}
static int uas_try_complete(struct scsi_cmnd *cmnd, const char *caller)
@@ -302,7 +303,7 @@ static void uas_data_cmplt(struct urb *urb)
} else {
sdb-resid = sdb-length - urb-actual_length;
}
- if (cmdinfo-aborted) {
+ if (cmdinfo-state COMMAND_ABORTED) {
return;
}
uas_try_complete(cmnd, __func__);
@@ -570,7 +571,6 @@ static int uas_queuecommand_lck(struct scsi_cmnd *cmnd,
cmdinfo-state = SUBMIT_STATUS_URB |
ALLOC_CMD_URB | SUBMIT_CMD_URB;
- cmdinfo-aborted = 0;
switch (cmnd-sc_data_direction) {
case DMA_FROM_DEVICE:
@@ -652,7 +652,7 @@ static int uas_eh_abort_handler(struct scsi_cmnd *cmnd)
int ret;
uas_log_cmd_state(cmnd, __func__);
- cmdinfo-aborted = 1;
+ cmdinfo-state |= COMMAND_ABORTED;
ret = uas_eh_task_mgmt(cmnd, ABORT TASK, TMF_ABORT_TASK);
if (cmdinfo-state DATA_IN_URB_INFLIGHT)
usb_kill_urb(cmdinfo-data_in_urb);
--
1.7.1
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/5] uas: fix task management
Allocate one tag for task management functions and
use it properly.
Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
drivers/usb/storage/uas.c |6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index ab66365..1d326c5 100644
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -611,7 +611,7 @@ static int uas_eh_task_mgmt(struct scsi_cmnd *cmnd,
{
struct Scsi_Host *shost = cmnd-device-host;
struct uas_dev_info *devinfo = (void *)shost-hostdata[0];
- u16 tag = ; /* FIXME */
+ u16 tag = devinfo-qdepth - 1;
memset(devinfo-response, 0, sizeof(devinfo-response));
if (uas_submit_sense_urb(shost, GFP_NOIO, tag)) {
@@ -701,7 +701,7 @@ static int uas_slave_configure(struct scsi_device *sdev)
{
struct uas_dev_info *devinfo = sdev-hostdata;
scsi_set_tag_type(sdev, MSG_ORDERED_TAG);
- scsi_activate_tcq(sdev, devinfo-qdepth - 2);
+ scsi_activate_tcq(sdev, devinfo-qdepth - 3);
return 0;
}
@@ -880,7 +880,7 @@ static int uas_probe(struct usb_interface *intf, const
struct usb_device_id *id)
init_usb_anchor(devinfo-data_urbs);
uas_configure_endpoints(devinfo);
- result = scsi_init_shared_tag_map(shost, devinfo-qdepth - 2);
+ result = scsi_init_shared_tag_map(shost, devinfo-qdepth - 3);
if (result)
goto free;
--
1.7.1
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 5/5] uas: add locking
Add spinlock to protect uas data structures.
Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
drivers/usb/storage/uas.c | 45 +
1 files changed, 41 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index df1d72e..cb5c9e3 100644
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -50,6 +50,7 @@ struct uas_dev_info {
unsigned use_streams:1;
unsigned uas_sense_old:1;
struct scsi_cmnd *cmnd;
+ spinlock_t lock;
};
enum {
@@ -91,6 +92,7 @@ static void uas_do_work(struct work_struct *work)
struct uas_cmd_info *cmdinfo;
struct uas_cmd_info *temp;
struct list_head list;
+ unsigned long flags;
int err;
spin_lock_irq(uas_work_lock);
@@ -101,7 +103,10 @@ static void uas_do_work(struct work_struct *work)
struct scsi_pointer *scp = (void *)cmdinfo;
struct scsi_cmnd *cmnd = container_of(scp,
struct scsi_cmnd, SCp);
+ struct uas_dev_info *devinfo = (void *)cmnd-device-hostdata;
+ spin_lock_irqsave(devinfo-lock, flags);
err = uas_submit_urbs(cmnd, cmnd-device-hostdata, GFP_NOIO);
+ spin_unlock_irqrestore(devinfo-lock, flags);
if (err) {
list_del(cmdinfo-list);
spin_lock_irq(uas_work_lock);
@@ -182,7 +187,9 @@ static void uas_log_cmd_state(struct scsi_cmnd *cmnd, const
char *caller)
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(!spin_is_locked(devinfo-lock));
if (cmdinfo-state (COMMAND_INFLIGHT |
DATA_IN_URB_INFLIGHT |
DATA_OUT_URB_INFLIGHT))
@@ -222,6 +229,7 @@ static void uas_stat_cmplt(struct urb *urb)
struct uas_dev_info *devinfo = (void *)shost-hostdata[0];
struct scsi_cmnd *cmnd;
struct uas_cmd_info *cmdinfo;
+ unsigned long flags;
u16 tag;
if (urb-status) {
@@ -235,6 +243,7 @@ static void uas_stat_cmplt(struct urb *urb)
return;
}
+ spin_lock_irqsave(devinfo-lock, flags);
tag = be16_to_cpup(iu-tag) - 1;
if (tag == 0)
cmnd = devinfo-cmnd;
@@ -243,6 +252,7 @@ static void uas_stat_cmplt(struct urb *urb)
if (!cmnd) {
if (iu-iu_id != IU_ID_RESPONSE) {
usb_free_urb(urb);
+ spin_unlock_irqrestore(devinfo-lock, flags);
return;
}
} else {
@@ -262,10 +272,16 @@ static void uas_stat_cmplt(struct urb *urb)
uas_sense(urb, cmnd);
if (cmnd-result != 0) {
/* cancel data transfers on error */
- if (cmdinfo-state DATA_IN_URB_INFLIGHT)
+ if (cmdinfo-state DATA_IN_URB_INFLIGHT) {
+ spin_unlock_irqrestore(devinfo-lock, flags);
usb_unlink_urb(cmdinfo-data_in_urb);
- if (cmdinfo-state DATA_OUT_URB_INFLIGHT)
+ spin_lock_irqsave(devinfo-lock, flags);
+ }
+ if (cmdinfo-state DATA_OUT_URB_INFLIGHT) {
+ spin_unlock_irqrestore(devinfo-lock, flags);
usb_unlink_urb(cmdinfo-data_out_urb);
+ spin_lock_irqsave(devinfo-lock, flags);
+ }
}
cmdinfo-state = ~COMMAND_INFLIGHT;
uas_try_complete(cmnd, __func__);
@@ -285,14 +301,18 @@ static void uas_stat_cmplt(struct urb *urb)
Bogus IU (%d) received on status pipe\n, iu-iu_id);
}
usb_free_urb(urb);
+ spin_unlock_irqrestore(devinfo-lock, flags);
}
static void uas_data_cmplt(struct urb *urb)
{
struct scsi_cmnd *cmnd = urb-context;
struct uas_cmd_info *cmdinfo = (void *)cmnd-SCp;
+ struct uas_dev_info *devinfo = (void *)cmnd-device-hostdata;
struct scsi_data_buffer *sdb = NULL;
+ 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;
@@ -308,6 +328,7 @@ static void uas_data_cmplt(struct urb *urb)
sdb-resid = sdb-length - urb-actual_length;
}
uas_try_complete(cmnd, __func__);
+ spin_unlock_irqrestore(devinfo-lock, flags);
}
static struct urb *uas_alloc_data_urb(struct uas_dev_info *devinfo, gfp_t gfp,
@@ -474,6 +495,7 @@ static int uas_submit_urbs
[PATCH 4/5] uas: fix abort
Properly report aborted commands.
Also don't access cmdinfo after kicking task management,
it may not be valid any more once it returns.
Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
drivers/usb/storage/uas.c | 11 ---
1 files changed, 4 insertions(+), 7 deletions(-)
diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index 42976ec..df1d72e 100644
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -191,6 +191,10 @@ static int uas_try_complete(struct scsi_cmnd *cmnd, const
char *caller)
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);
+ cmnd-result = DID_ABORT 16;
+ }
cmnd-scsi_done(cmnd);
return 0;
}
@@ -303,9 +307,6 @@ static void uas_data_cmplt(struct urb *urb)
} else {
sdb-resid = sdb-length - urb-actual_length;
}
- if (cmdinfo-state COMMAND_ABORTED) {
- return;
- }
uas_try_complete(cmnd, __func__);
}
@@ -654,10 +655,6 @@ static int uas_eh_abort_handler(struct scsi_cmnd *cmnd)
uas_log_cmd_state(cmnd, __func__);
cmdinfo-state |= COMMAND_ABORTED;
ret = uas_eh_task_mgmt(cmnd, ABORT TASK, TMF_ABORT_TASK);
- if (cmdinfo-state DATA_IN_URB_INFLIGHT)
- usb_kill_urb(cmdinfo-data_in_urb);
- if (cmdinfo-state DATA_OUT_URB_INFLIGHT)
- usb_kill_urb(cmdinfo-data_out_urb);
return ret;
}
--
1.7.1
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 5/5] uas: add locking
Add spinlock to protect uas data structures.
[ v2: s/GFP_NOIO/GFP_ATOMIC/, better don't sleep when holding a spinlock ]
Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
drivers/usb/storage/uas.c | 51 ++--
1 files changed, 44 insertions(+), 7 deletions(-)
diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index df1d72e..1578909 100644
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -50,6 +50,7 @@ struct uas_dev_info {
unsigned use_streams:1;
unsigned uas_sense_old:1;
struct scsi_cmnd *cmnd;
+ spinlock_t lock;
};
enum {
@@ -91,6 +92,7 @@ static void uas_do_work(struct work_struct *work)
struct uas_cmd_info *cmdinfo;
struct uas_cmd_info *temp;
struct list_head list;
+ unsigned long flags;
int err;
spin_lock_irq(uas_work_lock);
@@ -101,7 +103,10 @@ static void uas_do_work(struct work_struct *work)
struct scsi_pointer *scp = (void *)cmdinfo;
struct scsi_cmnd *cmnd = container_of(scp,
struct scsi_cmnd, SCp);
- err = uas_submit_urbs(cmnd, cmnd-device-hostdata, GFP_NOIO);
+ struct uas_dev_info *devinfo = (void *)cmnd-device-hostdata;
+ spin_lock_irqsave(devinfo-lock, flags);
+ err = uas_submit_urbs(cmnd, cmnd-device-hostdata, GFP_ATOMIC);
+ spin_unlock_irqrestore(devinfo-lock, flags);
if (err) {
list_del(cmdinfo-list);
spin_lock_irq(uas_work_lock);
@@ -182,7 +187,9 @@ static void uas_log_cmd_state(struct scsi_cmnd *cmnd, const
char *caller)
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(!spin_is_locked(devinfo-lock));
if (cmdinfo-state (COMMAND_INFLIGHT |
DATA_IN_URB_INFLIGHT |
DATA_OUT_URB_INFLIGHT))
@@ -222,6 +229,7 @@ static void uas_stat_cmplt(struct urb *urb)
struct uas_dev_info *devinfo = (void *)shost-hostdata[0];
struct scsi_cmnd *cmnd;
struct uas_cmd_info *cmdinfo;
+ unsigned long flags;
u16 tag;
if (urb-status) {
@@ -235,6 +243,7 @@ static void uas_stat_cmplt(struct urb *urb)
return;
}
+ spin_lock_irqsave(devinfo-lock, flags);
tag = be16_to_cpup(iu-tag) - 1;
if (tag == 0)
cmnd = devinfo-cmnd;
@@ -243,6 +252,7 @@ static void uas_stat_cmplt(struct urb *urb)
if (!cmnd) {
if (iu-iu_id != IU_ID_RESPONSE) {
usb_free_urb(urb);
+ spin_unlock_irqrestore(devinfo-lock, flags);
return;
}
} else {
@@ -262,10 +272,16 @@ static void uas_stat_cmplt(struct urb *urb)
uas_sense(urb, cmnd);
if (cmnd-result != 0) {
/* cancel data transfers on error */
- if (cmdinfo-state DATA_IN_URB_INFLIGHT)
+ if (cmdinfo-state DATA_IN_URB_INFLIGHT) {
+ spin_unlock_irqrestore(devinfo-lock, flags);
usb_unlink_urb(cmdinfo-data_in_urb);
- if (cmdinfo-state DATA_OUT_URB_INFLIGHT)
+ spin_lock_irqsave(devinfo-lock, flags);
+ }
+ if (cmdinfo-state DATA_OUT_URB_INFLIGHT) {
+ spin_unlock_irqrestore(devinfo-lock, flags);
usb_unlink_urb(cmdinfo-data_out_urb);
+ spin_lock_irqsave(devinfo-lock, flags);
+ }
}
cmdinfo-state = ~COMMAND_INFLIGHT;
uas_try_complete(cmnd, __func__);
@@ -285,14 +301,18 @@ static void uas_stat_cmplt(struct urb *urb)
Bogus IU (%d) received on status pipe\n, iu-iu_id);
}
usb_free_urb(urb);
+ spin_unlock_irqrestore(devinfo-lock, flags);
}
static void uas_data_cmplt(struct urb *urb)
{
struct scsi_cmnd *cmnd = urb-context;
struct uas_cmd_info *cmdinfo = (void *)cmnd-SCp;
+ struct uas_dev_info *devinfo = (void *)cmnd-device-hostdata;
struct scsi_data_buffer *sdb = NULL;
+ 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;
@@ -308,6 +328,7 @@ static void uas_data_cmplt(struct urb *urb)
sdb-resid = sdb-length - urb-actual_length;
}
uas_try_complete(cmnd, __func__);
+ spin_unlock_irqrestore
Re: disconnect and uas_work_list
On 08/17/12 15:01, Sebastian Andrzej Siewior wrote: On Fri, Aug 17, 2012 at 02:13:40PM +0200, Oliver Neukum wrote: I just noticed some of my patches here got reverted. Do you have any hardware or did you just stumble over it? I have just my target UAS gadget… No hardware. Do you have suggestions where to get hardware? No. I've been asking around in stores but nothing. I've seen a lot different USB 3.0 sticks or hard disks but according to their descriptors they all been doing BOT. Sarah friends had some early Intel thingy afaik. I have my own that is tcm_usb_gadget with dummy_hcd or any UDC you find. Gerd posted patches as well so maybe he knows where to get real hardware. I don't know, /me has a single 3.0 stick which does BOT too. I'm testing uas in a virtual machine, qemu 1.2 (to be released soon) features uas emulation support. Only usb 2.0 for now, qemu usb emulation can't handle streams and other 3.0 stuff. Yet. cheers, Gerd -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
