Patchwork [Precise] USB: Fix 'bad dma' problem on WDM device disconnect

login
register
mail settings
Submitter Herton Ronaldo Krzesinski
Date Dec. 13, 2012, 4:23 p.m.
Message ID <20121213162327.GE3021@herton-Z68MA-D2H-B3>
Download mbox | patch
Permalink /patch/206152/
State New
Headers show

Comments

Herton Ronaldo Krzesinski - Dec. 13, 2012, 4:23 p.m.
On Thu, Dec 13, 2012 at 11:10:43PM +0800, Ming Lei wrote:
> On Thu, Dec 13, 2012 at 10:34 PM, Leann Ogasawara
> <leann.ogasawara@canonical.com> wrote:
> >> commit 7a55df4694e1758a7a1c3b1432e36fca0c859d6a
> >> Author: Bjørn Mork <bjorn@mork.no>
> >> Date:   Mon Jan 16 15:11:59 2012 +0100
> >>
> >>     USB: cdc-wdm: better allocate a buffer that is at least as big as
> >> we tell the USB core
> >
> > I'm still a little confused... The above patch is also already in
> > Precise.  We pulled it in via upstream stable.
> 
> Yes, that is just the problem because it is the patch which causes
> the 'bad dma' error in LP1081879 and coherent memory leak.
> 
> The patch should have done the change in both allocation and free,
> but it only did it in allocation.

Hi Ming, I remember sawing a bug in one of my 'patch pilot' duties which
seems related to your problem. It's this one:
https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1074157

The problem is that commit cafbe85fb0d00d32988905c4978df433ca9b6512
changed the usb_alloc_coherent call but not the usb_free_coherent calls
as you also noted. The problem is that upstream changed the allocation
on the driver right after commit cafbe85fb0d, not using anymore the
usb_*_coherent functions, and there is no upstream change to
cherry-pick.

At least precise (3.2 based kernels), need this change because of having commit
cafbe85fb0d00d32988905c4978df433ca9b6512:


I think this will be worth may be a special exception for stable, but
meanwhile if you could test the diff and confirm I could send it as a
SAUCE patch for Precise (and Oneiric if it's affected too).

> 
> Thanks,
> --
> Ming Lei
> 
> -- 
> kernel-team mailing list
> kernel-team@lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team
>
Ming Lei - Dec. 14, 2012, 9:25 a.m.
On Fri, Dec 14, 2012 at 12:23 AM, Herton Ronaldo Krzesinski
<herton.krzesinski@canonical.com> wrote:
> On Thu, Dec 13, 2012 at 11:10:43PM +0800, Ming Lei wrote:
>> On Thu, Dec 13, 2012 at 10:34 PM, Leann Ogasawara
>> <leann.ogasawara@canonical.com> wrote:
>> >> commit 7a55df4694e1758a7a1c3b1432e36fca0c859d6a
>> >> Author: Bjørn Mork <bjorn@mork.no>
>> >> Date:   Mon Jan 16 15:11:59 2012 +0100
>> >>
>> >>     USB: cdc-wdm: better allocate a buffer that is at least as big as
>> >> we tell the USB core
>> >
>> > I'm still a little confused... The above patch is also already in
>> > Precise.  We pulled it in via upstream stable.
>>
>> Yes, that is just the problem because it is the patch which causes
>> the 'bad dma' error in LP1081879 and coherent memory leak.
>>
>> The patch should have done the change in both allocation and free,
>> but it only did it in allocation.
>
> Hi Ming, I remember sawing a bug in one of my 'patch pilot' duties which
> seems related to your problem. It's this one:
> https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1074157
>
> The problem is that commit cafbe85fb0d00d32988905c4978df433ca9b6512
> changed the usb_alloc_coherent call but not the usb_free_coherent calls
> as you also noted. The problem is that upstream changed the allocation
> on the driver right after commit cafbe85fb0d, not using anymore the
> usb_*_coherent functions, and there is no upstream change to
> cherry-pick.
>
> At least precise (3.2 based kernels), need this change because of having commit
> cafbe85fb0d00d32988905c4978df433ca9b6512:
>
> diff --git a/drivers/usb/class/cdc-wdm.c b/drivers/usb/class/cdc-wdm.c
> index 9aaed0d..97b2c55 100644
> --- a/drivers/usb/class/cdc-wdm.c
> +++ b/drivers/usb/class/cdc-wdm.c
> @@ -301,7 +301,7 @@ static void cleanup(struct wdm_device *desc)
>                           desc->sbuf,
>                           desc->validity->transfer_dma);
>         usb_free_coherent(interface_to_usbdev(desc->intf),
> -                         desc->bMaxPacketSize0,
> +                         desc->wMaxCommand,
>                           desc->inbuf,
>                           desc->response->transfer_dma);
>         kfree(desc->orq);
> @@ -788,7 +788,7 @@ out:
>  err3:
>         usb_set_intfdata(intf, NULL);
>         usb_free_coherent(interface_to_usbdev(desc->intf),
> -                         desc->bMaxPacketSize0,
> +                         desc->wMaxCommand,
>                         desc->inbuf,
>                         desc->response->transfer_dma);
>  err2:
>
> I think this will be worth may be a special exception for stable, but
> meanwhile if you could test the diff and confirm I could send it as a
> SAUCE patch for Precise (and Oneiric if it's affected too).

This one has been tested OK in LP1081879.

Thanks
--
Ming Lei

Patch

diff --git a/drivers/usb/class/cdc-wdm.c b/drivers/usb/class/cdc-wdm.c
index 9aaed0d..97b2c55 100644
--- a/drivers/usb/class/cdc-wdm.c
+++ b/drivers/usb/class/cdc-wdm.c
@@ -301,7 +301,7 @@  static void cleanup(struct wdm_device *desc)
 			  desc->sbuf,
 			  desc->validity->transfer_dma);
 	usb_free_coherent(interface_to_usbdev(desc->intf),
-			  desc->bMaxPacketSize0,
+			  desc->wMaxCommand,
 			  desc->inbuf,
 			  desc->response->transfer_dma);
 	kfree(desc->orq);
@@ -788,7 +788,7 @@  out:
 err3:
 	usb_set_intfdata(intf, NULL);
 	usb_free_coherent(interface_to_usbdev(desc->intf),
-			  desc->bMaxPacketSize0,
+			  desc->wMaxCommand,
 			desc->inbuf,
 			desc->response->transfer_dma);
 err2: