| Message ID | 1355324652-20305-1-git-send-email-ming.lei@canonical.com |
|---|---|
| State | New |
| Headers | show |
On 12/12/2012 07:04 AM, Ming Lei wrote: > From: Robert Lukassen <Robert.Lukassen@tomtom.com> > > In the WDM class driver a disconnect event leads to calls to > usb_free_coherent to put back two USB DMA buffers allocated earlier. > The call to usb_free_coherent uses a different size parameter > (desc->wMaxCommand) than the corresponding call to usb_alloc_coherent > (desc->bMaxPacketSize0). > > When a disconnect event occurs, this leads to 'bad dma' complaints > from usb core because the USB DMA buffer is being pushed back to the > 'buffer-2048' pool from which it has not been allocated. > > This patch against the most recent linux-2.6 kernel ensures that the > parameters used by usb_alloc_coherent & usb_free_coherent calls in > cdc-wdm.c match. > > Looks this patch on stable tree is missed in precise release. > > BugLink: https://bugs.launchpad.net/bugs/1081879 > > Signed-off-by: Robert Lukassen <robert.lukassen@tomtom.com> > Cc: stable <stable@kernel.org> > Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de> > Signed-off-by: Ming Lei <ming.lei@canonical.com> > --- > drivers/usb/class/cdc-wdm.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/usb/class/cdc-wdm.c b/drivers/usb/class/cdc-wdm.c > index 47085e5..a97c018 100644 > --- a/drivers/usb/class/cdc-wdm.c > +++ b/drivers/usb/class/cdc-wdm.c > @@ -281,7 +281,7 @@ static void cleanup(struct wdm_device *desc) > desc->sbuf, > desc->validity->transfer_dma); > usb_free_coherent(interface_to_usbdev(desc->intf), > - desc->wMaxCommand, > + desc->bMaxPacketSize0, > desc->inbuf, > desc->response->transfer_dma); > kfree(desc->orq); > Looks reasonable.
On 12/12/12 15:04, Ming Lei wrote: > From: Robert Lukassen <Robert.Lukassen@tomtom.com> > > In the WDM class driver a disconnect event leads to calls to > usb_free_coherent to put back two USB DMA buffers allocated earlier. > The call to usb_free_coherent uses a different size parameter > (desc->wMaxCommand) than the corresponding call to usb_alloc_coherent > (desc->bMaxPacketSize0). > > When a disconnect event occurs, this leads to 'bad dma' complaints > from usb core because the USB DMA buffer is being pushed back to the > 'buffer-2048' pool from which it has not been allocated. > > This patch against the most recent linux-2.6 kernel ensures that the > parameters used by usb_alloc_coherent & usb_free_coherent calls in > cdc-wdm.c match. > > Looks this patch on stable tree is missed in precise release. > > BugLink: https://bugs.launchpad.net/bugs/1081879 > > Signed-off-by: Robert Lukassen <robert.lukassen@tomtom.com> > Cc: stable <stable@kernel.org> > Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de> > Signed-off-by: Ming Lei <ming.lei@canonical.com> > --- > drivers/usb/class/cdc-wdm.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/usb/class/cdc-wdm.c b/drivers/usb/class/cdc-wdm.c > index 47085e5..a97c018 100644 > --- a/drivers/usb/class/cdc-wdm.c > +++ b/drivers/usb/class/cdc-wdm.c > @@ -281,7 +281,7 @@ static void cleanup(struct wdm_device *desc) > desc->sbuf, > desc->validity->transfer_dma); > usb_free_coherent(interface_to_usbdev(desc->intf), > - desc->wMaxCommand, > + desc->bMaxPacketSize0, > desc->inbuf, > desc->response->transfer_dma); > kfree(desc->orq); > I'm happy to take this. Acked-by: Colin Ian King <colin.king@canonical.com>
Hi Ming,
I'm a little confused here. This patch was included upstream as of 2.6.39:
~/linux$ git show 878b753e32ca765cd346a5d3038d630178ec78ff
commit 878b753e32ca765cd346a5d3038d630178ec78ff
Author: Robert Lukassen <Robert.Lukassen@tomtom.com>
Date: Wed Mar 16 12:13:34 2011 +0100
USB: Fix 'bad dma' problem on WDM device disconnect
~/linux$ git describe --contains 878b753e32ca765cd346a5d3038d630178ec78ff
v2.6.39-rc1~85^2~3
Thus, this patch is already available in Precise by default. Am I
missing something here? Have you tested with a stock Precise kernel?
Thanks,
Leann
On 12/12/2012 07:04 AM, Ming Lei wrote:
> From: Robert Lukassen <Robert.Lukassen@tomtom.com>
>
> In the WDM class driver a disconnect event leads to calls to
> usb_free_coherent to put back two USB DMA buffers allocated earlier.
> The call to usb_free_coherent uses a different size parameter
> (desc->wMaxCommand) than the corresponding call to usb_alloc_coherent
> (desc->bMaxPacketSize0).
>
> When a disconnect event occurs, this leads to 'bad dma' complaints
> from usb core because the USB DMA buffer is being pushed back to the
> 'buffer-2048' pool from which it has not been allocated.
>
> This patch against the most recent linux-2.6 kernel ensures that the
> parameters used by usb_alloc_coherent & usb_free_coherent calls in
> cdc-wdm.c match.
>
> Looks this patch on stable tree is missed in precise release.
>
> BugLink: https://bugs.launchpad.net/bugs/1081879
>
> Signed-off-by: Robert Lukassen <robert.lukassen@tomtom.com>
> Cc: stable <stable@kernel.org>
> Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
> Signed-off-by: Ming Lei <ming.lei@canonical.com>
> ---
> drivers/usb/class/cdc-wdm.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/usb/class/cdc-wdm.c b/drivers/usb/class/cdc-wdm.c
> index 47085e5..a97c018 100644
> --- a/drivers/usb/class/cdc-wdm.c
> +++ b/drivers/usb/class/cdc-wdm.c
> @@ -281,7 +281,7 @@ static void cleanup(struct wdm_device *desc)
> desc->sbuf,
> desc->validity->transfer_dma);
> usb_free_coherent(interface_to_usbdev(desc->intf),
> - desc->wMaxCommand,
> + desc->bMaxPacketSize0,
> desc->inbuf,
> desc->response->transfer_dma);
> kfree(desc->orq);
Hi Leann, On Thu, Dec 13, 2012 at 1:29 AM, Leann Ogasawara <leann.ogasawara@canonical.com> wrote: > Hi Ming, > > I'm a little confused here. This patch was included upstream as of 2.6.39: > > ~/linux$ git show 878b753e32ca765cd346a5d3038d630178ec78ff > commit 878b753e32ca765cd346a5d3038d630178ec78ff > Author: Robert Lukassen <Robert.Lukassen@tomtom.com> > Date: Wed Mar 16 12:13:34 2011 +0100 > > USB: Fix 'bad dma' problem on WDM device disconnect > > ~/linux$ git describe --contains 878b753e32ca765cd346a5d3038d630178ec78ff > v2.6.39-rc1~85^2~3 > > Thus, this patch is already available in Precise by default. Am I > missing something here? Have you tested with a stock Precise kernel? You are correct, looks I miss that, and the root cause is the below commit, which is a partial fix, so we still need the patch to avoid the 'bad dma' and coherent buffer leak. The patch isn't needed for upstream kernel, so need to push it to stable tree first or you can apply it directly on precise? 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 Thanks, -- Ming Lei
On 12/12/2012 08:33 PM, Ming Lei wrote: > Hi Leann, > > On Thu, Dec 13, 2012 at 1:29 AM, Leann Ogasawara > <leann.ogasawara@canonical.com> wrote: >> Hi Ming, >> >> I'm a little confused here. This patch was included upstream as of 2.6.39: >> >> ~/linux$ git show 878b753e32ca765cd346a5d3038d630178ec78ff >> commit 878b753e32ca765cd346a5d3038d630178ec78ff >> Author: Robert Lukassen <Robert.Lukassen@tomtom.com> >> Date: Wed Mar 16 12:13:34 2011 +0100 >> >> USB: Fix 'bad dma' problem on WDM device disconnect >> >> ~/linux$ git describe --contains 878b753e32ca765cd346a5d3038d630178ec78ff >> v2.6.39-rc1~85^2~3 >> >> Thus, this patch is already available in Precise by default. Am I >> missing something here? Have you tested with a stock Precise kernel? > You are correct, looks I miss that, and the root cause is the below commit, > which is a partial fix, so we still need the patch to avoid the 'bad dma' and > coherent buffer leak. The patch isn't needed for upstream kernel, so need to > push it to stable tree first or you can apply it directly on precise? > > 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. ~/ubuntu-precise$ git describe --contains 7a55df4694e1758a7a1c3b1432e36fca0c859d6a v3.2.3~23 Thanks, Leann
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. Thanks, -- Ming Lei
diff --git a/drivers/usb/class/cdc-wdm.c b/drivers/usb/class/cdc-wdm.c index 47085e5..a97c018 100644 --- a/drivers/usb/class/cdc-wdm.c +++ b/drivers/usb/class/cdc-wdm.c @@ -281,7 +281,7 @@ static void cleanup(struct wdm_device *desc) desc->sbuf, desc->validity->transfer_dma); usb_free_coherent(interface_to_usbdev(desc->intf), - desc->wMaxCommand, + desc->bMaxPacketSize0, desc->inbuf, desc->response->transfer_dma); kfree(desc->orq);