USB transfer_buffer allocations on 64bit systems
in [Kernel]
|
Prev: mfd: Initialise WM831x IRQ masks on chip even if interrupts not in use
Next: INFO: task apache2:7601 blocked for more than 120 seconds.
From: Daniel Mack on 15 Apr 2010 03:40 On Wed, Apr 14, 2010 at 06:21:05PM +0100, Pedro Ribeiro wrote: > On 14 April 2010 17:36, Daniel Mack <daniel(a)caiaq.de> wrote: > > No worries - I agree. But unfortunately, I'm out of ideas now, and my > > initial thoughts about what might cause the trouble were abviously not > > able to explain the issue. Does anyone see further steps of tracking > > this issue down? > > > > Thanks, > > Daniel > > > > Well if this is a dirty / dangerous hack, what about your first patch? > I've been testing it for days and has given me no problems. [For those who haven't followed all the discussions - this patch used usb_buffer_alloc() instead of kmalloc() in the audio USB driver] No, Alan is right. As long as we don't know what's going on, it shouldn't be fixed that way. There might be an update to all USB drivers to use a special allocation function in order to avoid DMA bounce buffers for non-64-bit aware host controllers, but that's certainly a second step. First, the bug that you see needs attention, and the longer you can reproduce it, the better :) Daniel -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Alan Stern on 15 Apr 2010 11:30 On Thu, 15 Apr 2010, Pedro Ribeiro wrote: > I enabled CONFIG_DMAR_BROKEN_GFX_WA=y and the result is the same. A > delay in the boot process and usb devices don't work properly, > including my USB mouse. > > Strange, since you have the same platform as me. The extra usb devices > you were seeing are because of my docking station - but it makes no > difference whether I'm docked or not for the purposes of the original > bug or this situation right now. The dmesg I'm attaching is without > the computer being docked. It's not possible to determine the reason for the timeout errors between timestamps 16 and 53 from the small amount of debugging information in the log. Clearly something is going wrong with the communication between the computer and the EHCI controller. And clearly the kernel config changes are responsible. But I don't know what to do to track it down any farther. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Pedro Ribeiro on 19 Apr 2010 20:20 On 15 April 2010 16:20, Alan Stern <stern(a)rowland.harvard.edu> wrote: > On Thu, 15 Apr 2010, Pedro Ribeiro wrote: > >> I enabled �CONFIG_DMAR_BROKEN_GFX_WA=y and the result is the same. A >> delay in the boot process and usb devices don't work properly, >> including my USB mouse. >> >> Strange, since you have the same platform as me. The extra usb devices >> you were seeing are because of my docking station - but it makes no >> difference whether I'm docked or not for the purposes of the original >> bug or this situation right now. The dmesg I'm attaching is without >> the computer being docked. > > It's not possible to determine the reason for the timeout errors > between timestamps 16 and 53 from the small amount of debugging > information in the log. �Clearly something is going wrong with the > communication between the computer and the EHCI controller. �And > clearly the kernel config changes are responsible. > > But I don't know what to do to track it down any farther. > > Alan Stern > > I guess this is pretty much a dead end until anyone else can reproduce it! I'll continue to use Daniel's patches privately. Daniel, should I use the big initial patch or the GFP_DMA one? Which one is better for a system which is only rebooted every week or so (I usually hibernate)? Regards, Pedro -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Daniel Mack on 7 May 2010 03:50 On Tue, Apr 20, 2010 at 01:16:58AM +0100, Pedro Ribeiro wrote: > On 15 April 2010 16:20, Alan Stern <stern(a)rowland.harvard.edu> wrote: > > On Thu, 15 Apr 2010, Pedro Ribeiro wrote: > > > >> I enabled �CONFIG_DMAR_BROKEN_GFX_WA=y and the result is the same. A > >> delay in the boot process and usb devices don't work properly, > >> including my USB mouse. > >> > >> Strange, since you have the same platform as me. The extra usb devices > >> you were seeing are because of my docking station - but it makes no > >> difference whether I'm docked or not for the purposes of the original > >> bug or this situation right now. The dmesg I'm attaching is without > >> the computer being docked. > > > > It's not possible to determine the reason for the timeout errors > > between timestamps 16 and 53 from the small amount of debugging > > information in the log. �Clearly something is going wrong with the > > communication between the computer and the EHCI controller. �And > > clearly the kernel config changes are responsible. > > > > But I don't know what to do to track it down any farther. > > > > Alan Stern > > > > > > I guess this is pretty much a dead end until anyone else can reproduce it! Hmm, I think I finally found the reason for all this confusion. No idea why I didn't come up with the following explanation any earlier. The problem is again (summarized): On 64bit machines, with 4GB or more, the allocated buffers for USB transfers might be beyond the 32bit boundary. In this case, the IOMMU should take care and install DMA bounce buffer to copy over the buffer before the transfer actually happens. The problem is, however, that this copy mechanism takes place when the URB with its associated buffer is submitted, not when the EHCI will actually do the transfer. In the particular case of audio drivers, though, the contents of the buffers are likely to change after the submission. What we do here is that we map the audio stream buffers which are used by ALSA to the output URBs, so they're filled asychronously. Once the buffer is actually sent out on the bus, it is believed to contain proper audio date. If it doesn't, that's due to too tight audio timing or other problems. This breaks once buffers are magically bounced in the background. So - long story short: these audio buffers need to be DMA coherent. The patch below does that, and Pedro excessively tested this patch for weeks now :) It was just the final explanation _why_ it does the right thing that was yet missing. If nobody has objections, can we still squeeze it into .34? Thanks, Daniel From 5672ca44b9b4617f6c29c88409da13b1bf475547 Mon Sep 17 00:00:00 2001 From: Daniel Mack <daniel(a)caiaq.de> Date: Wed, 7 Apr 2010 01:03:09 +0200 Subject: [PATCH] ALSA: usb/caiaq: use usb_buffer_alloc() Use usb_buffer_alloc() and usb_buffer_free() for transfer buffers. We need DMA-coherent memory in this case as buffer contents are likely to be modified after the URB was submitted, because the URB buffers are mapped to the audio streams. On x86_64, buffers allocated with kmalloc() may be beyond the boundaries of 32bit accessible memory, and DMA bounce buffers will live at other locations, unaccessible by the driver, and hence outside of the audio buffer mapping. Signed-off-by: Daniel Mack <daniel(a)caiaq.de> Tested-by: Pedro Ribeiro <pedrib(a)gmail.com> Cc: Takashi Iwai <tiwai(a)suse.de> Cc: Alan Stern <stern(a)rowland.harvard.edu> Cc: Chris Wright <chrisw(a)sous-sol.org> Cc: David Woodhouse <dwmw2(a)infradead.org> Cc: Andi Kleen <andi(a)firstfloor.org> Cc: Andrew Morton <akpm(a)linux-foundation.org> Cc: Greg KH <gregkh(a)suse.de> Cc: iommu(a)lists.linux-foundation.org Cc: Kernel development list <linux-kernel(a)vger.kernel.org> Cc: USB list <linux-usb(a)vger.kernel.org> Cc: stable(a)kernel.org --- sound/usb/caiaq/audio.c | 57 ++++++++++++++++++++++++++-------------------- 1 files changed, 32 insertions(+), 25 deletions(-) diff --git a/sound/usb/caiaq/audio.c b/sound/usb/caiaq/audio.c index 4328cad..adbeefd 100644 --- a/sound/usb/caiaq/audio.c +++ b/sound/usb/caiaq/audio.c @@ -552,46 +552,47 @@ static struct urb **alloc_urbs(struct snd_usb_caiaqdev *dev, int dir, int *ret) } for (i = 0; i < N_URBS; i++) { - urbs[i] = usb_alloc_urb(FRAMES_PER_URB, GFP_KERNEL); - if (!urbs[i]) { + struct urb *u = usb_alloc_urb(FRAMES_PER_URB, GFP_KERNEL); + if (!u) { log("unable to usb_alloc_urb(), OOM!?\n"); *ret = -ENOMEM; return urbs; } - urbs[i]->transfer_buffer = - kmalloc(FRAMES_PER_URB * BYTES_PER_FRAME, GFP_KERNEL); - if (!urbs[i]->transfer_buffer) { - log("unable to kmalloc() transfer buffer, OOM!?\n"); + urbs[i] = u; + u->dev = usb_dev; + u->pipe = pipe; + u->transfer_buffer_length = + FRAMES_PER_URB * BYTES_PER_FRAME; + u->context = &dev->data_cb_info[i]; + u->interval = 1; + u->transfer_flags = URB_ISO_ASAP; + u->number_of_packets = FRAMES_PER_URB; + u->complete = (dir == SNDRV_PCM_STREAM_CAPTURE) ? + read_completed : write_completed; + u->transfer_buffer = usb_alloc_coherent(usb_dev, + u->transfer_buffer_length, + GFP_KERNEL, &u->transfer_dma); + if (!u->transfer_buffer) { + log("usb_alloc_coherent() failed, OOM!?\n"); *ret = -ENOMEM; return urbs; } for (frame = 0; frame < FRAMES_PER_URB; frame++) { struct usb_iso_packet_descriptor *iso = - &urbs[i]->iso_frame_desc[frame]; + &u->iso_frame_desc[frame]; iso->offset = BYTES_PER_FRAME * frame; iso->length = BYTES_PER_FRAME; } - - urbs[i]->dev = usb_dev; - urbs[i]->pipe = pipe; - urbs[i]->transfer_buffer_length = FRAMES_PER_URB - * BYTES_PER_FRAME; - urbs[i]->context = &dev->data_cb_info[i]; - urbs[i]->interval = 1; - urbs[i]->transfer_flags = URB_ISO_ASAP; - urbs[i]->number_of_packets = FRAMES_PER_URB; - urbs[i]->complete = (dir == SNDRV_PCM_STREAM_CAPTURE) ? - read_completed : write_completed; } *ret = 0; return urbs; } -static void free_urbs(struct urb **urbs) +static void free_urbs(struct usb_device *usb_dev, struct urb **urbs) { int i; @@ -603,7 +604,10 @@ static void free_urbs(struct urb **urbs) continue; usb_kill_urb(urbs[i]); - kfree(urbs[i]->transfer_buffer); + usb_free_coherent(usb_dev, + urbs[i]->transfer_buffer_length, + urbs[i]->transfer_buffer, + urbs[i]->transfer_dma); usb_free_urb(urbs[i]); } @@ -613,6 +617,7 @@ static void free_urbs(struct urb **urbs) int snd_usb_caiaq_audio_init(struct snd_usb_caiaqdev *dev) { int i, ret; + struct usb_device *usb_dev = dev->chip.dev; dev->n_audio_in = max(dev->spec.num_analog_audio_in, dev->spec.num_digital_audio_in) / @@ -689,15 +694,15 @@ int snd_usb_caiaq_audio_init(struct snd_usb_caiaqdev *dev) dev->data_urbs_in = alloc_urbs(dev, SNDRV_PCM_STREAM_CAPTURE, &ret); if (ret < 0) { kfree(dev->data_cb_info); - free_urbs(dev->data_urbs_in); + free_urbs(usb_dev, dev->data_urbs_in); return ret; } dev->data_urbs_out = alloc_urbs(dev, SNDRV_PCM_STREAM_PLAYBACK, &ret); if (ret < 0) { kfree(dev->data_cb_info); - free_urbs(dev->data_urbs_in); - free_urbs(dev->data_urbs_out); + free_urbs(usb_dev, dev->data_urbs_in); + free_urbs(usb_dev, dev->data_urbs_out); return ret; } @@ -706,10 +711,12 @@ int snd_usb_caiaq_audio_init(struct snd_usb_caiaqdev *dev) void snd_usb_caiaq_audio_free(struct snd_usb_caiaqdev *dev) { + struct usb_device *usb_dev = dev->chip.dev; + debug("%s(%p)\n", __func__, dev); stream_stop(dev); - free_urbs(dev->data_urbs_in); - free_urbs(dev->data_urbs_out); + free_urbs(usb_dev, dev->data_urbs_in); + free_urbs(usb_dev, dev->data_urbs_out); kfree(dev->data_cb_info); } -- 1.7.1 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Clemens Ladisch on 7 May 2010 05:50
Daniel Mack wrote: > The problem is again (summarized): > > On 64bit machines, with 4GB or more, the allocated buffers for USB > transfers might be beyond the 32bit boundary. In this case, the IOMMU > should take care and install DMA bounce buffer to copy over the buffer > before the transfer actually happens. The problem is, however, that this > copy mechanism takes place when the URB with its associated buffer is > submitted, not when the EHCI will actually do the transfer. > > In the particular case of audio drivers, though, the contents of the > buffers are likely to change after the submission. What we do here > is that we map the audio stream buffers which are used by ALSA to > the output URBs, so they're filled asychronously. Once the buffer is > actually sent out on the bus, it is believed to contain proper audio > date. If it doesn't, that's due to too tight audio timing or other > problems. This breaks once buffers are magically bounced in the > background. At least the audio class and ua101 drivers don't do this and fill the buffers before they are submitted. > So - long story short: these audio buffers need to be DMA coherent. Does the USB API actually guarantee that all controllers use DMA, i.e., that the buffers can be filled after submission? Regards, Clemens -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ |