* [PATCH] slub: fix a memory leak in get_partial_node()
@ 2012-05-15 19:01 ` Joonsoo Kim
0 siblings, 0 replies; 21+ messages in thread
From: Joonsoo Kim @ 2012-05-15 19:01 UTC (permalink / raw)
To: Pekka Enberg
Cc: Christoph Lameter, linux-kernel, linux-mm, Greg Kroah-Hartman,
stable, Joonsoo Kim
In the case which is below,
1. acquire slab for cpu partial list
2. free object to it by remote cpu
3. page->freelist = t
then memory leak is occurred.
Change acquire_slab() not to zap freelist when it works for cpu partial list.
I think it is a sufficient solution for fixing a memory leak.
Below is output of 'slabinfo -r kmalloc-256'
when './perf stat -r 30 hackbench 50 process 4000 > /dev/null' is done.
***Vanilla***
Sizes (bytes) Slabs Debug Memory
------------------------------------------------------------------------
Object : 256 Total : 468 Sanity Checks : Off Total: 3833856
SlabObj: 256 Full : 111 Redzoning : Off Used : 2004992
SlabSiz: 8192 Partial: 302 Poisoning : Off Loss : 1828864
Loss : 0 CpuSlab: 55 Tracking : Off Lalig: 0
Align : 8 Objects: 32 Tracing : Off Lpadd: 0
***Patched***
Sizes (bytes) Slabs Debug Memory
------------------------------------------------------------------------
Object : 256 Total : 300 Sanity Checks : Off Total: 2457600
SlabObj: 256 Full : 204 Redzoning : Off Used : 2348800
SlabSiz: 8192 Partial: 33 Poisoning : Off Loss : 108800
Loss : 0 CpuSlab: 63 Tracking : Off Lalig: 0
Align : 8 Objects: 32 Tracing : Off Lpadd: 0
Total and loss number is the impact of this patch.
Signed-off-by: Joonsoo Kim <js1304@gmail.com>
diff --git a/mm/slub.c b/mm/slub.c
index ffe13fd..a7a766a 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -1514,15 +1514,19 @@ static inline void *acquire_slab(struct kmem_cache *s,
freelist = page->freelist;
counters = page->counters;
new.counters = counters;
- if (mode)
+ if (mode) {
new.inuse = page->objects;
+ new.freelist = NULL;
+ } else {
+ new.freelist = freelist;
+ }
VM_BUG_ON(new.frozen);
new.frozen = 1;
} while (!__cmpxchg_double_slab(s, page,
freelist, counters,
- NULL, new.counters,
+ new.freelist, new.counters,
"lock and freeze"));
remove_partial(n, page);
@@ -1564,7 +1568,6 @@ static void *get_partial_node(struct kmem_cache *s,
object = t;
available = page->objects - page->inuse;
} else {
- page->freelist = t;
available = put_cpu_partial(s, page, 0);
stat(s, CPU_PARTIAL_NODE);
}
--
1.7.9.5
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH] slub: fix a memory leak in get_partial_node()
@ 2012-05-15 19:01 ` Joonsoo Kim
0 siblings, 0 replies; 21+ messages in thread
From: Joonsoo Kim @ 2012-05-15 19:01 UTC (permalink / raw)
To: Pekka Enberg
Cc: Christoph Lameter, linux-kernel, linux-mm, Greg Kroah-Hartman,
stable, Joonsoo Kim
In the case which is below,
1. acquire slab for cpu partial list
2. free object to it by remote cpu
3. page->freelist = t
then memory leak is occurred.
Change acquire_slab() not to zap freelist when it works for cpu partial list.
I think it is a sufficient solution for fixing a memory leak.
Below is output of 'slabinfo -r kmalloc-256'
when './perf stat -r 30 hackbench 50 process 4000 > /dev/null' is done.
***Vanilla***
Sizes (bytes) Slabs Debug Memory
------------------------------------------------------------------------
Object : 256 Total : 468 Sanity Checks : Off Total: 3833856
SlabObj: 256 Full : 111 Redzoning : Off Used : 2004992
SlabSiz: 8192 Partial: 302 Poisoning : Off Loss : 1828864
Loss : 0 CpuSlab: 55 Tracking : Off Lalig: 0
Align : 8 Objects: 32 Tracing : Off Lpadd: 0
***Patched***
Sizes (bytes) Slabs Debug Memory
------------------------------------------------------------------------
Object : 256 Total : 300 Sanity Checks : Off Total: 2457600
SlabObj: 256 Full : 204 Redzoning : Off Used : 2348800
SlabSiz: 8192 Partial: 33 Poisoning : Off Loss : 108800
Loss : 0 CpuSlab: 63 Tracking : Off Lalig: 0
Align : 8 Objects: 32 Tracing : Off Lpadd: 0
Total and loss number is the impact of this patch.
Signed-off-by: Joonsoo Kim <js1304@gmail.com>
diff --git a/mm/slub.c b/mm/slub.c
index ffe13fd..a7a766a 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -1514,15 +1514,19 @@ static inline void *acquire_slab(struct kmem_cache *s,
freelist = page->freelist;
counters = page->counters;
new.counters = counters;
- if (mode)
+ if (mode) {
new.inuse = page->objects;
+ new.freelist = NULL;
+ } else {
+ new.freelist = freelist;
+ }
VM_BUG_ON(new.frozen);
new.frozen = 1;
} while (!__cmpxchg_double_slab(s, page,
freelist, counters,
- NULL, new.counters,
+ new.freelist, new.counters,
"lock and freeze"));
remove_partial(n, page);
@@ -1564,7 +1568,6 @@ static void *get_partial_node(struct kmem_cache *s,
object = t;
available = page->objects - page->inuse;
} else {
- page->freelist = t;
available = put_cpu_partial(s, page, 0);
stat(s, CPU_PARTIAL_NODE);
}
--
1.7.9.5
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH] slub: fix a memory leak in get_partial_node()
2012-05-15 19:01 ` Joonsoo Kim
@ 2012-05-15 19:10 ` Greg Kroah-Hartman
-1 siblings, 0 replies; 21+ messages in thread
From: Greg Kroah-Hartman @ 2012-05-15 19:10 UTC (permalink / raw)
To: Joonsoo Kim
Cc: Pekka Enberg, Christoph Lameter, linux-kernel, linux-mm, stable
On Wed, May 16, 2012 at 04:01:38AM +0900, Joonsoo Kim wrote:
> In the case which is below,
>
> 1. acquire slab for cpu partial list
> 2. free object to it by remote cpu
> 3. page->freelist = t
>
> then memory leak is occurred.
>
> Change acquire_slab() not to zap freelist when it works for cpu partial list.
> I think it is a sufficient solution for fixing a memory leak.
>
> Below is output of 'slabinfo -r kmalloc-256'
> when './perf stat -r 30 hackbench 50 process 4000 > /dev/null' is done.
>
> ***Vanilla***
> Sizes (bytes) Slabs Debug Memory
> ------------------------------------------------------------------------
> Object : 256 Total : 468 Sanity Checks : Off Total: 3833856
> SlabObj: 256 Full : 111 Redzoning : Off Used : 2004992
> SlabSiz: 8192 Partial: 302 Poisoning : Off Loss : 1828864
> Loss : 0 CpuSlab: 55 Tracking : Off Lalig: 0
> Align : 8 Objects: 32 Tracing : Off Lpadd: 0
>
> ***Patched***
> Sizes (bytes) Slabs Debug Memory
> ------------------------------------------------------------------------
> Object : 256 Total : 300 Sanity Checks : Off Total: 2457600
> SlabObj: 256 Full : 204 Redzoning : Off Used : 2348800
> SlabSiz: 8192 Partial: 33 Poisoning : Off Loss : 108800
> Loss : 0 CpuSlab: 63 Tracking : Off Lalig: 0
> Align : 8 Objects: 32 Tracing : Off Lpadd: 0
>
> Total and loss number is the impact of this patch.
>
> Signed-off-by: Joonsoo Kim <js1304@gmail.com>
<formletter>
This is not the correct way to submit patches for inclusion in the
stable kernel tree. Please read Documentation/stable_kernel_rules.txt
for how to do this properly.
</formletter>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] slub: fix a memory leak in get_partial_node()
@ 2012-05-15 19:10 ` Greg Kroah-Hartman
0 siblings, 0 replies; 21+ messages in thread
From: Greg Kroah-Hartman @ 2012-05-15 19:10 UTC (permalink / raw)
To: Joonsoo Kim
Cc: Pekka Enberg, Christoph Lameter, linux-kernel, linux-mm, stable
On Wed, May 16, 2012 at 04:01:38AM +0900, Joonsoo Kim wrote:
> In the case which is below,
>
> 1. acquire slab for cpu partial list
> 2. free object to it by remote cpu
> 3. page->freelist = t
>
> then memory leak is occurred.
>
> Change acquire_slab() not to zap freelist when it works for cpu partial list.
> I think it is a sufficient solution for fixing a memory leak.
>
> Below is output of 'slabinfo -r kmalloc-256'
> when './perf stat -r 30 hackbench 50 process 4000 > /dev/null' is done.
>
> ***Vanilla***
> Sizes (bytes) Slabs Debug Memory
> ------------------------------------------------------------------------
> Object : 256 Total : 468 Sanity Checks : Off Total: 3833856
> SlabObj: 256 Full : 111 Redzoning : Off Used : 2004992
> SlabSiz: 8192 Partial: 302 Poisoning : Off Loss : 1828864
> Loss : 0 CpuSlab: 55 Tracking : Off Lalig: 0
> Align : 8 Objects: 32 Tracing : Off Lpadd: 0
>
> ***Patched***
> Sizes (bytes) Slabs Debug Memory
> ------------------------------------------------------------------------
> Object : 256 Total : 300 Sanity Checks : Off Total: 2457600
> SlabObj: 256 Full : 204 Redzoning : Off Used : 2348800
> SlabSiz: 8192 Partial: 33 Poisoning : Off Loss : 108800
> Loss : 0 CpuSlab: 63 Tracking : Off Lalig: 0
> Align : 8 Objects: 32 Tracing : Off Lpadd: 0
>
> Total and loss number is the impact of this patch.
>
> Signed-off-by: Joonsoo Kim <js1304@gmail.com>
<formletter>
This is not the correct way to submit patches for inclusion in the
stable kernel tree. Please read Documentation/stable_kernel_rules.txt
for how to do this properly.
</formletter>
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] slub: fix a memory leak in get_partial_node()
2012-05-15 19:01 ` Joonsoo Kim
@ 2012-05-15 20:36 ` Christoph Lameter
-1 siblings, 0 replies; 21+ messages in thread
From: Christoph Lameter @ 2012-05-15 20:36 UTC (permalink / raw)
To: Joonsoo Kim
Cc: Pekka Enberg, linux-kernel, linux-mm, Greg Kroah-Hartman, stable
On Wed, 16 May 2012, Joonsoo Kim wrote:
> In the case which is below,
>
> 1. acquire slab for cpu partial list
> 2. free object to it by remote cpu
> 3. page->freelist = t
>
> then memory leak is occurred.
Hmmm... Ok so we cannot assign page->freelist in get_partial_node() for
the cpu partial slabs. It must be done in the cmpxchg transition.
Acked-by: Christoph Lameter <cl@linux.com>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] slub: fix a memory leak in get_partial_node()
@ 2012-05-15 20:36 ` Christoph Lameter
0 siblings, 0 replies; 21+ messages in thread
From: Christoph Lameter @ 2012-05-15 20:36 UTC (permalink / raw)
To: Joonsoo Kim
Cc: Pekka Enberg, linux-kernel, linux-mm, Greg Kroah-Hartman, stable
On Wed, 16 May 2012, Joonsoo Kim wrote:
> In the case which is below,
>
> 1. acquire slab for cpu partial list
> 2. free object to it by remote cpu
> 3. page->freelist = t
>
> then memory leak is occurred.
Hmmm... Ok so we cannot assign page->freelist in get_partial_node() for
the cpu partial slabs. It must be done in the cmpxchg transition.
Acked-by: Christoph Lameter <cl@linux.com>
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] slub: fix a memory leak in get_partial_node()
2012-05-15 20:36 ` Christoph Lameter
@ 2012-05-16 6:35 ` Pekka Enberg
-1 siblings, 0 replies; 21+ messages in thread
From: Pekka Enberg @ 2012-05-16 6:35 UTC (permalink / raw)
To: Christoph Lameter
Cc: Joonsoo Kim, linux-kernel, linux-mm, Greg Kroah-Hartman, stable
<On Tue, 15 May 2012, Christoph Lameter wrote:
> On Wed, 16 May 2012, Joonsoo Kim wrote:
>
> > In the case which is below,
> >
> > 1. acquire slab for cpu partial list
> > 2. free object to it by remote cpu
> > 3. page->freelist = t
> >
> > then memory leak is occurred.
>
> Hmmm... Ok so we cannot assign page->freelist in get_partial_node() for
> the cpu partial slabs. It must be done in the cmpxchg transition.
>
> Acked-by: Christoph Lameter <cl@linux.com>
Joonsoo, can you please fix up the stable submission format, add
Christoph's ACK and resend?
Pekka
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] slub: fix a memory leak in get_partial_node()
@ 2012-05-16 6:35 ` Pekka Enberg
0 siblings, 0 replies; 21+ messages in thread
From: Pekka Enberg @ 2012-05-16 6:35 UTC (permalink / raw)
To: Christoph Lameter
Cc: Joonsoo Kim, linux-kernel, linux-mm, Greg Kroah-Hartman, stable
<On Tue, 15 May 2012, Christoph Lameter wrote:
> On Wed, 16 May 2012, Joonsoo Kim wrote:
>
> > In the case which is below,
> >
> > 1. acquire slab for cpu partial list
> > 2. free object to it by remote cpu
> > 3. page->freelist = t
> >
> > then memory leak is occurred.
>
> Hmmm... Ok so we cannot assign page->freelist in get_partial_node() for
> the cpu partial slabs. It must be done in the cmpxchg transition.
>
> Acked-by: Christoph Lameter <cl@linux.com>
Joonsoo, can you please fix up the stable submission format, add
Christoph's ACK and resend?
Pekka
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] slub: fix a memory leak in get_partial_node()
2012-05-16 6:35 ` Pekka Enberg
(?)
@ 2012-05-16 13:56 ` JoonSoo Kim
-1 siblings, 0 replies; 21+ messages in thread
From: JoonSoo Kim @ 2012-05-16 13:56 UTC (permalink / raw)
To: Pekka Enberg
Cc: Christoph Lameter, linux-kernel, linux-mm, Greg Kroah-Hartman, stable
2012/5/16 Pekka Enberg <penberg@kernel.org>:
> <On Tue, 15 May 2012, Christoph Lameter wrote:
>
>> On Wed, 16 May 2012, Joonsoo Kim wrote:
>>
>> > In the case which is below,
>> >
>> > 1. acquire slab for cpu partial list
>> > 2. free object to it by remote cpu
>> > 3. page->freelist = t
>> >
>> > then memory leak is occurred.
>>
>> Hmmm... Ok so we cannot assign page->freelist in get_partial_node() for
>> the cpu partial slabs. It must be done in the cmpxchg transition.
>>
>> Acked-by: Christoph Lameter <cl@linux.com>
>
> Joonsoo, can you please fix up the stable submission format, add
> Christoph's ACK and resend?
>
> Pekka
Thanks for comment.
I'm a kernel newbie,
so could you please tell me how to fix up the stable submission format?
I'm eager to fix it up, but I don't know how to.
I read stable_kernel_rules.txt, this article tells me I must note
upstream commit ID.
Above patch is not included in upstream currently, so I can't find
upstream commit ID.
Is 'Acked-by from MAINTAINER' sufficient for submitting to stable-kernel?
Is below format right for stable submission format?
To: Linus Torvalds <torvalds@linux-foundation.org>, Greg Kroah-Hartman
<gregkh@linuxfoundation.org>, Pekka Enberg <penberg@kernel.org>
CC: Christoph Lameter <cl@linux.com>, linux-kernel@vger.kernel.org,
linux-mm@kvack.org
[ Upstream commit xxxxxxxxxxxxxxxxxxx ]
Comment is here
Acked-by:
Signed-off-by:
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] slub: fix a memory leak in get_partial_node()
@ 2012-05-16 13:56 ` JoonSoo Kim
0 siblings, 0 replies; 21+ messages in thread
From: JoonSoo Kim @ 2012-05-16 13:56 UTC (permalink / raw)
To: Pekka Enberg
Cc: Christoph Lameter, linux-kernel, linux-mm, Greg Kroah-Hartman, stable
2012/5/16 Pekka Enberg <penberg@kernel.org>:
> <On Tue, 15 May 2012, Christoph Lameter wrote:
>
>> On Wed, 16 May 2012, Joonsoo Kim wrote:
>>
>> > In the case which is below,
>> >
>> > 1. acquire slab for cpu partial list
>> > 2. free object to it by remote cpu
>> > 3. page->freelist = t
>> >
>> > then memory leak is occurred.
>>
>> Hmmm... Ok so we cannot assign page->freelist in get_partial_node() for
>> the cpu partial slabs. It must be done in the cmpxchg transition.
>>
>> Acked-by: Christoph Lameter <cl@linux.com>
>
> Joonsoo, can you please fix up the stable submission format, add
> Christoph's ACK and resend?
>
> � � � � � � � � � � � �Pekka
Thanks for comment.
I'm a kernel newbie,
so could you please tell me how to fix up the stable submission format?
I'm eager to fix it up, but I don't know how to.
I read stable_kernel_rules.txt, this article tells me I must note
upstream commit ID.
Above patch is not included in upstream currently, so I can't find
upstream commit ID.
Is 'Acked-by from MAINTAINER' sufficient for submitting to stable-kernel?
Is below format right for stable submission format?
To: Linus Torvalds <torvalds@linux-foundation.org>, Greg Kroah-Hartman
<gregkh@linuxfoundation.org>, Pekka Enberg <penberg@kernel.org>
CC: Christoph Lameter <cl@linux.com>, linux-kernel@vger.kernel.org,
linux-mm@kvack.org
[ Upstream commit xxxxxxxxxxxxxxxxxxx ]
Comment is here
Acked-by:
Signed-off-by:
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] slub: fix a memory leak in get_partial_node()
@ 2012-05-16 13:56 ` JoonSoo Kim
0 siblings, 0 replies; 21+ messages in thread
From: JoonSoo Kim @ 2012-05-16 13:56 UTC (permalink / raw)
To: Pekka Enberg
Cc: Christoph Lameter, linux-kernel, linux-mm, Greg Kroah-Hartman, stable
2012/5/16 Pekka Enberg <penberg@kernel.org>:
> <On Tue, 15 May 2012, Christoph Lameter wrote:
>
>> On Wed, 16 May 2012, Joonsoo Kim wrote:
>>
>> > In the case which is below,
>> >
>> > 1. acquire slab for cpu partial list
>> > 2. free object to it by remote cpu
>> > 3. page->freelist = t
>> >
>> > then memory leak is occurred.
>>
>> Hmmm... Ok so we cannot assign page->freelist in get_partial_node() for
>> the cpu partial slabs. It must be done in the cmpxchg transition.
>>
>> Acked-by: Christoph Lameter <cl@linux.com>
>
> Joonsoo, can you please fix up the stable submission format, add
> Christoph's ACK and resend?
>
> Pekka
Thanks for comment.
I'm a kernel newbie,
so could you please tell me how to fix up the stable submission format?
I'm eager to fix it up, but I don't know how to.
I read stable_kernel_rules.txt, this article tells me I must note
upstream commit ID.
Above patch is not included in upstream currently, so I can't find
upstream commit ID.
Is 'Acked-by from MAINTAINER' sufficient for submitting to stable-kernel?
Is below format right for stable submission format?
To: Linus Torvalds <torvalds@linux-foundation.org>, Greg Kroah-Hartman
<gregkh@linuxfoundation.org>, Pekka Enberg <penberg@kernel.org>
CC: Christoph Lameter <cl@linux.com>, linux-kernel@vger.kernel.org,
linux-mm@kvack.org
[ Upstream commit xxxxxxxxxxxxxxxxxxx ]
Comment is here
Acked-by:
Signed-off-by:
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] slub: fix a memory leak in get_partial_node()
2012-05-16 13:56 ` JoonSoo Kim
(?)
@ 2012-05-16 14:50 ` Greg Kroah-Hartman
-1 siblings, 0 replies; 21+ messages in thread
From: Greg Kroah-Hartman @ 2012-05-16 14:50 UTC (permalink / raw)
To: JoonSoo Kim
Cc: Pekka Enberg, Christoph Lameter, linux-kernel, linux-mm, stable
On Wed, May 16, 2012 at 10:56:50PM +0900, JoonSoo Kim wrote:
> 2012/5/16 Pekka Enberg <penberg@kernel.org>:
> > <On Tue, 15 May 2012, Christoph Lameter wrote:
> >
> >> On Wed, 16 May 2012, Joonsoo Kim wrote:
> >>
> >> > In the case which is below,
> >> >
> >> > 1. acquire slab for cpu partial list
> >> > 2. free object to it by remote cpu
> >> > 3. page->freelist = t
> >> >
> >> > then memory leak is occurred.
> >>
> >> Hmmm... Ok so we cannot assign page->freelist in get_partial_node() for
> >> the cpu partial slabs. It must be done in the cmpxchg transition.
> >>
> >> Acked-by: Christoph Lameter <cl@linux.com>
> >
> > Joonsoo, can you please fix up the stable submission format, add
> > Christoph's ACK and resend?
> >
> > Pekka
>
> Thanks for comment.
> I'm a kernel newbie,
> so could you please tell me how to fix up the stable submission format?
> I'm eager to fix it up, but I don't know how to.
>
> I read stable_kernel_rules.txt, this article tells me I must note
> upstream commit ID.
> Above patch is not included in upstream currently, so I can't find
> upstream commit ID.
> Is 'Acked-by from MAINTAINER' sufficient for submitting to stable-kernel?
> Is below format right for stable submission format?
No.
Please read the second item in the list that says: "Procedure for
submitting patches to the -stable tree" in the file,
Documentation/stable_kernel_rulest.txt. It states:
- To have the patch automatically included in the stable tree, add the tag
Cc: stable@vger.kernel.org
in the sign-off area. Once the patch is merged it will be applied to
the stable tree without anything else needing to be done by the author
or subsystem maintainer.
Does that help?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] slub: fix a memory leak in get_partial_node()
@ 2012-05-16 14:50 ` Greg Kroah-Hartman
0 siblings, 0 replies; 21+ messages in thread
From: Greg Kroah-Hartman @ 2012-05-16 14:50 UTC (permalink / raw)
To: JoonSoo Kim
Cc: Pekka Enberg, Christoph Lameter, linux-kernel, linux-mm, stable
On Wed, May 16, 2012 at 10:56:50PM +0900, JoonSoo Kim wrote:
> 2012/5/16 Pekka Enberg <penberg@kernel.org>:
> > <On Tue, 15 May 2012, Christoph Lameter wrote:
> >
> >> On Wed, 16 May 2012, Joonsoo Kim wrote:
> >>
> >> > In the case which is below,
> >> >
> >> > 1. acquire slab for cpu partial list
> >> > 2. free object to it by remote cpu
> >> > 3. page->freelist = t
> >> >
> >> > then memory leak is occurred.
> >>
> >> Hmmm... Ok so we cannot assign page->freelist in get_partial_node() for
> >> the cpu partial slabs. It must be done in the cmpxchg transition.
> >>
> >> Acked-by: Christoph Lameter <cl@linux.com>
> >
> > Joonsoo, can you please fix up the stable submission format, add
> > Christoph's ACK and resend?
> >
> > � � � � � � � � � � � �Pekka
>
> Thanks for comment.
> I'm a kernel newbie,
> so could you please tell me how to fix up the stable submission format?
> I'm eager to fix it up, but I don't know how to.
>
> I read stable_kernel_rules.txt, this article tells me I must note
> upstream commit ID.
> Above patch is not included in upstream currently, so I can't find
> upstream commit ID.
> Is 'Acked-by from MAINTAINER' sufficient for submitting to stable-kernel?
> Is below format right for stable submission format?
No.
Please read the second item in the list that says: "Procedure for
submitting patches to the -stable tree" in the file,
Documentation/stable_kernel_rulest.txt. It states:
- To have the patch automatically included in the stable tree, add the tag
Cc: stable@vger.kernel.org
in the sign-off area. Once the patch is merged it will be applied to
the stable tree without anything else needing to be done by the author
or subsystem maintainer.
Does that help?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] slub: fix a memory leak in get_partial_node()
@ 2012-05-16 14:50 ` Greg Kroah-Hartman
0 siblings, 0 replies; 21+ messages in thread
From: Greg Kroah-Hartman @ 2012-05-16 14:50 UTC (permalink / raw)
To: JoonSoo Kim
Cc: Pekka Enberg, Christoph Lameter, linux-kernel, linux-mm, stable
On Wed, May 16, 2012 at 10:56:50PM +0900, JoonSoo Kim wrote:
> 2012/5/16 Pekka Enberg <penberg@kernel.org>:
> > <On Tue, 15 May 2012, Christoph Lameter wrote:
> >
> >> On Wed, 16 May 2012, Joonsoo Kim wrote:
> >>
> >> > In the case which is below,
> >> >
> >> > 1. acquire slab for cpu partial list
> >> > 2. free object to it by remote cpu
> >> > 3. page->freelist = t
> >> >
> >> > then memory leak is occurred.
> >>
> >> Hmmm... Ok so we cannot assign page->freelist in get_partial_node() for
> >> the cpu partial slabs. It must be done in the cmpxchg transition.
> >>
> >> Acked-by: Christoph Lameter <cl@linux.com>
> >
> > Joonsoo, can you please fix up the stable submission format, add
> > Christoph's ACK and resend?
> >
> > Pekka
>
> Thanks for comment.
> I'm a kernel newbie,
> so could you please tell me how to fix up the stable submission format?
> I'm eager to fix it up, but I don't know how to.
>
> I read stable_kernel_rules.txt, this article tells me I must note
> upstream commit ID.
> Above patch is not included in upstream currently, so I can't find
> upstream commit ID.
> Is 'Acked-by from MAINTAINER' sufficient for submitting to stable-kernel?
> Is below format right for stable submission format?
No.
Please read the second item in the list that says: "Procedure for
submitting patches to the -stable tree" in the file,
Documentation/stable_kernel_rulest.txt. It states:
- To have the patch automatically included in the stable tree, add the tag
Cc: stable@vger.kernel.org
in the sign-off area. Once the patch is merged it will be applied to
the stable tree without anything else needing to be done by the author
or subsystem maintainer.
Does that help?
thanks,
greg k-h
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH RESEND] slub: fix a memory leak in get_partial_node()
2012-05-16 6:35 ` Pekka Enberg
@ 2012-05-16 15:13 ` Joonsoo Kim
-1 siblings, 0 replies; 21+ messages in thread
From: Joonsoo Kim @ 2012-05-16 15:13 UTC (permalink / raw)
To: Pekka Enberg
Cc: Christoph Lameter, linux-kernel, linux-mm, Greg Kroah-Hartman,
Joonsoo Kim, stable
In the case which is below,
1. acquire slab for cpu partial list
2. free object to it by remote cpu
3. page->freelist = t
then memory leak is occurred.
Change acquire_slab() not to zap freelist when it works for cpu partial list.
I think it is a sufficient solution for fixing a memory leak.
Below is output of 'slabinfo -r kmalloc-256'
when './perf stat -r 30 hackbench 50 process 4000 > /dev/null' is done.
***Vanilla***
Sizes (bytes) Slabs Debug Memory
------------------------------------------------------------------------
Object : 256 Total : 468 Sanity Checks : Off Total: 3833856
SlabObj: 256 Full : 111 Redzoning : Off Used : 2004992
SlabSiz: 8192 Partial: 302 Poisoning : Off Loss : 1828864
Loss : 0 CpuSlab: 55 Tracking : Off Lalig: 0
Align : 8 Objects: 32 Tracing : Off Lpadd: 0
***Patched***
Sizes (bytes) Slabs Debug Memory
------------------------------------------------------------------------
Object : 256 Total : 300 Sanity Checks : Off Total: 2457600
SlabObj: 256 Full : 204 Redzoning : Off Used : 2348800
SlabSiz: 8192 Partial: 33 Poisoning : Off Loss : 108800
Loss : 0 CpuSlab: 63 Tracking : Off Lalig: 0
Align : 8 Objects: 32 Tracing : Off Lpadd: 0
Total and loss number is the impact of this patch.
Cc: <stable@vger.kernel.org>
Acked-by: Christoph Lameter <cl@linux.com>
Signed-off-by: Joonsoo Kim <js1304@gmail.com>
diff --git a/mm/slub.c b/mm/slub.c
index ffe13fd..a7a766a 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -1514,15 +1514,19 @@ static inline void *acquire_slab(struct kmem_cache *s,
freelist = page->freelist;
counters = page->counters;
new.counters = counters;
- if (mode)
+ if (mode) {
new.inuse = page->objects;
+ new.freelist = NULL;
+ } else {
+ new.freelist = freelist;
+ }
VM_BUG_ON(new.frozen);
new.frozen = 1;
} while (!__cmpxchg_double_slab(s, page,
freelist, counters,
- NULL, new.counters,
+ new.freelist, new.counters,
"lock and freeze"));
remove_partial(n, page);
@@ -1564,7 +1568,6 @@ static void *get_partial_node(struct kmem_cache *s,
object = t;
available = page->objects - page->inuse;
} else {
- page->freelist = t;
available = put_cpu_partial(s, page, 0);
stat(s, CPU_PARTIAL_NODE);
}
--
1.7.9.5
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH RESEND] slub: fix a memory leak in get_partial_node()
@ 2012-05-16 15:13 ` Joonsoo Kim
0 siblings, 0 replies; 21+ messages in thread
From: Joonsoo Kim @ 2012-05-16 15:13 UTC (permalink / raw)
To: Pekka Enberg
Cc: Christoph Lameter, linux-kernel, linux-mm, Greg Kroah-Hartman,
Joonsoo Kim, stable
In the case which is below,
1. acquire slab for cpu partial list
2. free object to it by remote cpu
3. page->freelist = t
then memory leak is occurred.
Change acquire_slab() not to zap freelist when it works for cpu partial list.
I think it is a sufficient solution for fixing a memory leak.
Below is output of 'slabinfo -r kmalloc-256'
when './perf stat -r 30 hackbench 50 process 4000 > /dev/null' is done.
***Vanilla***
Sizes (bytes) Slabs Debug Memory
------------------------------------------------------------------------
Object : 256 Total : 468 Sanity Checks : Off Total: 3833856
SlabObj: 256 Full : 111 Redzoning : Off Used : 2004992
SlabSiz: 8192 Partial: 302 Poisoning : Off Loss : 1828864
Loss : 0 CpuSlab: 55 Tracking : Off Lalig: 0
Align : 8 Objects: 32 Tracing : Off Lpadd: 0
***Patched***
Sizes (bytes) Slabs Debug Memory
------------------------------------------------------------------------
Object : 256 Total : 300 Sanity Checks : Off Total: 2457600
SlabObj: 256 Full : 204 Redzoning : Off Used : 2348800
SlabSiz: 8192 Partial: 33 Poisoning : Off Loss : 108800
Loss : 0 CpuSlab: 63 Tracking : Off Lalig: 0
Align : 8 Objects: 32 Tracing : Off Lpadd: 0
Total and loss number is the impact of this patch.
Cc: <stable@vger.kernel.org>
Acked-by: Christoph Lameter <cl@linux.com>
Signed-off-by: Joonsoo Kim <js1304@gmail.com>
diff --git a/mm/slub.c b/mm/slub.c
index ffe13fd..a7a766a 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -1514,15 +1514,19 @@ static inline void *acquire_slab(struct kmem_cache *s,
freelist = page->freelist;
counters = page->counters;
new.counters = counters;
- if (mode)
+ if (mode) {
new.inuse = page->objects;
+ new.freelist = NULL;
+ } else {
+ new.freelist = freelist;
+ }
VM_BUG_ON(new.frozen);
new.frozen = 1;
} while (!__cmpxchg_double_slab(s, page,
freelist, counters,
- NULL, new.counters,
+ new.freelist, new.counters,
"lock and freeze"));
remove_partial(n, page);
@@ -1564,7 +1568,6 @@ static void *get_partial_node(struct kmem_cache *s,
object = t;
available = page->objects - page->inuse;
} else {
- page->freelist = t;
available = put_cpu_partial(s, page, 0);
stat(s, CPU_PARTIAL_NODE);
}
--
1.7.9.5
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH] slub: fix a memory leak in get_partial_node()
2012-05-16 14:50 ` Greg Kroah-Hartman
(?)
@ 2012-05-16 15:17 ` JoonSoo Kim
-1 siblings, 0 replies; 21+ messages in thread
From: JoonSoo Kim @ 2012-05-16 15:17 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Pekka Enberg, Christoph Lameter, linux-kernel, linux-mm, stable
2012/5/16 Greg Kroah-Hartman <gregkh@linuxfoundation.org>:
>> I read stable_kernel_rules.txt, this article tells me I must note
>> upstream commit ID.
>> Above patch is not included in upstream currently, so I can't find
>> upstream commit ID.
>> Is 'Acked-by from MAINTAINER' sufficient for submitting to stable-kernel?
>> Is below format right for stable submission format?
>
> No.
>
> Please read the second item in the list that says: "Procedure for
> submitting patches to the -stable tree" in the file,
> Documentation/stable_kernel_rulest.txt. It states:
>
> - To have the patch automatically included in the stable tree, add the tag
> Cc: stable@vger.kernel.org
> in the sign-off area. Once the patch is merged it will be applied to
> the stable tree without anything else needing to be done by the author
> or subsystem maintainer.
>
> Does that help?
>
> thanks,
>
> greg k-h
Thanks, very helpful.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] slub: fix a memory leak in get_partial_node()
@ 2012-05-16 15:17 ` JoonSoo Kim
0 siblings, 0 replies; 21+ messages in thread
From: JoonSoo Kim @ 2012-05-16 15:17 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Pekka Enberg, Christoph Lameter, linux-kernel, linux-mm, stable
2012/5/16 Greg Kroah-Hartman <gregkh@linuxfoundation.org>:
>> I read stable_kernel_rules.txt, this article tells me I must note
>> upstream commit ID.
>> Above patch is not included in upstream currently, so I can't find
>> upstream commit ID.
>> Is 'Acked-by from MAINTAINER' sufficient for submitting to stable-kernel?
>> Is below format right for stable submission format?
>
> No.
>
> Please read the second item in the list that says: "Procedure for
> submitting patches to the -stable tree" in the file,
> Documentation/stable_kernel_rulest.txt. �It states:
>
> �- To have the patch automatically included in the stable tree, add the tag
> � � Cc: stable@vger.kernel.org
> � in the sign-off area. Once the patch is merged it will be applied to
> � the stable tree without anything else needing to be done by the author
> � or subsystem maintainer.
>
> Does that help?
>
> thanks,
>
> greg k-h
Thanks, very helpful.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] slub: fix a memory leak in get_partial_node()
@ 2012-05-16 15:17 ` JoonSoo Kim
0 siblings, 0 replies; 21+ messages in thread
From: JoonSoo Kim @ 2012-05-16 15:17 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Pekka Enberg, Christoph Lameter, linux-kernel, linux-mm, stable
2012/5/16 Greg Kroah-Hartman <gregkh@linuxfoundation.org>:
>> I read stable_kernel_rules.txt, this article tells me I must note
>> upstream commit ID.
>> Above patch is not included in upstream currently, so I can't find
>> upstream commit ID.
>> Is 'Acked-by from MAINTAINER' sufficient for submitting to stable-kernel?
>> Is below format right for stable submission format?
>
> No.
>
> Please read the second item in the list that says: "Procedure for
> submitting patches to the -stable tree" in the file,
> Documentation/stable_kernel_rulest.txt. It states:
>
> - To have the patch automatically included in the stable tree, add the tag
> Cc: stable@vger.kernel.org
> in the sign-off area. Once the patch is merged it will be applied to
> the stable tree without anything else needing to be done by the author
> or subsystem maintainer.
>
> Does that help?
>
> thanks,
>
> greg k-h
Thanks, very helpful.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH RESEND] slub: fix a memory leak in get_partial_node()
2012-05-16 15:13 ` Joonsoo Kim
@ 2012-05-18 9:25 ` Pekka Enberg
-1 siblings, 0 replies; 21+ messages in thread
From: Pekka Enberg @ 2012-05-18 9:25 UTC (permalink / raw)
To: Joonsoo Kim
Cc: Christoph Lameter, linux-kernel, linux-mm, Greg Kroah-Hartman, stable
On Thu, 17 May 2012, Joonsoo Kim wrote:
> In the case which is below,
>
> 1. acquire slab for cpu partial list
> 2. free object to it by remote cpu
> 3. page->freelist = t
>
> then memory leak is occurred.
>
> Change acquire_slab() not to zap freelist when it works for cpu partial list.
> I think it is a sufficient solution for fixing a memory leak.
>
> Below is output of 'slabinfo -r kmalloc-256'
> when './perf stat -r 30 hackbench 50 process 4000 > /dev/null' is done.
>
> ***Vanilla***
> Sizes (bytes) Slabs Debug Memory
> ------------------------------------------------------------------------
> Object : 256 Total : 468 Sanity Checks : Off Total: 3833856
> SlabObj: 256 Full : 111 Redzoning : Off Used : 2004992
> SlabSiz: 8192 Partial: 302 Poisoning : Off Loss : 1828864
> Loss : 0 CpuSlab: 55 Tracking : Off Lalig: 0
> Align : 8 Objects: 32 Tracing : Off Lpadd: 0
>
> ***Patched***
> Sizes (bytes) Slabs Debug Memory
> ------------------------------------------------------------------------
> Object : 256 Total : 300 Sanity Checks : Off Total: 2457600
> SlabObj: 256 Full : 204 Redzoning : Off Used : 2348800
> SlabSiz: 8192 Partial: 33 Poisoning : Off Loss : 108800
> Loss : 0 CpuSlab: 63 Tracking : Off Lalig: 0
> Align : 8 Objects: 32 Tracing : Off Lpadd: 0
>
> Total and loss number is the impact of this patch.
>
> Cc: <stable@vger.kernel.org>
> Acked-by: Christoph Lameter <cl@linux.com>
> Signed-off-by: Joonsoo Kim <js1304@gmail.com>
Applied, thanks!
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH RESEND] slub: fix a memory leak in get_partial_node()
@ 2012-05-18 9:25 ` Pekka Enberg
0 siblings, 0 replies; 21+ messages in thread
From: Pekka Enberg @ 2012-05-18 9:25 UTC (permalink / raw)
To: Joonsoo Kim
Cc: Christoph Lameter, linux-kernel, linux-mm, Greg Kroah-Hartman, stable
On Thu, 17 May 2012, Joonsoo Kim wrote:
> In the case which is below,
>
> 1. acquire slab for cpu partial list
> 2. free object to it by remote cpu
> 3. page->freelist = t
>
> then memory leak is occurred.
>
> Change acquire_slab() not to zap freelist when it works for cpu partial list.
> I think it is a sufficient solution for fixing a memory leak.
>
> Below is output of 'slabinfo -r kmalloc-256'
> when './perf stat -r 30 hackbench 50 process 4000 > /dev/null' is done.
>
> ***Vanilla***
> Sizes (bytes) Slabs Debug Memory
> ------------------------------------------------------------------------
> Object : 256 Total : 468 Sanity Checks : Off Total: 3833856
> SlabObj: 256 Full : 111 Redzoning : Off Used : 2004992
> SlabSiz: 8192 Partial: 302 Poisoning : Off Loss : 1828864
> Loss : 0 CpuSlab: 55 Tracking : Off Lalig: 0
> Align : 8 Objects: 32 Tracing : Off Lpadd: 0
>
> ***Patched***
> Sizes (bytes) Slabs Debug Memory
> ------------------------------------------------------------------------
> Object : 256 Total : 300 Sanity Checks : Off Total: 2457600
> SlabObj: 256 Full : 204 Redzoning : Off Used : 2348800
> SlabSiz: 8192 Partial: 33 Poisoning : Off Loss : 108800
> Loss : 0 CpuSlab: 63 Tracking : Off Lalig: 0
> Align : 8 Objects: 32 Tracing : Off Lpadd: 0
>
> Total and loss number is the impact of this patch.
>
> Cc: <stable@vger.kernel.org>
> Acked-by: Christoph Lameter <cl@linux.com>
> Signed-off-by: Joonsoo Kim <js1304@gmail.com>
Applied, thanks!
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2012-05-18 9:25 UTC | newest]
Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-05-15 19:01 [PATCH] slub: fix a memory leak in get_partial_node() Joonsoo Kim
2012-05-15 19:01 ` Joonsoo Kim
2012-05-15 19:10 ` Greg Kroah-Hartman
2012-05-15 19:10 ` Greg Kroah-Hartman
2012-05-15 20:36 ` Christoph Lameter
2012-05-15 20:36 ` Christoph Lameter
2012-05-16 6:35 ` Pekka Enberg
2012-05-16 6:35 ` Pekka Enberg
2012-05-16 13:56 ` JoonSoo Kim
2012-05-16 13:56 ` JoonSoo Kim
2012-05-16 13:56 ` JoonSoo Kim
2012-05-16 14:50 ` Greg Kroah-Hartman
2012-05-16 14:50 ` Greg Kroah-Hartman
2012-05-16 14:50 ` Greg Kroah-Hartman
2012-05-16 15:17 ` JoonSoo Kim
2012-05-16 15:17 ` JoonSoo Kim
2012-05-16 15:17 ` JoonSoo Kim
2012-05-16 15:13 ` [PATCH RESEND] " Joonsoo Kim
2012-05-16 15:13 ` Joonsoo Kim
2012-05-18 9:25 ` Pekka Enberg
2012-05-18 9:25 ` Pekka Enberg
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.