Re: [PATCH 4/4] usb/hcd: Ensure scatter-gather is not used for isoc transfers

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On 07/06/2012 07:52 PM, Greg Kroah-Hartman wrote:
On Wed, Jul 04, 2012 at 09:18:04AM +0200, Hans de Goede wrote:
We don't support sg for isoc transfers, enforce this.

Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx>
Acked-by: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>
---
  drivers/usb/core/hcd.c |    7 ++++++-
  1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index 140d3e1..3d2f48f 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -1389,7 +1389,12 @@ int usb_hcd_map_urb_for_dma(struct usb_hcd *hcd, struct urb *urb,
  	    && !(urb->transfer_flags & URB_NO_TRANSFER_DMA_MAP)) {
  		if (hcd->self.uses_dma) {
  			if (urb->num_sgs) {
-				int n = dma_map_sg(
+				int n;
+
+				/* We don't support sg for isoc transfers ! */
+				BUG_ON(usb_endpoint_xfer_isoc(&urb->ep->desc));

No, sorry, I will not take new BUG_ON() calls in any drivers.  You just
crashed a machine, and there is no way that a user can recover.  What
are they supposed to do here?  Who are they going to tell to fix the
issue?

I can see a WARN_ON() and then handle the error properly, but don't
crash a box, that's just rude.

Well this bug_on is intended for driver authors, so that users never get
hit by this because it is catched during development. The problem is that
currently trying to sg with isoc transfers fails silently (all the
data gets dma-ed to / from physical address 0). So the idea is that if
a driver author makes the same mistake I did while working on the
scatter-gather support for usbfs (where I tried to use it for isoc
initially too), they don't end up scratching their head as long as
I did :)

With that said / explained I completely agree. I guess a WARN_ON + error
return is also better for developers, as with a panic the actual reason may
well have scrolled off screen / data may be lost etc.

So I'll respin this one.

Regards,

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


[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux