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 <[email protected]>
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
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 <[email protected]>
<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>
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 <[email protected]>
<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 <[email protected]>
Joonsoo, can you please fix up the stable submission format, add
Christoph's ACK and resend?
Pekka
2012/5/16 Pekka Enberg <[email protected]>:
> <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 <[email protected]>
>
> 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 <[email protected]>, Greg Kroah-Hartman
<[email protected]>, Pekka Enberg <[email protected]>
CC: Christoph Lameter <[email protected]>, [email protected],
[email protected]
[ Upstream commit xxxxxxxxxxxxxxxxxxx ]
Comment is here
Acked-by:
Signed-off-by:
On Wed, May 16, 2012 at 10:56:50PM +0900, JoonSoo Kim wrote:
> 2012/5/16 Pekka Enberg <[email protected]>:
> > <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 <[email protected]>
> >
> > 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: [email protected]
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
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: <[email protected]>
Acked-by: Christoph Lameter <[email protected]>
Signed-off-by: Joonsoo Kim <[email protected]>
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
2012/5/16 Greg Kroah-Hartman <[email protected]>:
>> 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: [email protected]
> ? 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.
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: <[email protected]>
> Acked-by: Christoph Lameter <[email protected]>
> Signed-off-by: Joonsoo Kim <[email protected]>
Applied, thanks!