linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
* [patch] slob: fix gfp flags for order-0 page allocations
@ 2010-08-22 23:16 David Rientjes
  2010-08-23  0:51 ` Christoph Lameter
  2010-08-24  4:26 ` Matt Mackall
  0 siblings, 2 replies; 8+ messages in thread
From: David Rientjes @ 2010-08-22 23:16 UTC (permalink / raw)
  To: Pekka Enberg; +Cc: Matt Mackall, Christoph Lameter, linux-mm

kmalloc_node() may allocate higher order slob pages, but the __GFP_COMP
bit is only passed to the page allocator and not represented in the
tracepoint event.  The bit should be passed to trace_kmalloc_node() as
well.

Signed-off-by: David Rientjes <rientjes@google.com>
---
 mm/slob.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/mm/slob.c b/mm/slob.c
--- a/mm/slob.c
+++ b/mm/slob.c
@@ -500,7 +500,9 @@ void *__kmalloc_node(size_t size, gfp_t gfp, int node)
 	} else {
 		unsigned int order = get_order(size);
 
-		ret = slob_new_pages(gfp | __GFP_COMP, get_order(size), node);
+		if (likely(order))
+			gfp |= __GFP_COMP;
+		ret = slob_new_pages(gfp, order, node);
 		if (ret) {
 			struct page *page;
 			page = virt_to_page(ret);

--
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [patch] slob: fix gfp flags for order-0 page allocations
  2010-08-22 23:16 [patch] slob: fix gfp flags for order-0 page allocations David Rientjes
@ 2010-08-23  0:51 ` Christoph Lameter
  2010-08-24  4:26 ` Matt Mackall
  1 sibling, 0 replies; 8+ messages in thread
From: Christoph Lameter @ 2010-08-23  0:51 UTC (permalink / raw)
  To: David Rientjes; +Cc: Pekka Enberg, Matt Mackall, linux-mm

> diff --git a/mm/slob.c b/mm/slob.c
> --- a/mm/slob.c
> +++ b/mm/slob.c
> @@ -500,7 +500,9 @@ void *__kmalloc_node(size_t size, gfp_t gfp, int node)
>  	} else {
>  		unsigned int order = get_order(size);
>
> -		ret = slob_new_pages(gfp | __GFP_COMP, get_order(size), node);
> +		if (likely(order))
> +			gfp |= __GFP_COMP;
> +		ret = slob_new_pages(gfp, order, node);
>  		if (ret) {
>  			struct page *page;

Also gets rid of the double get_order().

Reviewed-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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [patch] slob: fix gfp flags for order-0 page allocations
  2010-08-22 23:16 [patch] slob: fix gfp flags for order-0 page allocations David Rientjes
  2010-08-23  0:51 ` Christoph Lameter
@ 2010-08-24  4:26 ` Matt Mackall
  2010-08-24  4:36   ` David Rientjes
  1 sibling, 1 reply; 8+ messages in thread
From: Matt Mackall @ 2010-08-24  4:26 UTC (permalink / raw)
  To: David Rientjes; +Cc: Pekka Enberg, Christoph Lameter, linux-mm

On Sun, 2010-08-22 at 16:16 -0700, David Rientjes wrote:
> kmalloc_node() may allocate higher order slob pages, but the __GFP_COMP
> bit is only passed to the page allocator and not represented in the
> tracepoint event.  The bit should be passed to trace_kmalloc_node() as
> well.
> 
> Signed-off-by: David Rientjes <rientjes@google.com>

>  		unsigned int order = get_order(size);
>  
> -		ret = slob_new_pages(gfp | __GFP_COMP, get_order(size), node);
> +		if (likely(order))
> +			gfp |= __GFP_COMP;

Why is it likely? I would hope that the majority of page allocations are
in fact order 0.

-- 
Mathematics is the supreme nostalgia of our time.


--
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [patch] slob: fix gfp flags for order-0 page allocations
  2010-08-24  4:26 ` Matt Mackall
@ 2010-08-24  4:36   ` David Rientjes
  2010-08-24 15:20     ` Matt Mackall
  0 siblings, 1 reply; 8+ messages in thread
From: David Rientjes @ 2010-08-24  4:36 UTC (permalink / raw)
  To: Matt Mackall; +Cc: Pekka Enberg, Christoph Lameter, linux-mm

On Mon, 23 Aug 2010, Matt Mackall wrote:

> > kmalloc_node() may allocate higher order slob pages, but the __GFP_COMP
> > bit is only passed to the page allocator and not represented in the
> > tracepoint event.  The bit should be passed to trace_kmalloc_node() as
> > well.
> > 
> > Signed-off-by: David Rientjes <rientjes@google.com>
> 
> >  		unsigned int order = get_order(size);
> >  
> > -		ret = slob_new_pages(gfp | __GFP_COMP, get_order(size), node);
> > +		if (likely(order))
> > +			gfp |= __GFP_COMP;
> 
> Why is it likely? I would hope that the majority of page allocations are
> in fact order 0.
> 

This code only executes when size >= PAGE_SIZE + align, so I would assume 
that the vast majority of times this is actually higher order allocs 
(which is probably why __GFP_COMP was implicitly added to the gfpmask in 
the first place).  Is there evidence to show otherwise?

--
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [patch] slob: fix gfp flags for order-0 page allocations
  2010-08-24  4:36   ` David Rientjes
@ 2010-08-24 15:20     ` Matt Mackall
  2010-08-24 15:37       ` Christoph Lameter
  2010-08-24 17:44       ` Pekka Enberg
  0 siblings, 2 replies; 8+ messages in thread
From: Matt Mackall @ 2010-08-24 15:20 UTC (permalink / raw)
  To: David Rientjes; +Cc: Pekka Enberg, Christoph Lameter, linux-mm

On Mon, 2010-08-23 at 21:36 -0700, David Rientjes wrote:
> On Mon, 23 Aug 2010, Matt Mackall wrote:
> 
> > > kmalloc_node() may allocate higher order slob pages, but the __GFP_COMP
> > > bit is only passed to the page allocator and not represented in the
> > > tracepoint event.  The bit should be passed to trace_kmalloc_node() as
> > > well.
> > > 
> > > Signed-off-by: David Rientjes <rientjes@google.com>
> > 
> > >  		unsigned int order = get_order(size);
> > >  
> > > -		ret = slob_new_pages(gfp | __GFP_COMP, get_order(size), node);
> > > +		if (likely(order))
> > > +			gfp |= __GFP_COMP;
> > 
> > Why is it likely? I would hope that the majority of page allocations are
> > in fact order 0.
> > 
> 
> This code only executes when size >= PAGE_SIZE + align, so I would assume 
> that the vast majority of times this is actually higher order allocs 
> (which is probably why __GFP_COMP was implicitly added to the gfpmask in 
> the first place).  Is there evidence to show otherwise?

(peeks at code)

Ok, that + should be a -. But yes, you're right, the bucket around an
order-0 allocation is quite small.

Acked-by: Matt Mackall <mpm@selenic.com>


By the way, has anyone seen anything like this leak reported?

/proc/slabinfo:

kmalloc-32        1113344 1113344     32  128    1 : tunables    0    0
0 : slabdata   8698   8698      0

That's /proc/slabinfo on my laptop with SLUB. It looks like my last
reboot popped me back to 2.6.33 so it may also be old news, but I
couldn't spot any reports with Google.

-- 
Mathematics is the supreme nostalgia of our time.


--
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [patch] slob: fix gfp flags for order-0 page allocations
  2010-08-24 15:20     ` Matt Mackall
@ 2010-08-24 15:37       ` Christoph Lameter
  2010-08-24 20:24         ` David Rientjes
  2010-08-24 17:44       ` Pekka Enberg
  1 sibling, 1 reply; 8+ messages in thread
From: Christoph Lameter @ 2010-08-24 15:37 UTC (permalink / raw)
  To: Matt Mackall; +Cc: David Rientjes, Pekka Enberg, linux-mm

On Tue, 24 Aug 2010, Matt Mackall wrote:

> kmalloc-32        1113344 1113344     32  128    1 : tunables    0    0
> 0 : slabdata   8698   8698      0
>
> That's /proc/slabinfo on my laptop with SLUB. It looks like my last
> reboot popped me back to 2.6.33 so it may also be old news, but I
> couldn't spot any reports with Google.

Boot with "slub_debug" as a kernel parameter

and then do a

cat /sys/kernel/slab/kmalloc-32/alloc_calls

to find the caller allocating the objets.

--
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [patch] slob: fix gfp flags for order-0 page allocations
  2010-08-24 15:20     ` Matt Mackall
  2010-08-24 15:37       ` Christoph Lameter
@ 2010-08-24 17:44       ` Pekka Enberg
  1 sibling, 0 replies; 8+ messages in thread
From: Pekka Enberg @ 2010-08-24 17:44 UTC (permalink / raw)
  To: Matt Mackall; +Cc: David Rientjes, Pekka Enberg, Christoph Lameter, linux-mm

On Tue, 24 Aug 2010, Matt Mackall wrote:
> (peeks at code)
>
> Ok, that + should be a -. But yes, you're right, the bucket around an
> order-0 allocation is quite small.
>
> Acked-by: Matt Mackall <mpm@selenic.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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [patch] slob: fix gfp flags for order-0 page allocations
  2010-08-24 15:37       ` Christoph Lameter
@ 2010-08-24 20:24         ` David Rientjes
  0 siblings, 0 replies; 8+ messages in thread
From: David Rientjes @ 2010-08-24 20:24 UTC (permalink / raw)
  To: Christoph Lameter; +Cc: Matt Mackall, Pekka Enberg, linux-mm

On Tue, 24 Aug 2010, Christoph Lameter wrote:

> > kmalloc-32        1113344 1113344     32  128    1 : tunables    0    0
> > 0 : slabdata   8698   8698      0
> >
> > That's /proc/slabinfo on my laptop with SLUB. It looks like my last
> > reboot popped me back to 2.6.33 so it may also be old news, but I
> > couldn't spot any reports with Google.
> 
> Boot with "slub_debug" as a kernel parameter
> 
> and then do a
> 
> cat /sys/kernel/slab/kmalloc-32/alloc_calls
> 
> to find the caller allocating the objets.
> 

I'd suspect this was anon_vma, and enabling CONFIG_DEBUG_KMEMLEAK would 
probably reveal exactly where it's getting leaked.

--
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2010-08-24 20:21 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-08-22 23:16 [patch] slob: fix gfp flags for order-0 page allocations David Rientjes
2010-08-23  0:51 ` Christoph Lameter
2010-08-24  4:26 ` Matt Mackall
2010-08-24  4:36   ` David Rientjes
2010-08-24 15:20     ` Matt Mackall
2010-08-24 15:37       ` Christoph Lameter
2010-08-24 20:24         ` David Rientjes
2010-08-24 17:44       ` Pekka Enberg

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox