linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
* [RFC][PATCH] slab: fix caller tracking on CONFIG_OPTIMIZE_INLINING.
@ 2011-09-24  3:08 Tetsuo Handa
  2011-09-24 23:00 ` David Rientjes
  2011-09-26  6:17 ` [RFC][PATCH] slab: fix caller tracking on CONFIG_OPTIMIZE_INLINING Pekka Enberg
  0 siblings, 2 replies; 5+ messages in thread
From: Tetsuo Handa @ 2011-09-24  3:08 UTC (permalink / raw)
  To: cl, penberg, mpm, vegard.nossum, dmonakhov, catalin.marinas,
	rientjes, dfeng
  Cc: linux-mm

If CONFIG_OPTIMIZE_INLINING=y, /proc/slab_allocators shows entries like

  size-512: 5 kzalloc+0xb/0x10
  size-256: 31 kzalloc+0xb/0x10

which are useless for debugging.
Use "__always_inline" rather than "inline" in order to make
/proc/slab_allocators show caller of kzalloc() if caller tracking is enabled.

Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
----------
diff --git a/include/linux/slab.h b/include/linux/slab.h
index 573c809..2b745c0 100644
--- a/include/linux/slab.h
+++ b/include/linux/slab.h
@@ -188,6 +188,18 @@ size_t ksize(const void *);
 #else
 #include <linux/slab_def.h>
 #endif
+/*
+ * /proc/slab_allocator needs _RET_IP_ value. If CONFIG_OPTIMIZE_INLINING=y,
+ * use of "inline" causes compilers to pass address of kzalloc() etc. rather
+ * than address of caller. Thus, use "__always_inline" if _RET_IP_ value is
+ * needed.
+ */
+#if defined(CONFIG_DEBUG_SLAB) || defined(CONFIG_SLUB) || \
+	(defined(CONFIG_SLAB) && defined(CONFIG_TRACING))
+#define slabtrace_inline __always_inline
+#else
+#define slabtrace_inline inline
+#endif
 
 /**
  * kcalloc - allocate memory for an array. The memory is set to zero.
@@ -240,7 +252,7 @@ size_t ksize(const void *);
  * for general use, and so are not documented here. For a full list of
  * potential flags, always refer to linux/gfp.h.
  */
