diff mbox

[2/2] USB: ehci-tegra: fix inefficient copy of unaligned buffers

Message ID 1428400012-7563-3-git-send-email-johan@kernel.org
State Deferred
Headers show

Commit Message

Johan Hovold April 7, 2015, 9:46 a.m. UTC
Make sure only to copy any actual data rather than the whole buffer,
when releasing the temporary buffer used for unaligned transfer buffers.

Note that the corresponding fix of musb also fixes a lockup on
disconnect, where repeated failed transfers would starve the hub
workqueue from processing the disconnect, which would have prevented the
urbs from being resubmitted. In this case there is no data to forward,
but the full buffer length was being copied nonetheless.

Compile-tested only.

Fixes: fbf9865c6d96 ("USB: ehci: tegra: Align DMA transfers to 32 bytes")
Cc: stable <stable@vger.kernel.org>	# v2.6.39
Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/usb/host/ehci-tegra.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Alan Stern April 7, 2015, 3:30 p.m. UTC | #1
On Tue, 7 Apr 2015, Johan Hovold wrote:

> Make sure only to copy any actual data rather than the whole buffer,
> when releasing the temporary buffer used for unaligned transfer buffers.
> 
> Note that the corresponding fix of musb also fixes a lockup on
> disconnect, where repeated failed transfers would starve the hub
> workqueue from processing the disconnect, which would have prevented the
> urbs from being resubmitted. In this case there is no data to forward,
> but the full buffer length was being copied nonetheless.

This is wrong for isochronous transfers, because the transfer data
generally isn't contiguous in memory.

It would be okay to do this for other transfer types, though.

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Johan Hovold April 7, 2015, 7:59 p.m. UTC | #2
On Tue, Apr 07, 2015 at 11:30:15AM -0400, Alan Stern wrote:
> On Tue, 7 Apr 2015, Johan Hovold wrote:
> 
> > Make sure only to copy any actual data rather than the whole buffer,
> > when releasing the temporary buffer used for unaligned transfer buffers.
> > 
> > Note that the corresponding fix of musb also fixes a lockup on
> > disconnect, where repeated failed transfers would starve the hub
> > workqueue from processing the disconnect, which would have prevented the
> > urbs from being resubmitted. In this case there is no data to forward,
> > but the full buffer length was being copied nonetheless.
> 
> This is wrong for isochronous transfers, because the transfer data
> generally isn't contiguous in memory.
> 
> It would be okay to do this for other transfer types, though.

Thanks for pointing that out. I'll re-spin the series tomorrow.

Johan
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/usb/host/ehci-tegra.c b/drivers/usb/host/ehci-tegra.c
index ff9af29b4e9f..dfe2a7abc36d 100644
--- a/drivers/usb/host/ehci-tegra.c
+++ b/drivers/usb/host/ehci-tegra.c
@@ -313,7 +313,7 @@  static void free_dma_aligned_buffer(struct urb *urb)
 
 	if (usb_urb_dir_in(urb))
 		memcpy(temp->old_xfer_buffer, temp->data,
-		       urb->transfer_buffer_length);
+		       urb->actual_length);
 	urb->transfer_buffer = temp->old_xfer_buffer;
 	kfree(temp->kmalloc_ptr);