* [PATCH] fix memory leak in mm/slab.c::alloc_kmemlist()
@ 2006-03-12 13:28 Jesper Juhl
2006-03-12 22:41 ` Andrew Morton
0 siblings, 1 reply; 6+ messages in thread
From: Jesper Juhl @ 2006-03-12 13:28 UTC (permalink / raw)
To: linux-kernel; +Cc: Andrew Morton, Jesper Juhl
The Coverity checker found that we may leak memory in
mm/slab.c::alloc_kmemlist()
This should fix the leak and coverity bug #589
Signed-off-by: Jesper Juhl <jesper.juhl@gmail.com>
---
mm/slab.c | 4 +++-
1 files changed, 3 insertions(+), 1 deletion(-)
--- linux-2.6.16-rc6-orig/mm/slab.c 2006-03-12 14:19:17.000000000 +0100
+++ linux-2.6.16-rc6/mm/slab.c 2006-03-12 14:22:40.000000000 +0100
@@ -3366,8 +3366,10 @@ static int alloc_kmemlist(struct kmem_ca
continue;
}
if (!(l3 = kmalloc_node(sizeof(struct kmem_list3),
- GFP_KERNEL, node)))
+ GFP_KERNEL, node))) {
+ kfree(new);
goto fail;
+ }
kmem_list3_init(l3);
l3->next_reap = jiffies + REAPTIMEOUT_LIST3 +
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] fix memory leak in mm/slab.c::alloc_kmemlist()
2006-03-12 13:28 [PATCH] fix memory leak in mm/slab.c::alloc_kmemlist() Jesper Juhl
@ 2006-03-12 22:41 ` Andrew Morton
2006-03-13 7:34 ` Jesper Juhl
0 siblings, 1 reply; 6+ messages in thread
From: Andrew Morton @ 2006-03-12 22:41 UTC (permalink / raw)
To: Jesper Juhl; +Cc: linux-kernel, jesper.juhl, Christoph Lameter
Jesper Juhl <jesper.juhl@gmail.com> wrote:
>
>
> The Coverity checker found that we may leak memory in
> mm/slab.c::alloc_kmemlist()
> This should fix the leak and coverity bug #589
>
>
> Signed-off-by: Jesper Juhl <jesper.juhl@gmail.com>
> ---
>
> mm/slab.c | 4 +++-
> 1 files changed, 3 insertions(+), 1 deletion(-)
>
> --- linux-2.6.16-rc6-orig/mm/slab.c 2006-03-12 14:19:17.000000000 +0100
> +++ linux-2.6.16-rc6/mm/slab.c 2006-03-12 14:22:40.000000000 +0100
> @@ -3366,8 +3366,10 @@ static int alloc_kmemlist(struct kmem_ca
> continue;
> }
> if (!(l3 = kmalloc_node(sizeof(struct kmem_list3),
> - GFP_KERNEL, node)))
> + GFP_KERNEL, node))) {
> + kfree(new);
> goto fail;
> + }
>
> kmem_list3_init(l3);
> l3->next_reap = jiffies + REAPTIMEOUT_LIST3 +
It's more complicated than that. We can also leak new_alien. And if any
allocation in that for_each_online_node() loop fails I guess we need to
back out all the allocations we've done thus far, which means another loop.
ug.
Patches against rc6-mm1 would be preferred please, that code's changed
quite a bit.
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] fix memory leak in mm/slab.c::alloc_kmemlist()
2006-03-12 22:41 ` Andrew Morton
@ 2006-03-13 7:34 ` Jesper Juhl
2006-03-13 21:33 ` Jesper Juhl
0 siblings, 1 reply; 6+ messages in thread
From: Jesper Juhl @ 2006-03-13 7:34 UTC (permalink / raw)
To: Andrew Morton; +Cc: linux-kernel, Christoph Lameter
On 3/12/06, Andrew Morton <akpm@osdl.org> wrote:
> Jesper Juhl <jesper.juhl@gmail.com> wrote:
> >
> >
> > The Coverity checker found that we may leak memory in
> > mm/slab.c::alloc_kmemlist()
> > This should fix the leak and coverity bug #589
> >
[snip]
>
> It's more complicated than that. We can also leak new_alien. And if any
> allocation in that for_each_online_node() loop fails I guess we need to
> back out all the allocations we've done thus far, which means another loop.
> ug.
>
Ok, I'll take a second (and much more thorough) look at it tonight.
And I'll be sure to describe whatever changes I make and the reasoning
behind.
> Patches against rc6-mm1 would be preferred please, that code's changed
> quite a bit.
>
Sure thing.
--
Jesper Juhl <jesper.juhl@gmail.com>
Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html
Plain text mails only, please http://www.expita.com/nomime.html
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] fix memory leak in mm/slab.c::alloc_kmemlist()
2006-03-13 7:34 ` Jesper Juhl
@ 2006-03-13 21:33 ` Jesper Juhl
2006-03-13 21:46 ` Christoph Lameter
0 siblings, 1 reply; 6+ messages in thread
From: Jesper Juhl @ 2006-03-13 21:33 UTC (permalink / raw)
To: Andrew Morton; +Cc: linux-kernel, Christoph Lameter
On 3/13/06, Jesper Juhl <jesper.juhl@gmail.com> wrote:
> On 3/12/06, Andrew Morton <akpm@osdl.org> wrote:
> > Jesper Juhl <jesper.juhl@gmail.com> wrote:
> > >
> > >
> > > The Coverity checker found that we may leak memory in
> > > mm/slab.c::alloc_kmemlist()
> > > This should fix the leak and coverity bug #589
> > >
> [snip]
> >
> > It's more complicated than that. We can also leak new_alien. And if any
> > allocation in that for_each_online_node() loop fails I guess we need to
> > back out all the allocations we've done thus far, which means another loop.
> > ug.
> >
> Ok, I'll take a second (and much more thorough) look at it tonight.
> And I'll be sure to describe whatever changes I make and the reasoning
> behind.
>
>
> > Patches against rc6-mm1 would be preferred please, that code's changed
> > quite a bit.
> >
> Sure thing.
>
Ok, I've been playing around with a few ways to resolve this, but I'm
a bit pressed for time and won't have properly tested patches ready
tonight. I will however keep at this, so you'll see patches
releatively shortly, just give me another day or two and I'll have
this fixed in a nice way (nice little task to work at :) ...
--
Jesper Juhl <jesper.juhl@gmail.com>
Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html
Plain text mails only, please http://www.expita.com/nomime.html
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] fix memory leak in mm/slab.c::alloc_kmemlist()
2006-03-13 21:33 ` Jesper Juhl
@ 2006-03-13 21:46 ` Christoph Lameter
2006-03-13 21:59 ` Jesper Juhl
0 siblings, 1 reply; 6+ messages in thread
From: Christoph Lameter @ 2006-03-13 21:46 UTC (permalink / raw)
To: Jesper Juhl; +Cc: Andrew Morton, linux-kernel
On Mon, 13 Mar 2006, Jesper Juhl wrote:
> Ok, I've been playing around with a few ways to resolve this, but I'm
> a bit pressed for time and won't have properly tested patches ready
> tonight. I will however keep at this, so you'll see patches
> releatively shortly, just give me another day or two and I'll have
> this fixed in a nice way (nice little task to work at :) ...
Maybe extract a alloc_kmemlist_node and free_kmemlist_node from
alloc_kmemlist()? It gets a bit complicated if all of this is handled within
alloc_kmemlist and having these separate may simplify recovery on out of
memory.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] fix memory leak in mm/slab.c::alloc_kmemlist()
2006-03-13 21:46 ` Christoph Lameter
@ 2006-03-13 21:59 ` Jesper Juhl
0 siblings, 0 replies; 6+ messages in thread
From: Jesper Juhl @ 2006-03-13 21:59 UTC (permalink / raw)
To: Christoph Lameter; +Cc: Andrew Morton, linux-kernel
On 3/13/06, Christoph Lameter <clameter@sgi.com> wrote:
> On Mon, 13 Mar 2006, Jesper Juhl wrote:
>
> > Ok, I've been playing around with a few ways to resolve this, but I'm
> > a bit pressed for time and won't have properly tested patches ready
> > tonight. I will however keep at this, so you'll see patches
> > releatively shortly, just give me another day or two and I'll have
> > this fixed in a nice way (nice little task to work at :) ...
>
> Maybe extract a alloc_kmemlist_node and free_kmemlist_node from
> alloc_kmemlist()? It gets a bit complicated if all of this is handled within
> alloc_kmemlist and having these separate may simplify recovery on out of
> memory.
>
Nice Idea. So far I've been trying to handle it all inside the
function, but maybe sepperating stuff out might simplify it..
anyway, i'll keep working on this and will submit patches in a day or two...
It really is a nice little problem to work on and I'll find a solution
for it (well, I already have at least one, but it's ugly as hell, want
to find a better one) - gimme a few days.
--
Jesper Juhl <jesper.juhl@gmail.com>
Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html
Plain text mails only, please http://www.expita.com/nomime.html
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2006-03-13 21:59 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-03-12 13:28 [PATCH] fix memory leak in mm/slab.c::alloc_kmemlist() Jesper Juhl
2006-03-12 22:41 ` Andrew Morton
2006-03-13 7:34 ` Jesper Juhl
2006-03-13 21:33 ` Jesper Juhl
2006-03-13 21:46 ` Christoph Lameter
2006-03-13 21:59 ` Jesper Juhl
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox