leak in do_ubd_request

leak in do_ubd_request

[Miklos Szeredi]

When writing big files on 2.6.24-rc3-mm1, there can be substiantial
number of leaked requests (CONFIG_DEBUG_SLAB_LEAK enabled):

# cat /proc/slab_allocators |grep do_ubd_request
size-128: 25687 do_ubd_request+0xe0/0x19d

There seems to be no corruption though.  Looking at the code, I can't
see how the requests could leak...

Miklos

Re: leak in do_ubd_request

[Miklos Szeredi]

> There seems to be no corruption though.  Looking at the code, I can't
> see how the requests could leak...

Didn't look close enough.

Index: linux/arch/um/drivers/ubd_kern.c
===================================================================
--- linux.orig/arch/um/drivers/ubd_kern.c	2007-11-27 19:23:37.000000000 +0100
+++ linux/arch/um/drivers/ubd_kern.c	2007-11-27 19:24:24.000000000 +0100
@@ -1132,6 +1132,7 @@ static void do_ubd_request(struct reques
 					       "errno = %d\n", -n);
 				else if(list_empty(&dev->restart))
 					list_add(&dev->restart, &restart);
+				kfree(io_req);
 				return;
 			}
 

Re: [uml-devel] leak in do_ubd_request

[Jeff Dike]

On Tue, Nov 27, 2007 at 07:26:38PM +0100, Miklos Szeredi wrote:
> +				kfree(io_req);

Whoops, nice catch.

Can you sign that off for me?

				Jeff

-- 
Work email - jdike at linux dot intel dot com

Re: [uml-devel] leak in do_ubd_request

[Miklos Szeredi]

> On Tue, Nov 27, 2007 at 07:26:38PM +0100, Miklos Szeredi wrote:
> > +				kfree(io_req);
> 
> Whoops, nice catch.
> 
> Can you sign that off for me?

Sure.  The patch works for me, but please check that it also makes
sense.

Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>

Re: [uml-devel] leak in do_ubd_request

[Jeff Dike]

On Tue, Nov 27, 2007 at 09:29:23PM +0100, Miklos Szeredi wrote:
> Sure.  The patch works for me, but please check that it also makes
> sense.

I did - it's a straight-forward leak and fix.

				Jeff

-- 
Work email - jdike at linux dot intel dot com

Re: [uml-devel] leak in do_ubd_request

[Andrew Morton]

On Tue, 27 Nov 2007 17:20:20 -0500 Jeff Dike <jdike@addtoit.com> wrote:

> On Tue, Nov 27, 2007 at 09:29:23PM +0100, Miklos Szeredi wrote:
> > Sure.  The patch works for me, but please check that it also makes
> > sense.
> 
> I did - it's a straight-forward leak and fix.
> 

Do we have any idea which patch this patch fixes?

Re: [uml-devel] leak in do_ubd_request

[Miklos Szeredi]

> On Tue, 27 Nov 2007 17:20:20 -0500 Jeff Dike <jdike@addtoit.com> wrote:
> 
> > On Tue, Nov 27, 2007 at 09:29:23PM +0100, Miklos Szeredi wrote:
> > > Sure.  The patch works for me, but please check that it also makes
> > > sense.
> > 
> > I did - it's a straight-forward leak and fix.
> > 
> 
> Do we have any idea which patch this patch fixes?

I seems like a mainline bug, and has been present for some time
(forever?).

I have no idea why this wasn't noticed earlier.

Miklos

Re: [uml-devel] leak in do_ubd_request

[Jeff Dike]

On Wed, Nov 28, 2007 at 10:37:53AM +0100, Miklos Szeredi wrote:
> > Do we have any idea which patch this patch fixes?
> 
> I seems like a mainline bug, and has been present for some time
> (forever?).
> 
> I have no idea why this wasn't noticed earlier.

Not forever, but it's been there for a while - it was introduced here:

commit 2adcec2197897365e0a0f657f1098cbfdb44bc8b
Author: Jeff Dike <jdike@addtoit.com>
Date:   Sun May 6 14:51:37 2007 -0700

    uml: send pointers instead of structures to I/O thread
    
    Instead of writing entire structures between UML and the I/O thread, we send
    pointers.  This cuts down on the amount of data being copied and possibly
    allows more requests to be pending between the two.
    
    This requires that the requests be kmalloced and freed instead of living on
    the stack.
    
    Signed-off-by: Jeff Dike <jdike@linux.intel.com>
    Cc: Paolo 'Blaisorblade' Giarrusso <blaisorblade@yahoo.it>
    Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
    Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

				Jeff

-- 
Work email - jdike at linux dot intel dot com