Bug 12768

Summary: usb_alloc_urb() leaks memory together with uvcvideo driver
Product: Drivers Reporter: Márton Németh (nm127)
Component: Video(Other)Assignee: drivers_video-other
Status: CLOSED CODE_FIX    
Severity: normal CC: alan, laurent.pinchart+bugzilla-kernel
Priority: P1    
Hardware: All   
OS: Linux   
Kernel Version: 2.6.28 Subsystem:
Regression: No Bisected commit-id:
Attachments: 2.6.28 kernel config
output of v4l-info for uvcvideo driver with CNF7129 webcam
lsusb and lsusb -v
debug patch for memory leak

Description Márton Németh 2009-02-23 22:08:36 UTC
Latest working kernel version:
Earliest failing kernel version:
Distribution:
Hardware Environment: EeePC 901
Software Environment: Debian 5.0
Problem Description:

Steps to reproduce:
1. Boot the system
2. start an xterm window and execute the following command:

$ while true; do clear; cat /proc/slab_allocators |grep usb_alloc; sleep 1; done

This will print out similar lines each second:

size-2048: 18 usb_alloc_dev+0x1d/0x212 [usbcore]
size-2048: 2280 usb_alloc_urb+0xc/0x2b [usbcore]
size-1024: 85 usb_alloc_urb+0xc/0x2b [usbcore]
size-128: 10 usb_alloc_urb+0xc/0x2b [usbcore]

3. Start xawtv, this will show the picture of the webcam
4. Exit xawtv

Current result:
In the output of /proc/slab_allocators the number of blocks allocated by usb_alloc_urb() increases, however, the xawtv is no longer running:

size-2048: 18 usb_alloc_dev+0x1d/0x212 [usbcore]
size-2048: 2280 usb_alloc_urb+0xc/0x2b [usbcore]
size-1024: 100 usb_alloc_urb+0xc/0x2b [usbcore]
size-128: 10 usb_alloc_urb+0xc/0x2b [usbcore]

Each time xawtv is started and stopped the value increases at the usb_alloc_urb().

Expected result: the same memory usage is reached again after xawtv exited.
Comment 1 Márton Németh 2009-02-23 22:12:38 UTC
Created attachment 20345 [details]
2.6.28 kernel config

CONFIG_DEBUG_SLAB_LEAK=y was used to have statistics about memory usage in /proc/slab_allocators .
Comment 2 Márton Németh 2009-02-23 22:16:09 UTC
Created attachment 20346 [details]
output of v4l-info for uvcvideo driver with CNF7129 webcam
Comment 3 Márton Németh 2009-02-23 22:17:41 UTC
Created attachment 20347 [details]
lsusb and lsusb -v

