Re: Allocating buffers for USB transfers (again)

From: Alan Stern
Date: Wed Aug 10 2011 - 10:32:27 EST

On Wed, 10 Aug 2011, Daniel Mack wrote:

> So it seems we can finally close this issue. I got myself a new laptop
> recently, and was actually happy to see that this machine showed the
> bug as well. So I started investigating on what's going on and after
> hours of debugging in the wrong area, I stumbled over a bug in my own
> code. The patch attached fixes the problem for me, and William (one of
> the very few people who saw it, too) also reported green light.
> However, it remains a puzzle to me where the acutal root cause is for
> this is, and I would much like to know. Let me briefly explain what
> I'm seeing.
> The driver operates in a mode so that it copies the layout of its
> inbound stream to the output stream, thus guaranteeing the data rate
> of outbound data is equal to that of the input stream. For that, I
> walk the isochronous frames of each received urb and prepare an urb
> for sending that has the same iso packet structure in terms of lengths
> and offsets, jumping over and ignoring frames that are actually
> invalid (status != 0). So here was the problem: if there were frames
> with any different status field, the driver would calculate the wrong
> offset and the playback stream will have artefacts. So, a classic bug
> you might think, and the fix is trivial.
> However, what I really don't understand is why this wasn't observed
> any earlier by any of the many users, and why iit makes a difference
> which type of memory I use for this. Recall that my original patch
> that allocates DMA coherent memory for the transfer buffers also fixes
> the problem just fine. Or put it that way: when allocating "normal"
> memory using kmalloc(), the stream appears to have "holes" in the iso
> packet structure, while with DMA coherent memory, the layout is
> different. And all this only happens on fairly new 64bit-enabled
> machines.
> Can anyone explain this?

Looking at the driver's current code, it appears that your patch does
not fix the bug properly. Using discontiguous regions in the transfer
buffer is perfectly okay. The real problem is later on, where you do:

if (send_it) {
out->number_of_packets = FRAMES_PER_URB;

This should be

out->number_of_packets = outframe;

The way it is now, the USB stack will try to use data from all the
frame descriptors, and the last few will be stale because the loop
doesn't set them.

I don't know why this would cause the strange symptoms, though.

Alan Stern

To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at
Please read the FAQ at