-static inline void *kcalloc(size_t n, size_t size, gfp_t flags)
+static slabtrace_inline void *kcalloc(size_t n, size_t size, gfp_t flags)
 {
 	if (size != 0 && n > ULONG_MAX / size)
 		return NULL;
@@ -258,19 +270,19 @@ static inline void *kcalloc(size_t n, size_t size, gfp_t flags)
  * if available. Equivalent to kmalloc() in the non-NUMA single-node
  * case.
  */
-static inline void *kmalloc_node(size_t size, gfp_t flags, int node)
+static slabtrace_inline void *kmalloc_node(size_t size, gfp_t flags, int node)
 {
 	return kmalloc(size, flags);
 }
 
-static inline void *__kmalloc_node(size_t size, gfp_t flags, int node)
+static slabtrace_inline void *__kmalloc_node(size_t size, gfp_t flags, int node)
 {
 	return __kmalloc(size, flags);
 }
 
 void *kmem_cache_alloc(struct kmem_cache *, gfp_t);
 
-static inline void *kmem_cache_alloc_node(struct kmem_cache *cachep,
+static slabtrace_inline void *kmem_cache_alloc_node(struct kmem_cache *cachep,
 					gfp_t flags, int node)
 {
 	return kmem_cache_alloc(cachep, flags);
@@ -325,7 +337,7 @@ extern void *__kmalloc_node_track_caller(size_t, gfp_t, int, unsigned long);
 /*
  * Shortcuts
  */
-static inline void *kmem_cache_zalloc(struct kmem_cache *k, gfp_t flags)
+static slabtrace_inline void *kmem_cache_zalloc(struct kmem_cache *k, gfp_t flags)
 {
 	return kmem_cache_alloc(k, flags | __GFP_ZERO);
 }
@@ -335,7 +347,7 @@ static inline void *kmem_cache_zalloc(struct kmem_cache *k, gfp_t flags)
  * @size: how many bytes of memory are required.
  * @flags: the type of memory to allocate (see kmalloc).
  */
-static inline void *kzalloc(size_t size, gfp_t flags)
+static slabtrace_inline void *kzalloc(size_t size, gfp_t flags)
 {
 	return kmalloc(size, flags | __GFP_ZERO);
 }
@@ -346,7 +358,7 @@ static inline void *kzalloc(size_t size, gfp_t flags)
  * @flags: the type of memory to allocate (see kmalloc).
  * @node: memory node from which to allocate
  */
-static inline void *kzalloc_node(size_t size, gfp_t flags, int node)
+static slabtrace_inline void *kzalloc_node(size_t size, gfp_t flags, int node)
 {
 	return kmalloc_node(size, flags | __GFP_ZERO, node);
 }

--
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] 5+ messages in thread

* Re: [RFC][PATCH] slab: fix caller tracking on CONFIG_OPTIMIZE_INLINING.
  2011-09-24  3:08 [RFC][PATCH] slab: fix caller tracking on CONFIG_OPTIMIZE_INLINING Tetsuo Handa
@ 2011-09-24 23:00 ` David Rientjes
  2011-09-25  5:21   ` [RFC][PATCH] slab: fix caller tracking onCONFIG_OPTIMIZE_INLINING Tetsuo Handa
  2011-09-26  6:17 ` [RFC][PATCH] slab: fix caller tracking on CONFIG_OPTIMIZE_INLINING Pekka Enberg
  1 sibling, 1 reply; 5+ messages in thread
From: David Rientjes @ 2011-09-24 23:00 UTC (permalink / raw)
  To: Tetsuo Handa
  Cc: cl, penberg, mpm, vegard.nossum, dmonakhov, catalin.marinas,
	dfeng, linux-mm

On Sat, 24 Sep 2011, Tetsuo Handa wrote:

> If CONFIG_OPTIMIZE_INLINING=y, /proc/slab_allocators shows entries like
> 
>   size-512: 5 kzalloc+0xb/0x10
>   size-256: 31 kzalloc+0xb/0x10
> 
> which are useless for debugging.
> Use "__always_inline" rather than "inline" in order to make
> /proc/slab_allocators show caller of kzalloc() if caller tracking is enabled.
> 

This is only an issue for gcc 4.x compilers, correct?

> Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> ----------
> diff --git a/include/linux/slab.h b/include/linux/slab.h
> index 573c809..2b745c0 100644
> --- a/include/linux/slab.h
> +++ b/include/linux/slab.h
> @@ -188,6 +188,18 @@ size_t ksize(const void *);
>  #else
>  #include <linux/slab_def.h>
>  #endif
> +/*
> + * /proc/slab_allocator needs _RET_IP_ value. If CONFIG_OPTIMIZE_INLINING=y,
> + * use of "inline" causes compilers to pass address of kzalloc() etc. rather
> + * than address of caller. Thus, use "__always_inline" if _RET_IP_ value is
> + * needed.
> + */
> +#if defined(CONFIG_DEBUG_SLAB) || defined(CONFIG_SLUB) || \
> +	(defined(CONFIG_SLAB) && defined(CONFIG_TRACING))

/proc/slab_allocators is for CONFIG_SLAB only, so why is this being done 
for slub when it already uses trace points with _RET_IP_?

> +#define slabtrace_inline __always_inline
> +#else
> +#define slabtrace_inline inline
> +#endif
>  
>  /**
>   * kcalloc - allocate memory for an array. The memory is set to zero.
> @@ -240,7 +252,7 @@ size_t ksize(const void *);
>   * for general use, and so are not documented here. For a full list of
>   * potential flags, always refer to linux/gfp.h.
>   */
> -static inline void *kcalloc(size_t n, size_t size, gfp_t flags)
> +static slabtrace_inline void *kcalloc(size_t n, size_t size, gfp_t flags)
>  {
>  	if (size != 0 && n > ULONG_MAX / size)
>  		return NULL;
> @@ -258,19 +270,19 @@ static inline void *kcalloc(size_t n, size_t size, gfp_t flags)
>   * if available. Equivalent to kmalloc() in the non-NUMA single-node
>   * case.
>   */
> -static inline void *kmalloc_node(size_t size, gfp_t flags, int node)
> +static slabtrace_inline void *kmalloc_node(size_t size, gfp_t flags, int node)
>  {
>  	return kmalloc(size, flags);
>  }
>  
> -static inline void *__kmalloc_node(size_t size, gfp_t flags, int node)
> +static slabtrace_inline void *__kmalloc_node(size_t size, gfp_t flags, int node)
>  {
>  	return __kmalloc(size, flags);
>  }
>  
>  void *kmem_cache_alloc(struct kmem_cache *, gfp_t);
>  
> -static inline void *kmem_cache_alloc_node(struct kmem_cache *cachep,
> +static slabtrace_inline void *kmem_cache_alloc_node(struct kmem_cache *cachep,
>  					gfp_t flags, int node)
>  {
>  	return kmem_cache_alloc(cachep, flags);
> @@ -325,7 +337,7 @@ extern void *__kmalloc_node_track_caller(size_t, gfp_t, int, unsigned long);
>  /*
>   * Shortcuts
>   */
> -static inline void *kmem_cache_zalloc(struct kmem_cache *k, gfp_t flags)
> +static slabtrace_inline void *kmem_cache_zalloc(struct kmem_cache *k, gfp_t flags)
>  {
>  	return kmem_cache_alloc(k, flags | __GFP_ZERO);
>  }
> @@ -335,7 +347,7 @@ static inline void *kmem_cache_zalloc(struct kmem_cache *k, gfp_t flags)
>   * @size: how many bytes of memory are required.
>   * @flags: the type of memory to allocate (see kmalloc).
>   */
> -static inline void *kzalloc(size_t size, gfp_t flags)
> +static slabtrace_inline void *kzalloc(size_t size, gfp_t flags)
>  {
>  	return kmalloc(size, flags | __GFP_ZERO);
>  }
> @@ -346,7 +358,7 @@ static inline void *kzalloc(size_t size, gfp_t flags)
>   * @flags: the type of memory to allocate (see kmalloc).
>   * @node: memory node from which to allocate
>   */
> -static inline void *kzalloc_node(size_t size, gfp_t flags, int node)
> +static slabtrace_inline void *kzalloc_node(size_t size, gfp_t flags, int node)
>  {
>  	return kmalloc_node(size, flags | __GFP_ZERO, node);
>  }
> 

So this is going against the inlining algorithms in gcc 4.x which will 
make the kernel image significantly larger even though there seems to be 
no benefit unless you have CONFIG_DEBUG_SLAB_LEAK, although this patch 
changes behavior for every system running CONFIG_SLAB with tracing 
support.

--
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] 5+ messages in thread

* Re: [RFC][PATCH] slab: fix caller tracking onCONFIG_OPTIMIZE_INLINING.
  2011-09-24 23:00 ` David Rientjes
@ 2011-09-25  5:21   ` Tetsuo Handa
  2011-09-27  1:22     ` David Rientjes
  0 siblings, 1 reply; 5+ messages in thread
From: Tetsuo Handa @ 2011-09-25  5:21 UTC (permalink / raw)
  To: rientjes
  Cc: cl, penberg, mpm, vegard.nossum, dmonakhov, catalin.marinas,
	dfeng, linux-mm

David Rientjes wrote:
> On Sat, 24 Sep 2011, Tetsuo Handa wrote:
> 
> > If CONFIG_OPTIMIZE_INLINING=y, /proc/slab_allocators shows entries like
> > 
> >   size-512: 5 kzalloc+0xb/0x10
> >   size-256: 31 kzalloc+0xb/0x10
> > 
> > which are useless for debugging.
> 
> This is only an issue for gcc 4.x compilers, correct?

Yes.

> So this is going against the inlining algorithms in gcc 4.x which will 
> make the kernel image significantly larger even though there seems to be 
> no benefit unless you have CONFIG_DEBUG_SLAB_LEAK, although this patch 
> changes behavior for every system running CONFIG_SLAB with tracing 
> support.

If use of address of kzalloc() itself is fine for tracing functionality, we
don't need to force tracing functionality to use caller address of kzalloc().

I merely want /proc/slab_allocators to print caller address of kzalloc() rather
than kzalloc() address itself.

--
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] 5+ messages in thread

* Re: [RFC][PATCH] slab: fix caller tracking on CONFIG_OPTIMIZE_INLINING.
  2011-09-24  3:08 [RFC][PATCH] slab: fix caller tracking on CONFIG_OPTIMIZE_INLINING Tetsuo Handa
  2011-09-24 23:00 ` David Rientjes
@ 2011-09-26  6:17 ` Pekka Enberg
  1 sibling, 0 replies; 5+ messages in thread
From: Pekka Enberg @ 2011-09-26  6:17 UTC (permalink / raw)
  To: Tetsuo Handa
  Cc: cl, mpm, vegard.nossum, dmonakhov, catalin.marinas, rientjes,
	dfeng, linux-mm

On Sat, 24 Sep 2011, Tetsuo Handa wrote:
> If CONFIG_OPTIMIZE_INLINING=y, /proc/slab_allocators shows entries like
>
>  size-512: 5 kzalloc+0xb/0x10
>  size-256: 31 kzalloc+0xb/0x10
>
> which are useless for debugging.
> Use "__always_inline" rather than "inline" in order to make
> /proc/slab_allocators show caller of kzalloc() if caller tracking is enabled.
>
> Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> ----------
> diff --git a/include/linux/slab.h b/include/linux/slab.h
> index 573c809..2b745c0 100644
> --- a/include/linux/slab.h
> +++ b/include/linux/slab.h
> @@ -188,6 +188,18 @@ size_t ksize(const void *);
> #else
> #include <linux/slab_def.h>
> #endif
> +/*
> + * /proc/slab_allocator needs _RET_IP_ value. If CONFIG_OPTIMIZE_INLINING=y,
> + * use of "inline" causes compilers to pass address of kzalloc() etc. rather
> + * than address of caller. Thus, use "__always_inline" if _RET_IP_ value is
> + * needed.
> + */
> +#if defined(CONFIG_DEBUG_SLAB) || defined(CONFIG_SLUB) || \
> +	(defined(CONFIG_SLAB) && defined(CONFIG_TRACING))
> +#define slabtrace_inline __always_inline
> +#else
> +#define slabtrace_inline inline
> +#endif
>
> /**
>  * kcalloc - allocate memory for an array. The memory is set to zero.
> @@ -240,7 +252,7 @@ size_t ksize(const void *);
>  * for general use, and so are not documented here. For a full list of
>  * potential flags, always refer to linux/gfp.h.
>  */
> -static inline void *kcalloc(size_t n, size_t size, gfp_t flags)
> +static slabtrace_inline void *kcalloc(size_t n, size_t size, gfp_t flags)

So who don't we just make these __always_inline and leave it at that?

> {
> 	if (size != 0 && n > ULONG_MAX / size)
> 		return NULL;
> @@ -258,19 +270,19 @@ static inline void *kcalloc(size_t n, size_t size, gfp_t flags)
>  * if available. Equivalent to kmalloc() in the non-NUMA single-node
>  * case.
>  */
> -static inline void *kmalloc_node(size_t size, gfp_t flags, int node)
> +static slabtrace_inline void *kmalloc_node(size_t size, gfp_t flags, int node)
> {
> 	return kmalloc(size, flags);
> }
>
> -static inline void *__kmalloc_node(size_t size, gfp_t flags, int node)
> +static slabtrace_inline void *__kmalloc_node(size_t size, gfp_t flags, int node)
> {
> 	return __kmalloc(size, flags);
> }
>
> void *kmem_cache_alloc(struct kmem_cache *, gfp_t);
>
> -static inline void *kmem_cache_alloc_node(struct kmem_cache *cachep,
> +static slabtrace_inline void *kmem_cache_alloc_node(struct kmem_cache *cachep,
> 					gfp_t flags, int node)
> {
> 	return kmem_cache_alloc(cachep, flags);
> @@ -325,7 +337,7 @@ extern void *__kmalloc_node_track_caller(size_t, gfp_t, int, unsigned long);
> /*
>  * Shortcuts
>  */
> -static inline void *kmem_cache_zalloc(struct kmem_cache *k, gfp_t flags)
> +static slabtrace_inline void *kmem_cache_zalloc(struct kmem_cache *k, gfp_t flags)
> {
> 	return kmem_cache_alloc(k, flags | __GFP_ZERO);
> }
> @@ -335,7 +347,7 @@ static inline void *kmem_cache_zalloc(struct kmem_cache *k, gfp_t flags)
>  * @size: how many bytes of memory are required.
>  * @flags: the type of memory to allocate (see kmalloc).
>  */
> -static inline void *kzalloc(size_t size, gfp_t flags)
> +static slabtrace_inline void *kzalloc(size_t size, gfp_t flags)
> {
> 	return kmalloc(size, flags | __GFP_ZERO);
> }
> @@ -346,7 +358,7 @@ static inline void *kzalloc(size_t size, gfp_t flags)
>  * @flags: the type of memory to allocate (see kmalloc).
>  * @node: memory node from which to allocate
>  */
> -static inline void *kzalloc_node(size_t size, gfp_t flags, int node)
> +static slabtrace_inline void *kzalloc_node(size_t size, gfp_t flags, int node)
> {
> 	return kmalloc_node(size, flags | __GFP_ZERO, node);
> }
>

--
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] 5+ messages in thread

* Re: [RFC][PATCH] slab: fix caller tracking onCONFIG_OPTIMIZE_INLINING.
  2011-09-25  5:21   ` [RFC][PATCH] slab: fix caller tracking onCONFIG_OPTIMIZE_INLINING Tetsuo Handa
@ 2011-09-27  1:22     ` David Rientjes
  0 siblings, 0 replies; 5+ messages in thread
From: David Rientjes @ 2011-09-27  1:22 UTC (permalink / raw)
  To: Tetsuo Handa
  Cc: cl, penberg, mpm, vegard.nossum, dmonakhov, catalin.marinas,
	dfeng, linux-mm

On Sun, 25 Sep 2011, Tetsuo Handa wrote:

> > So this is going against the inlining algorithms in gcc 4.x which will 
> > make the kernel image significantly larger even though there seems to be 
> > no benefit unless you have CONFIG_DEBUG_SLAB_LEAK, although this patch 
> > changes behavior for every system running CONFIG_SLAB with tracing 
> > support.
> 
> If use of address of kzalloc() itself is fine for tracing functionality, we
> don't need to force tracing functionality to use caller address of kzalloc().
> 
> I merely want /proc/slab_allocators to print caller address of kzalloc() rather
> than kzalloc() address itself.
> 

Yeah, I understand the intent of the patch, but I don't think we need to 
force inlining in all the conditions that you specified it.  We know that 
CONFIG_DEBUG_SLAB_LEAK kernels aren't performance critical and it seems 
reasonable that they aren't image size critical either, but we certainly 
don't need this for kernels configured for SLUB or for SLAB kernels with 
tracing support and not CONFIG_DEBUG_SLAB_LEAK.

The "caller" formal to cache_alloc_debugcheck_after() wants the true 
caller of the allocation for CONFIG_DEBUG_SLAB_LEAK.  kmalloc() is already 
__always_inline, so just define slabtrace_inline to be __always_inline for 
CONFIG_DEBUG_SLAB_LEAK?

--
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] 5+ messages in thread

end of thread, other threads:[~2011-09-27  1:22 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-09-24  3:08 [RFC][PATCH] slab: fix caller tracking on CONFIG_OPTIMIZE_INLINING Tetsuo Handa
2011-09-24 23:00 ` David Rientjes
2011-09-25  5:21   ` [RFC][PATCH] slab: fix caller tracking onCONFIG_OPTIMIZE_INLINING Tetsuo Handa
2011-09-27  1:22     ` David Rientjes
2011-09-26  6:17 ` [RFC][PATCH] slab: fix caller tracking on CONFIG_OPTIMIZE_INLINING Pekka Enberg

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