lkml.org 
[lkml]   [2011]   [Aug]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    SubjectRe: Allocating buffers for USB transfers (again)
    From
    On Thu, Jul 7, 2011 at 5:16 PM, Florian Mickler <florian@mickler.org> wrote:
    > [adding Robert Hancock]
    > see here for the whole thread: https://lkml.org/lkml/2011/7/7/150
    >
    > 2011/7/7 Daniel Mack <zonque@gmail.com>:
    >> On Thu, Jul 7, 2011 at 2:33 PM, Oliver Neukum <oliver@neukum.org> wrote:
    >>
    >>> PS:  Do you still see this if you enable 64bit DMA for EHCI?
    >>
    >> The problem is that I personally don't see that issue at all. I even
    >> installed 4GB of RAM to my development machine last year to be able to
    >> reproduce this, but I can't, even when the memory allocator is under
    >> heavy load. The only people who see this effect are Pedro Ribeiro and
    >> William Light (both in Cc:), and both have been very helpful in trying
    >> patches and reporting back in detail. Which instructions could we
    >> probably give to these people to finally hunt this issue down?
    >>
    >> Daniel
    >>
    >>
    >
    > Did someone affected already try the suggestions from Robert :
    > http://lkml.org/lkml/2010/4/10/66
    >
    > That is checking if it really is a 64bit vs 32bit issue by booting
    > with mem=4096M ?

    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?


    Many thanks to everyone involved in this - and especially to William
    and Pedro who spent a lot of time in trying my patches and sending me
    debug information.



    Daniel
    From 0b95c8b9402aaa39c0096df00209eb73450d08a8 Mon Sep 17 00:00:00 2001
    From: Daniel Mack <daniel@tamtam.(none)>
    Date: Fri, 5 Aug 2011 13:49:52 +0200
    Subject: [PATCH] ALSA: snd-usb-caiaq: Correct offset fields of outbound
    iso_frame_desc

    This fixes faulty outbount packets in case the inbound packets
    received from the hardware are fragmented and contain bogus input
    iso frames. The bug has been there for ages, but for some strange
    reasons, it was only triggered by newer machines in 64bit mode.

    Signed-off-by: Daniel Mack <zonque@gmail.com>
    Reported-and-tested-by: William Light <wrl@illest.net>
    Reported-by: Pedro Ribeiro <pedrib@gmail.com>
    Cc: stable@kernel.org
    ---
    sound/usb/caiaq/audio.c | 4 +++-
    1 files changed, 3 insertions(+), 1 deletions(-)

    diff --git a/sound/usb/caiaq/audio.c b/sound/usb/caiaq/audio.c
    index d0d493c..4c8249e 100644
    --- a/sound/usb/caiaq/audio.c
    +++ b/sound/usb/caiaq/audio.c
    @@ -614,6 +614,7 @@ static void read_completed(struct urb *urb)
    struct snd_usb_caiaqdev *dev;
    struct urb *out;
    int frame, len, send_it = 0, outframe = 0;
    + size_t offset = 0;

    if (urb->status || !info)
    return;
    @@ -634,7 +635,8 @@ static void read_completed(struct urb *urb)
    len = urb->iso_frame_desc[outframe].actual_length;
    out->iso_frame_desc[outframe].length = len;
    out->iso_frame_desc[outframe].actual_length = 0;
    - out->iso_frame_desc[outframe].offset = BYTES_PER_FRAME * frame;
    + out->iso_frame_desc[outframe].offset = offset;
    + offset += len;

    if (len > 0) {
    spin_lock(&dev->spinlock);
    --
    1.7.6
    \
     
     \ /
      Last update: 2011-08-10 09:53    [W:0.030 / U:0.784 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site