$ lsusb 
Bus 005 Device 003: ID 04f2:b071 Chicony Electronics Co., Ltd 
Bus 005 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub
Bus 003 Device 001: ID 1d6b:0001 Linux Foundation 1.1 root hub
Bus 004 Device 001: ID 1d6b:0001 Linux Foundation 1.1 root hub
Bus 002 Device 003: ID 1241:1503 Belkin Keyboard
Bus 002 Device 001: ID 1d6b:0001 Linux Foundation 1.1 root hub
Bus 001 Device 001: ID 1d6b:0001 Linux Foundation 1.1 root hub
Comment 4 Márton Németh 2009-02-23 22:50:33 UTC
Memory is also leaked when running svv (from http://moinejf.free.fr/svv.c
 ) together with libv4l-0.5.8 (from http://people.atrpms.net/~hdegoede/libv4l-0.5.8.tar.gz ) instead of xawtv.

In this case only 5 times 1024 bytes are leaked for each svv start and exit.
Comment 5 Márton Németh 2009-02-23 23:32:15 UTC
The memory is also leaked with Linux kernel 2.6.29-rc6.
Comment 6 Anonymous Emailer 2009-02-24 13:58:04 UTC
Reply-To: akpm@linux-foundation.org


(switched to email.  Please respond via emailed reply-to-all, not via the
bugzilla web interface).

On Mon, 23 Feb 2009 22:08:37 -0800 (PST)
bugme-daemon@bugzilla.kernel.org wrote:

> http://bugzilla.kernel.org/show_bug.cgi?id=12768

There's additional info at the link.

>            Summary: usb_alloc_urb() leaks memory together with uvcvideo
>                     driver
>            Product: Drivers
>            Version: 2.5
>      KernelVersion: 2.6.28
>           Platform: All
>         OS/Version: Linux
>               Tree: Mainline
>             Status: NEW
>           Severity: normal
>           Priority: P1
>          Component: USB
>         AssignedTo: greg@kroah.com
>         ReportedBy: nm127@freemail.hu
> 
> 
> Latest working kernel version:
> Earliest failing kernel version:
> Distribution:
> Hardware Environment: EeePC 901
> Software Environment: Debian 5.0
> Problem Description:
> 
> Steps to reproduce:
> 1. Boot the system
> 2. start an xterm window and execute the following command:
> 
> $ while true; do clear; cat /proc/slab_allocators |grep usb_alloc; sleep 1;
> done
> 
> This will print out similar lines each second:
> 
> size-2048: 18 usb_alloc_dev+0x1d/0x212 [usbcore]
> size-2048: 2280 usb_alloc_urb+0xc/0x2b [usbcore]
> size-1024: 85 usb_alloc_urb+0xc/0x2b [usbcore]
> size-128: 10 usb_alloc_urb+0xc/0x2b [usbcore]
> 
> 3. Start xawtv, this will show the picture of the webcam
> 4. Exit xawtv
> 
> Current result:
> In the output of /proc/slab_allocators the number of blocks allocated by
> usb_alloc_urb() increases, however, the xawtv is no longer running:
> 
> size-2048: 18 usb_alloc_dev+0x1d/0x212 [usbcore]
> size-2048: 2280 usb_alloc_urb+0xc/0x2b [usbcore]
> size-1024: 100 usb_alloc_urb+0xc/0x2b [usbcore]
> size-128: 10 usb_alloc_urb+0xc/0x2b [usbcore]
> 
> Each time xawtv is started and stopped the value increases at the
> usb_alloc_urb().
> 
> Expected result: the same memory usage is reached again after xawtv exited.
> 

I assume this is a v4l bug and not a USB core bug?
Comment 7 Markus Rechberger 2009-02-24 14:37:57 UTC
On Tue, Feb 24, 2009 at 10:57 PM, Andrew Morton
<akpm@linux-foundation.org> wrote:
>
> (switched to email. 
Comment 8 Mauro Carvalho Chehab 2009-02-24 18:04:01 UTC
On Tue, 24 Feb 2009 13:57:20 -0800
Andrew Morton <akpm@linux-foundation.org> wrote:

>> > In the output of /proc/slab_allocators the number of blocks allocated by
> > usb_alloc_urb() increases, however, the xawtv is no longer running:
> > 
> > size-2048: 18 usb_alloc_dev+0x1d/0x212 [usbcore]
> > size-2048: 2280 usb_alloc_urb+0xc/0x2b [usbcore]
> > size-1024: 100 usb_alloc_urb+0xc/0x2b [usbcore]
> > size-128: 10 usb_alloc_urb+0xc/0x2b [usbcore]
> > 
> > Each time xawtv is started and stopped the value increases at the
> > usb_alloc_urb().
> > 
> > Expected result: the same memory usage is reached again after xawtv exited.
> > 
> 
> I assume this is a v4l bug and not a USB core bug?

I guess this is a v4l bug. We've found several memory leaks on em28xx driver,
fixed at the development -git:

http://git.kernel.org/?p=linux/kernel/git/mchehab/devel.git

I'll do some tests again with the latest em28xx driver to double check if it is
there any other memory leak. If not, then we could replicate the same approach
into uvcvideo.

Cheers,
Mauro
Comment 9 Markus Rechberger 2009-02-24 18:15:44 UTC
On Wed, Feb 25, 2009 at 3:02 AM, Mauro Carvalho Chehab
<mchehab@infradead.org> wrote:
> On Tue, 24 Feb 2009 13:57:20 -0800
> Andrew Morton <akpm@linux-foundation.org> wrote:
>
>>> > In the output of /proc/slab_allocators the number of blocks allocated by
>> > usb_alloc_urb() increases, however, the xawtv is no longer running:
>> >
>> > size-2048: 18 usb_alloc_dev+0x1d/0x212 [usbcore]
>> > size-2048: 2280 usb_alloc_urb+0xc/0x2b [usbcore]
>> > size-1024: 100 usb_alloc_urb+0xc/0x2b [usbcore]
>> > size-128: 10 usb_alloc_urb+0xc/0x2b [usbcore]
>> >
>> > Each time xawtv is started and stopped the value increases at the
>> > usb_alloc_urb().
>> >
>> > Expected result: the same memory usage is reached again after xawtv
>> exited.
>> >
>>
>> I assume this is a v4l bug and not a USB core bug?
>
> I guess this is a v4l bug. We've found several memory leaks on em28xx driver,
> fixed at the development -git:
>
> http://git.kernel.org/?p=linux/kernel/git/mchehab/devel.git
>
> I'll do some tests again with the latest em28xx driver to double check if it
> is
> there any other memory leak. If not, then we could replicate the same
> approach
> into uvcvideo.
>

haha you never even had a look at the issue itself.

Markus

> Cheers,
> Mauro
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at 
Comment 10 Anonymous Emailer 2009-02-24 19:24:18 UTC
Reply-To: akpm@linux-foundation.org

On Wed, 25 Feb 2009 03:15:35 +0100 Markus Rechberger <mrechberger@gmail.com> wrote:

> On Wed, Feb 25, 2009 at 3:02 AM, Mauro Carvalho Chehab
> <mchehab@infradead.org> wrote:
> > On Tue, 24 Feb 2009 13:57:20 -0800
> > Andrew Morton <akpm@linux-foundation.org> wrote:
> >
> >>> > In the output of /proc/slab_allocators the number of blocks allocated
> by
> >> > usb_alloc_urb() increases, however, the xawtv is no longer running:
> >> >
> >> > size-2048: 18 usb_alloc_dev+0x1d/0x212 [usbcore]
> >> > size-2048: 2280 usb_alloc_urb+0xc/0x2b [usbcore]
> >> > size-1024: 100 usb_alloc_urb+0xc/0x2b [usbcore]
> >> > size-128: 10 usb_alloc_urb+0xc/0x2b [usbcore]
> >> >
> >> > Each time xawtv is started and stopped the value increases at the
> >> > usb_alloc_urb().
> >> >
> >> > Expected result: the same memory usage is reached again after xawtv
> exited.
> >> >
> >>
> >> I assume this is a v4l bug and not a USB core bug?
> >
> > I guess this is a v4l bug. We've found several memory leaks on em28xx
> driver,
> > fixed at the development -git:
> >
> > http://git.kernel.org/?p=linux/kernel/git/mchehab/devel.git
> >
> > I'll do some tests again with the latest em28xx driver to double check if
> it is
> > there any other memory leak. If not, then we could replicate the same
> approach
> > into uvcvideo.
> >
> 
> haha you never even had a look at the issue itself.

What a perfectly useless comment.
Comment 11 Markus Rechberger 2009-02-24 19:35:47 UTC
On Wed, Feb 25, 2009 at 4:23 AM, Andrew Morton
<akpm@linux-foundation.org> wrote:
> On Wed, 25 Feb 2009 03:15:35 +0100 Markus Rechberger <mrechberger@gmail.com>
> wrote:
>
>> On Wed, Feb 25, 2009 at 3:02 AM, Mauro Carvalho Chehab
>> <mchehab@infradead.org> wrote:
>> > On Tue, 24 Feb 2009 13:57:20 -0800
>> > Andrew Morton <akpm@linux-foundation.org> wrote:
>> >
>> >>> > In the output of /proc/slab_allocators the number of blocks allocated
>> by
>> >> > usb_alloc_urb() increases, however, the xawtv is no longer running:
>> >> >
>> >> > size-2048: 18 usb_alloc_dev+0x1d/0x212 [usbcore]
>> >> > size-2048: 2280 usb_alloc_urb+0xc/0x2b [usbcore]
>> >> > size-1024: 100 usb_alloc_urb+0xc/0x2b [usbcore]
>> >> > size-128: 10 usb_alloc_urb+0xc/0x2b [usbcore]
>> >> >
>> >> > Each time xawtv is started and stopped the value increases at the
>> >> > usb_alloc_urb().
>> >> >
>> >> > Expected result: the same memory usage is reached again after xawtv
>> exited.
>> >> >
>> >>
>> >> I assume this is a v4l bug and not a USB core bug?
>> >
>> > I guess this is a v4l bug. We've found several memory leaks on em28xx
>> driver,
>> > fixed at the development -git:
>> >
>> > http://git.kernel.org/?p=linux/kernel/git/mchehab/devel.git
>> >
>> > I'll do some tests again with the latest em28xx driver to double check if
>> it is
>> > there any other memory leak. If not, then we could replicate the same
>> approach
>> > into uvcvideo.
>> >
>>
>> haha you never even had a look at the issue itself.
>
> What a perfectly useless comment.
>

not more or less useless than the one before

http://mcentral.de/pipermail/em28xx/2009-February/002446.html

I followed the other em28xx work too and there has not been anything
which addressed that issue.

Markus
Comment 12 Márton Németh 2009-02-24 23:44:38 UTC
Please, please, please, can we concentrate on the subject?

What I did with the other out-of-tree em28xx-new driver was that
I printed out the urb->kref.refcount before and after each urb operation.

The result was that when the urb->complete function is called, the reference
count was still 2, instead of 1.

I could imagine three possible errors:
1. there is a bug in uvcvideo driver
2. there is a bug in v4l framework
3. there is a bug in usb subsystem

It would be good if someone who have a deeper knowledge than me on these fields could
give some hints or debug patches which would lead us closer to the solution.

Regards,

	M
Comment 13 Márton Németh 2009-02-25 05:22:36 UTC
Executing the following command also leaks 5 times 1024 bytes at usb_alloc_urb():

v4lctl snap jpeg full test.jpg
Comment 14 Mauro Carvalho Chehab 2009-02-25 07:50:27 UTC
On Wed, 25 Feb 2009 08:44:00 +0100
Németh Márton <nm127@freemail.hu> wrote:

> What I did with the other out-of-tree em28xx-new driver was that
> I printed out the urb->kref.refcount before and after each urb operation.
> 
> The result was that when the urb->complete function is called, the reference
> count was still 2, instead of 1.
> 
> I could imagine three possible errors:
> 1. there is a bug in uvcvideo driver
> 2. there is a bug in v4l framework
> 3. there is a bug in usb subsystem
> 
> It would be good if someone who have a deeper knowledge than me on these
> fields could
> give some hints or debug patches which would lead us closer to the solution.

Márton,

I did a test yesterday night with 2.6.29-rc6. The em28xx in-kernel still has
same problem we focused a while ago (except that, before, memory were going
exausted on a much higher rate, and we had memory leaks for every close() call).

What happens is that, sometimes, memory are not being freed by
usb_kill_urb()/usb_unlink_urb(). I'm trying to debug it right now, to
understand what's happening.


Cheers,
Mauro
Comment 15 Márton Németh 2009-02-25 08:26:44 UTC
Created attachment 20364 [details]
debug patch for memory leak

I applied the attached debug patch to 2.6.29-rc6. This prints out the urb->kref.refcount near to usb_alloc_urb(), usb_submit_urb() and usb_kill_urb() calls.

I did the following steps:
1. applied the attached patch
2. compiled and installed the new uvcvideo driver
3. rmmod uvcvideo
4. modprobe -k uvcvideo
5. executed the command: v4lctl snap jpeg full test.jpg
6. rmmod uvcvideo

The dmesg looks like this:

[11092.417202] uvcvideo: Found UVC 1.00 device CNF7129 (04f2:b071)
[11092.438007] input: CNF7129 as /class/input/input16
[11092.460403] drivers/media/video/uvc/uvc_status.c:190: urb=f67bcf68, refcount=1
[11092.460414] drivers/media/video/uvc/uvc_status.c:211: urb=f67bcf68, refcount=1
[11092.460459] drivers/media/video/uvc/uvc_status.c:214: urb=f67bcf68, refcount=2
[11092.460652] usbcore: registered new interface driver uvcvideo
[11092.460670] USB Video Class driver (v0.1.0)
[11111.748524] drivers/media/video/uvc/uvc_video.c:792: urb=f0a552e0, refcount=1
[11111.748544] drivers/media/video/uvc/uvc_video.c:792: urb=f616c240, refcount=1
[11111.748558] drivers/media/video/uvc/uvc_video.c:792: urb=f0aba9b0, refcount=1
[11111.748573] drivers/media/video/uvc/uvc_video.c:792: urb=f0a95120, refcount=1
[11111.748587] drivers/media/video/uvc/uvc_video.c:792: urb=f0abba10, refcount=1
[11112.525005] drivers/media/video/uvc/uvc_video.c:745: urb=f0a552e0, refcount=38
[11112.538111] drivers/media/video/uvc/uvc_video.c:748: urb=f0a552e0, refcount=33
[11112.538120] drivers/media/video/uvc/uvc_video.c:745: urb=f616c240, refcount=38
[11112.543107] drivers/media/video/uvc/uvc_video.c:748: urb=f616c240, refcount=33
[11112.543115] drivers/media/video/uvc/uvc_video.c:745: urb=f0aba9b0, refcount=38
[11112.548110] drivers/media/video/uvc/uvc_video.c:748: urb=f0aba9b0, refcount=33
[11112.548117] drivers/media/video/uvc/uvc_video.c:745: urb=f0a95120, refcount=38
[11112.553105] drivers/media/video/uvc/uvc_video.c:748: urb=f0a95120, refcount=33
[11112.553113] drivers/media/video/uvc/uvc_video.c:745: urb=f0abba10, refcount=38
[11112.558150] drivers/media/video/uvc/uvc_video.c:748: urb=f0abba10, refcount=33
[11738.783652] usbcore: deregistering interface driver uvcvideo
[11738.784187] drivers/media/video/uvc/uvc_status.c:221: urb=f67bcf68, refcount=1
[11738.784199] drivers/media/video/uvc/uvc_status.c:224: urb=f67bcf68, refcount=1

It seems that the urb at f67bcf68 gets freed correctly. However, following 5 urbs at f0a552e0, f616c240, f0aba9b0, f0a95120 and f0abba10 are not.
Comment 16 Márton Németh 2009-02-25 09:17:08 UTC
I could find a way which removes the mentioned bolcoks from /proc/slab_allocators (I hope the memory is free()d and not just disappears from the list):

1. rmmod uvcvideo
2. rmmod ehci_hcd
3. rmmod usbcore
4. modprobe -k uvcvideo
5. modprobe -k ehci_hcd
6. executed the command: v4lctl snap jpeg full test.jpg
   Now 5 blocks are not freed
7. rmmod uvcvideo
8. rmmod ehci_hcd
9. rmmod usbcore

When the usbcore module is unloaded, the blocks are no longer listed in /proc/slab_allocators.
Comment 17 Márton Németh 2009-02-25 09:25:51 UTC
(In reply to comment #16)
> I could find a way which removes the mentioned bolcoks from
> /proc/slab_allocators (I hope the memory is free()d and not just disappears
> from the list)

Sorry, the memory is not freed, just there is no symbol name anymore when the usbcore module is unloaded. The list item looks like this:

size-1024: 5 f834c1e63
Comment 18 Márton Németh 2009-02-25 10:27:35 UTC
I found an interesting article on lwn.net: 

Detecting kernel memory leaks: http://lwn.net/Articles/187979/

Unfortunately the latest patch is for 2.6.20-rc1:
http://homepage.ntlworld.com/cmarinas/kmemleak/
Comment 19 Laurent Pinchart 2009-02-25 13:52:23 UTC
On Wednesday 25 February 2009 13:49:29 Mauro Carvalho Chehab wrote:
> On Wed, 25 Feb 2009 08:44:00 +0100
>
> Németh Márton <nm127@freemail.hu> wrote:
> > What I did with the other out-of-tree em28xx-new driver was that
> > I printed out the urb->kref.refcount before and after each urb operation.
> >
> > The result was that when the urb->complete function is called, the
> > reference count was still 2, instead of 1.
> >
> > I could imagine three possible errors:
> > 1. there is a bug in uvcvideo driver
> > 2. there is a bug in v4l framework
> > 3. there is a bug in usb subsystem

I'd vote for 3 (with an option on 1, just in case).

> > It would be good if someone who have a deeper knowledge than me on these
> > fields could give some hints or debug patches which would lead us closer
> > to the solution.
>
> Márton,
>
> I did a test yesterday night with 2.6.29-rc6. The em28xx in-kernel still
> has same problem we focused a while ago (except that, before, memory were
> going exausted on a much higher rate, and we had memory leaks for every
> close() call).
>
> What happens is that, sometimes, memory are not being freed by
> usb_kill_urb()/usb_unlink_urb(). I'm trying to debug it right now, to
> understand what's happening.

Could this

http://article.gmane.org/gmane.linux.usb.general/15315/match=urb+leak

be related ?

Cheers,

Laurent Pinchart
Comment 20 Mauro Carvalho Chehab 2009-02-25 17:40:42 UTC
On Wed, 25 Feb 2009 22:55:43 +0100
Laurent Pinchart <laurent.pinchart@skynet.be> wrote:

> Could this
> 
> http://article.gmane.org/gmane.linux.usb.general/15315/match=urb+leak
> 
> be related ?

This seems to solve the bug with the em28xx driver. I'll let the stress test
run over the night and see if is everything is all right.

Cheers,
Mauro
Comment 21 Márton Németh 2009-02-25 22:54:06 UTC
(In reply to comment #19)
> Could this
> 
> http://article.gmane.org/gmane.linux.usb.general/15315/match=urb+leak
> 
> be related ?

Yes, that patch fixes the uvcvideo memory leak also.
Comment 22 Márton Németh 2009-03-02 23:08:56 UTC
(In reply to comment #19)
> http://article.gmane.org/gmane.linux.usb.general/15315/match=urb+leak

A link to read the follow-up messages on that thread: http://marc.info/?t=123541378600005&r=1&w=2
Comment 23 Márton Németh 2009-03-22 04:43:09 UTC
(In reply to comment #22)
> (In reply to comment #19)
> > http://article.gmane.org/gmane.linux.usb.general/15315/match=urb+leak
> 
> A link to read the follow-up messages on that thread:
> http://marc.info/?t=123541378600005&r=1&w=2

A new version of the patch was submitted at http://lkml.org/lkml/2009/3/20/463 by Greg KH.
Comment 24 Laurent Pinchart 2009-05-16 20:39:16 UTC
The patch has been applied before 2.6.29. Can someone please close this bug ?