linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 01/10] Makefile: Add option CONFIG_DISABLE_GCC_AUTOMATIC_INLINING
@ 2012-09-08 20:47 Ezequiel Garcia
  2012-09-08 20:47 ` [PATCH 02/10] mm, slob: Use NUMA_NO_NODE instead of -1 Ezequiel Garcia
                   ` (10 more replies)
  0 siblings, 11 replies; 27+ messages in thread
From: Ezequiel Garcia @ 2012-09-08 20:47 UTC (permalink / raw)
  To: linux-kernel, linux-mm; +Cc: Ezequiel Garcia, Michal Marek

As its name suggest this option prevents gcc from auto inlining
small functions. This is very important if one wants to obtain
traces with accurate call sites.

Without this option, gcc will collapse some small functions,
even when not marked as 'inline' thus making impossible to
correlate the trace caller address to the real function it belongs.

Of course, the resultant kernel is slower and slightly smaller,
but that's not an issue if the focus is on tracing accuracy.

Cc: Michal Marek <mmarek@suse.cz>
Signed-off-by: Ezequiel Garcia <elezegarcia@gmail.com>
---
 Makefile          |    4 ++++
 lib/Kconfig.debug |   11 +++++++++++
 2 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/Makefile b/Makefile
index ddf5be9..df6045a 100644
--- a/Makefile
+++ b/Makefile
@@ -561,6 +561,10 @@ else
 KBUILD_CFLAGS	+= -O2
 endif
 
+ifdef CONFIG_DISABLE_GCC_AUTOMATIC_INLINING
+KBUILD_CFLAGS	+= -fno-inline-small-functions
+endif
+
 include $(srctree)/arch/$(SRCARCH)/Makefile
 
 ifdef CONFIG_READABLE_ASM
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 2403a63..c8fd50f 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -1265,6 +1265,17 @@ config LATENCYTOP
 source mm/Kconfig.debug
 source kernel/trace/Kconfig
 
+config DISABLE_GCC_AUTOMATIC_INLINING
+	bool "Disable gcc automatic inlining"
+	depends on TRACING
+	help
+	  This option tells gcc he's not allowed to inline functions automatically,
+	  when they are not marked as 'inline'.
+	  In turn, this enables to trace an event with an accurate call site.
+	  Of course, the resultant kernel is slower and slightly smaller.
+
+	  Select this option only if you want to trace call sites accurately.
+
 config PROVIDE_OHCI1394_DMA_INIT
 	bool "Remote debugging over FireWire early on boot"
 	depends on PCI && X86
-- 
1.7.8.6


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

* [PATCH 02/10] mm, slob: Use NUMA_NO_NODE instead of -1
  2012-09-08 20:47 [PATCH 01/10] Makefile: Add option CONFIG_DISABLE_GCC_AUTOMATIC_INLINING Ezequiel Garcia
@ 2012-09-08 20:47 ` Ezequiel Garcia
  2012-09-09 21:27   ` David Rientjes
  2012-09-08 20:47 ` [PATCH 03/10] mm, slab: Remove silly function slab_buffer_size() Ezequiel Garcia
                   ` (9 subsequent siblings)
  10 siblings, 1 reply; 27+ messages in thread
From: Ezequiel Garcia @ 2012-09-08 20:47 UTC (permalink / raw)
  To: linux-kernel, linux-mm; +Cc: Ezequiel Garcia, Pekka Enberg

Cc: Pekka Enberg <penberg@kernel.org>
Signed-off-by: Ezequiel Garcia <elezegarcia@gmail.com>
---
 include/linux/slob_def.h |    6 ++++--
 mm/slob.c                |    6 +++---
 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/include/linux/slob_def.h b/include/linux/slob_def.h
index 0ec00b3..f28e14a 100644
--- a/include/linux/slob_def.h
+++ b/include/linux/slob_def.h
@@ -1,12 +1,14 @@
 #ifndef __LINUX_SLOB_DEF_H
 #define __LINUX_SLOB_DEF_H
 
+#include <linux/numa.h>
+
 void *kmem_cache_alloc_node(struct kmem_cache *, gfp_t flags, int node);
 
 static __always_inline void *kmem_cache_alloc(struct kmem_cache *cachep,
 					      gfp_t flags)
 {
-	return kmem_cache_alloc_node(cachep, flags, -1);
+	return kmem_cache_alloc_node(cachep, flags, NUMA_NO_NODE);
 }
 
 void *__kmalloc_node(size_t size, gfp_t flags, int node);
@@ -26,7 +28,7 @@ static __always_inline void *kmalloc_node(size_t size, gfp_t flags, int node)
  */
 static __always_inline void *kmalloc(size_t size, gfp_t flags)
 {
-	return __kmalloc_node(size, flags, -1);
+	return __kmalloc_node(size, flags, NUMA_NO_NODE);
 }
 
 static __always_inline void *__kmalloc(size_t size, gfp_t flags)
diff --git a/mm/slob.c b/mm/slob.c
index ae46edc..c0c1a93 100644
--- a/mm/slob.c
+++ b/mm/slob.c
@@ -193,7 +193,7 @@ static void *slob_new_pages(gfp_t gfp, int order, int node)
 	void *page;
 
 #ifdef CONFIG_NUMA
-	if (node != -1)
+	if (node != NUMA_NO_NODE)
 		page = alloc_pages_exact_node(node, gfp, order);
 	else
 #endif
@@ -289,7 +289,7 @@ static void *slob_alloc(size_t size, gfp_t gfp, int align, int node)
 		 * If there's a node specification, search for a partial
 		 * page with a matching node id in the freelist.
 		 */
-		if (node != -1 && page_to_nid(sp) != node)
+		if (node != NUMA_NO_NODE && page_to_nid(sp) != node)
 			continue;
 #endif
 		/* Enough room on this page? */
@@ -510,7 +510,7 @@ struct kmem_cache *__kmem_cache_create(const char *name, size_t size,
 	struct kmem_cache *c;
 
 	c = slob_alloc(sizeof(struct kmem_cache),
-		GFP_KERNEL, ARCH_KMALLOC_MINALIGN, -1);
+		GFP_KERNEL, ARCH_KMALLOC_MINALIGN, NUMA_NO_NODE);
 
 	if (c) {
 		c->name = name;
-- 
1.7.8.6


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

* [PATCH 03/10] mm, slab: Remove silly function slab_buffer_size()
  2012-09-08 20:47 [PATCH 01/10] Makefile: Add option CONFIG_DISABLE_GCC_AUTOMATIC_INLINING Ezequiel Garcia
  2012-09-08 20:47 ` [PATCH 02/10] mm, slob: Use NUMA_NO_NODE instead of -1 Ezequiel Garcia
@ 2012-09-08 20:47 ` Ezequiel Garcia
  2012-09-09 21:28   ` David Rientjes
  2012-09-08 20:47 ` [PATCH 04/10] mm, slob: Add support for kmalloc_track_caller() Ezequiel Garcia
                   ` (8 subsequent siblings)
  10 siblings, 1 reply; 27+ messages in thread
From: Ezequiel Garcia @ 2012-09-08 20:47 UTC (permalink / raw)
  To: linux-kernel, linux-mm; +Cc: Ezequiel Garcia, Pekka Enberg

This function is seldom used, and can be simply replaced with cachep->size.

Cc: Pekka Enberg <penberg@kernel.org>
Signed-off-by: Ezequiel Garcia <elezegarcia@gmail.com>
---
 include/linux/slab_def.h |    5 -----
 mm/slab.c                |   12 ++----------
 2 files changed, 2 insertions(+), 15 deletions(-)

diff --git a/include/linux/slab_def.h b/include/linux/slab_def.h
index 36d7031..604ebc8 100644
--- a/include/linux/slab_def.h
+++ b/include/linux/slab_def.h
@@ -113,17 +113,12 @@ void *__kmalloc(size_t size, gfp_t flags);
 #ifdef CONFIG_TRACING
 extern void *kmem_cache_alloc_trace(size_t size,
 				    struct kmem_cache *cachep, gfp_t flags);
-extern size_t slab_buffer_size(struct kmem_cache *cachep);
 #else
 static __always_inline void *
 kmem_cache_alloc_trace(size_t size, struct kmem_cache *cachep, gfp_t flags)
 {
 	return kmem_cache_alloc(cachep, flags);
 }
-static inline size_t slab_buffer_size(struct kmem_cache *cachep)
-{
-	return 0;
-}
 #endif
 
 static __always_inline void *kmalloc(size_t size, gfp_t flags)
diff --git a/mm/slab.c b/mm/slab.c
index 3b4587b..53e41de 100644
--- a/mm/slab.c
+++ b/mm/slab.c
@@ -498,14 +498,6 @@ static void **dbg_userword(struct kmem_cache *cachep, void *objp)
 
 #endif
 
-#ifdef CONFIG_TRACING
-size_t slab_buffer_size(struct kmem_cache *cachep)
-{
-	return cachep->size;
-}
-EXPORT_SYMBOL(slab_buffer_size);
-#endif
-
 /*
  * Do not go above this order unless 0 objects fit into the slab or
  * overridden on the command line.
@@ -3849,7 +3841,7 @@ kmem_cache_alloc_trace(size_t size, struct kmem_cache *cachep, gfp_t flags)
 	ret = __cache_alloc(cachep, flags, __builtin_return_address(0));
 
 	trace_kmalloc(_RET_IP_, ret,
-		      size, slab_buffer_size(cachep), flags);
+		      size, cachep->size, flags);
 	return ret;
 }
 EXPORT_SYMBOL(kmem_cache_alloc_trace);
@@ -3880,7 +3872,7 @@ void *kmem_cache_alloc_node_trace(size_t size,
 	ret = __cache_alloc_node(cachep, flags, nodeid,
 				  __builtin_return_address(0));
 	trace_kmalloc_node(_RET_IP_, ret,
-			   size, slab_buffer_size(cachep),
+			   size, cachep->size,
 			   flags, nodeid);
 	return ret;
 }
-- 
1.7.8.6


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

* [PATCH 04/10] mm, slob: Add support for kmalloc_track_caller()
  2012-09-08 20:47 [PATCH 01/10] Makefile: Add option CONFIG_DISABLE_GCC_AUTOMATIC_INLINING Ezequiel Garcia
  2012-09-08 20:47 ` [PATCH 02/10] mm, slob: Use NUMA_NO_NODE instead of -1 Ezequiel Garcia
  2012-09-08 20:47 ` [PATCH 03/10] mm, slab: Remove silly function slab_buffer_size() Ezequiel Garcia
@ 2012-09-08 20:47 ` Ezequiel Garcia
  2012-09-25 19:53   ` [patch slab/next] mm, slob: fix build breakage in __kmalloc_node_track_caller David Rientjes
  2012-09-08 20:47 ` [PATCH 05/10] mm, util: Use dup_user to duplicate user memory Ezequiel Garcia
                   ` (7 subsequent siblings)
  10 siblings, 1 reply; 27+ messages in thread
From: Ezequiel Garcia @ 2012-09-08 20:47 UTC (permalink / raw)
  To: linux-kernel, linux-mm; +Cc: Ezequiel Garcia, Pekka Enberg

Currently slob falls back to regular kmalloc for this case.
With this patch kmalloc_track_caller() is correctly implemented,
thus tracing the specified caller.

This is important to trace accurately allocations performed by
krealloc, kstrdup, kmemdup, etc.

Cc: Pekka Enberg <penberg@kernel.org>
Signed-off-by: Ezequiel Garcia <elezegarcia@gmail.com>
---
 include/linux/slab.h |    6 ++++--
 mm/slob.c            |   27 ++++++++++++++++++++++++---
 2 files changed, 28 insertions(+), 5 deletions(-)

diff --git a/include/linux/slab.h b/include/linux/slab.h
index 0dd2dfa..83d1a14 100644
--- a/include/linux/slab.h
+++ b/include/linux/slab.h
@@ -321,7 +321,8 @@ static inline void *kmem_cache_alloc_node(struct kmem_cache *cachep,
  * request comes from.
  */
 #if defined(CONFIG_DEBUG_SLAB) || defined(CONFIG_SLUB) || \
-	(defined(CONFIG_SLAB) && defined(CONFIG_TRACING))
+	(defined(CONFIG_SLAB) && defined(CONFIG_TRACING)) || \
+	(defined(CONFIG_SLOB) && defined(CONFIG_TRACING))
 extern void *__kmalloc_track_caller(size_t, gfp_t, unsigned long);
 #define kmalloc_track_caller(size, flags) \
 	__kmalloc_track_caller(size, flags, _RET_IP_)
@@ -340,7 +341,8 @@ extern void *__kmalloc_track_caller(size_t, gfp_t, unsigned long);
  * allocation request comes from.
  */
 #if defined(CONFIG_DEBUG_SLAB) || defined(CONFIG_SLUB) || \
-	(defined(CONFIG_SLAB) && defined(CONFIG_TRACING))
+	(defined(CONFIG_SLAB) && defined(CONFIG_TRACING)) || \
+	(defined(CONFIG_SLOB) && defined(CONFIG_TRACING))
 extern void *__kmalloc_node_track_caller(size_t, gfp_t, int, unsigned long);
 #define kmalloc_node_track_caller(size, flags, node) \
 	__kmalloc_node_track_caller(size, flags, node, \
diff --git a/mm/slob.c b/mm/slob.c
index c0c1a93..984c42a 100644
--- a/mm/slob.c
+++ b/mm/slob.c
@@ -424,7 +424,8 @@ out:
  * End of slob allocator proper. Begin kmem_cache_alloc and kmalloc frontend.
  */
 
-void *__kmalloc_node(size_t size, gfp_t gfp, int node)
+static __always_inline void *
+__do_kmalloc_node(size_t size, gfp_t gfp, int node, unsigned long caller)
 {
 	unsigned int *m;
 	int align = max(ARCH_KMALLOC_MINALIGN, ARCH_SLAB_MINALIGN);
@@ -445,7 +446,7 @@ void *__kmalloc_node(size_t size, gfp_t gfp, int node)
 		*m = size;
 		ret = (void *)m + align;
 
-		trace_kmalloc_node(_RET_IP_, ret,
+		trace_kmalloc_node(caller, ret,
 				   size, size + align, gfp, node);
 	} else {
 		unsigned int order = get_order(size);
@@ -454,15 +455,35 @@ void *__kmalloc_node(size_t size, gfp_t gfp, int node)
 			gfp |= __GFP_COMP;
 		ret = slob_new_pages(gfp, order, node);
 
-		trace_kmalloc_node(_RET_IP_, ret,
+		trace_kmalloc_node(caller, ret,
 				   size, PAGE_SIZE << order, gfp, node);
 	}
 
 	kmemleak_alloc(ret, size, 1, gfp);
 	return ret;
 }
+
+void *__kmalloc_node(size_t size, gfp_t gfp, int node)
+{
+	return __do_kmalloc_node(size, gfp, node, _RET_IP_);
+}
 EXPORT_SYMBOL(__kmalloc_node);
 
+#ifdef CONFIG_TRACING
+void *__kmalloc_track_caller(size_t size, gfp_t gfp, unsigned long caller)
+{
+	return __do_kmalloc_node(size, gfp, NUMA_NO_NODE, caller);
+}
+
+#ifdef CONFIG_NUMA
+void *__kmalloc_node_track_caller(size_t size, gfp_t gfpflags,
+					int node, unsigned long caller)
+{
+	return __do_kmalloc_node(size, gfp, node, caller);
+}
+#endif
+#endif
+
 void kfree(const void *block)
 {
 	struct page *sp;
-- 
1.7.8.6


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

* [PATCH 05/10] mm, util: Use dup_user to duplicate user memory
  2012-09-08 20:47 [PATCH 01/10] Makefile: Add option CONFIG_DISABLE_GCC_AUTOMATIC_INLINING Ezequiel Garcia
                   ` (2 preceding siblings ...)
  2012-09-08 20:47 ` [PATCH 04/10] mm, slob: Add support for kmalloc_track_caller() Ezequiel Garcia
@ 2012-09-08 20:47 ` Ezequiel Garcia
  2012-09-25  7:15   ` Pekka Enberg
  2012-09-25 21:29   ` Andrew Morton
  2012-09-08 20:47 ` [PATCH 06/10] mm, slab: Replace 'caller' type, void* -> unsigned long Ezequiel Garcia
                   ` (6 subsequent siblings)
  10 siblings, 2 replies; 27+ messages in thread
From: Ezequiel Garcia @ 2012-09-08 20:47 UTC (permalink / raw)
  To: linux-kernel, linux-mm; +Cc: Ezequiel Garcia, Pekka Enberg

Previously the strndup_user allocation was being done through memdup_user,
and the caller was wrongly traced as being strndup_user
(the correct trace must report the caller of strndup_user).

This is a common problem: in order to get accurate callsite tracing,
a utils function can't allocate through another utils function,
but instead do the allocation himself (or inlined).

Here we fix this by creating an always inlined dup_user() function to
performed the real allocation and to be used by memdup_user and strndup_user.

Cc: Pekka Enberg <penberg@kernel.org>
Signed-off-by: Ezequiel Garcia <elezegarcia@gmail.com>
---
 mm/util.c |   11 ++++++++---
 1 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/mm/util.c b/mm/util.c
index dc3036c..48d3ff8b 100644
--- a/mm/util.c
+++ b/mm/util.c
@@ -76,14 +76,14 @@ void *kmemdup(const void *src, size_t len, gfp_t gfp)
 EXPORT_SYMBOL(kmemdup);
 
 /**
- * memdup_user - duplicate memory region from user space
+ * dup_user - duplicate memory region from user space
  *
  * @src: source address in user space
  * @len: number of bytes to copy
  *
  * Returns an ERR_PTR() on failure.
  */
-void *memdup_user(const void __user *src, size_t len)
+static __always_inline void *dup_user(const void __user *src, size_t len)
 {
 	void *p;
 
@@ -103,6 +103,11 @@ void *memdup_user(const void __user *src, size_t len)
 
 	return p;
 }
+
+void *memdup_user(const void __user *src, size_t len)
+{
+	return dup_user(src, len);
+}
 EXPORT_SYMBOL(memdup_user);
 
 static __always_inline void *__do_krealloc(const void *p, size_t new_size,
@@ -214,7 +219,7 @@ char *strndup_user(const char __user *s, long n)
 	if (length > n)
 		return ERR_PTR(-EINVAL);
 
-	p = memdup_user(s, length);
+	p = dup_user(s, length);
 
 	if (IS_ERR(p))
 		return p;
-- 
1.7.8.6


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

* [PATCH 06/10] mm, slab: Replace 'caller' type, void* -> unsigned long
  2012-09-08 20:47 [PATCH 01/10] Makefile: Add option CONFIG_DISABLE_GCC_AUTOMATIC_INLINING Ezequiel Garcia
                   ` (3 preceding siblings ...)
  2012-09-08 20:47 ` [PATCH 05/10] mm, util: Use dup_user to duplicate user memory Ezequiel Garcia
@ 2012-09-08 20:47 ` Ezequiel Garcia
  2012-09-08 20:47 ` [PATCH 07/10] mm, slab: Match SLAB and SLUB kmem_cache_alloc_xxx_trace() prototype Ezequiel Garcia
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 27+ messages in thread
From: Ezequiel Garcia @ 2012-09-08 20:47 UTC (permalink / raw)
  To: linux-kernel, linux-mm; +Cc: Ezequiel Garcia, Pekka Enberg

This allows to use _RET_IP_ instead of builtin_address(0), thus
achiveing implementation consistency in all three allocators.
Though maybe a nitpick, the real goal behind this patch is
to be able to obtain common code between SLAB and SLUB.

Cc: Pekka Enberg <penberg@kernel.org>
Signed-off-by: Ezequiel Garcia <elezegarcia@gmail.com>
---
 mm/slab.c |   50 ++++++++++++++++++++++++--------------------------
 1 files changed, 24 insertions(+), 26 deletions(-)

diff --git a/mm/slab.c b/mm/slab.c
index 53e41de..bc342d1 100644
--- a/mm/slab.c
+++ b/mm/slab.c
@@ -3083,7 +3083,7 @@ static inline void verify_redzone_free(struct kmem_cache *cache, void *obj)
 }
 
 static void *cache_free_debugcheck(struct kmem_cache *cachep, void *objp,
-				   void *caller)
+				   unsigned long caller)
 {
 	struct page *page;
 	unsigned int objnr;
@@ -3103,7 +3103,7 @@ static void *cache_free_debugcheck(struct kmem_cache *cachep, void *objp,
 		*dbg_redzone2(cachep, objp) = RED_INACTIVE;
 	}
 	if (cachep->flags & SLAB_STORE_USER)
-		*dbg_userword(cachep, objp) = caller;
+		*dbg_userword(cachep, objp) = (void *)caller;
 
 	objnr = obj_to_index(cachep, slabp, objp);
 
@@ -3116,7 +3116,7 @@ static void *cache_free_debugcheck(struct kmem_cache *cachep, void *objp,
 	if (cachep->flags & SLAB_POISON) {
 #ifdef CONFIG_DEBUG_PAGEALLOC
 		if ((cachep->size % PAGE_SIZE)==0 && OFF_SLAB(cachep)) {
-			store_stackinfo(cachep, objp, (unsigned long)caller);
+			store_stackinfo(cachep, objp, caller);
 			kernel_map_pages(virt_to_page(objp),
 					 cachep->size / PAGE_SIZE, 0);
 		} else {
@@ -3269,7 +3269,7 @@ static inline void cache_alloc_debugcheck_before(struct kmem_cache *cachep,
 
 #if DEBUG
 static void *cache_alloc_debugcheck_after(struct kmem_cache *cachep,
-				gfp_t flags, void *objp, void *caller)
+				gfp_t flags, void *objp, unsigned long caller)
 {
 	if (!objp)
 		return objp;
@@ -3286,7 +3286,7 @@ static void *cache_alloc_debugcheck_after(struct kmem_cache *cachep,
 		poison_obj(cachep, objp, POISON_INUSE);
 	}
 	if (cachep->flags & SLAB_STORE_USER)
-		*dbg_userword(cachep, objp) = caller;
+		*dbg_userword(cachep, objp) = (void *)caller;
 
 	if (cachep->flags & SLAB_RED_ZONE) {
 		if (*dbg_redzone1(cachep, objp) != RED_INACTIVE ||
@@ -3561,7 +3561,7 @@ done:
  */
 static __always_inline void *
 __cache_alloc_node(struct kmem_cache *cachep, gfp_t flags, int nodeid,
-		   void *caller)
+		   unsigned long caller)
 {
 	unsigned long save_flags;
 	void *ptr;
@@ -3647,7 +3647,7 @@ __do_cache_alloc(struct kmem_cache *cachep, gfp_t flags)
 #endif /* CONFIG_NUMA */
 
 static __always_inline void *
-__cache_alloc(struct kmem_cache *cachep, gfp_t flags, void *caller)
+__cache_alloc(struct kmem_cache *cachep, gfp_t flags, unsigned long caller)
 {
 	unsigned long save_flags;
 	void *objp;
@@ -3783,7 +3783,7 @@ free_done:
  * be in this state _before_ it is released.  Called with disabled ints.
  */
 static inline void __cache_free(struct kmem_cache *cachep, void *objp,
-    void *caller)
+				unsigned long caller)
 {
 	struct array_cache *ac = cpu_cache_get(cachep);
 
@@ -3823,7 +3823,7 @@ static inline void __cache_free(struct kmem_cache *cachep, void *objp,
  */
 void *kmem_cache_alloc(struct kmem_cache *cachep, gfp_t flags)
 {
-	void *ret = __cache_alloc(cachep, flags, __builtin_return_address(0));
+	void *ret = __cache_alloc(cachep, flags, _RET_IP_);
 
 	trace_kmem_cache_alloc(_RET_IP_, ret,
 			       cachep->object_size, cachep->size, flags);
@@ -3838,7 +3838,7 @@ kmem_cache_alloc_trace(size_t size, struct kmem_cache *cachep, gfp_t flags)
 {
 	void *ret;
 
-	ret = __cache_alloc(cachep, flags, __builtin_return_address(0));
+	ret = __cache_alloc(cachep, flags, _RET_IP_);
 
 	trace_kmalloc(_RET_IP_, ret,
 		      size, cachep->size, flags);
@@ -3850,8 +3850,7 @@ EXPORT_SYMBOL(kmem_cache_alloc_trace);
 #ifdef CONFIG_NUMA
 void *kmem_cache_alloc_node(struct kmem_cache *cachep, gfp_t flags, int nodeid)
 {
-	void *ret = __cache_alloc_node(cachep, flags, nodeid,
-				       __builtin_return_address(0));
+	void *ret = __cache_alloc_node(cachep, flags, nodeid, _RET_IP_);
 
 	trace_kmem_cache_alloc_node(_RET_IP_, ret,
 				    cachep->object_size, cachep->size,
@@ -3869,8 +3868,8 @@ void *kmem_cache_alloc_node_trace(size_t size,
 {
 	void *ret;
 
-	ret = __cache_alloc_node(cachep, flags, nodeid,
-				  __builtin_return_address(0));
+	ret = __cache_alloc_node(cachep, flags, nodeid, _RET_IP);
+
 	trace_kmalloc_node(_RET_IP_, ret,
 			   size, cachep->size,
 			   flags, nodeid);
@@ -3880,7 +3879,7 @@ EXPORT_SYMBOL(kmem_cache_alloc_node_trace);
 #endif
 
 static __always_inline void *
-__do_kmalloc_node(size_t size, gfp_t flags, int node, void *caller)
+__do_kmalloc_node(size_t size, gfp_t flags, int node, unsigned long caller)
 {
 	struct kmem_cache *cachep;
 
@@ -3893,21 +3892,20 @@ __do_kmalloc_node(size_t size, gfp_t flags, int node, void *caller)
 #if defined(CONFIG_DEBUG_SLAB) || defined(CONFIG_TRACING)
 void *__kmalloc_node(size_t size, gfp_t flags, int node)
 {
-	return __do_kmalloc_node(size, flags, node,
-			__builtin_return_address(0));
+	return __do_kmalloc_node(size, flags, node, _RET_IP_);
 }
 EXPORT_SYMBOL(__kmalloc_node);
 
 void *__kmalloc_node_track_caller(size_t size, gfp_t flags,
 		int node, unsigned long caller)
 {
-	return __do_kmalloc_node(size, flags, node, (void *)caller);
+	return __do_kmalloc_node(size, flags, node, caller);
 }
 EXPORT_SYMBOL(__kmalloc_node_track_caller);
 #else
 void *__kmalloc_node(size_t size, gfp_t flags, int node)
 {
-	return __do_kmalloc_node(size, flags, node, NULL);
+	return __do_kmalloc_node(size, flags, node, 0);
 }
 EXPORT_SYMBOL(__kmalloc_node);
 #endif /* CONFIG_DEBUG_SLAB || CONFIG_TRACING */
@@ -3920,7 +3918,7 @@ EXPORT_SYMBOL(__kmalloc_node);
  * @caller: function caller for debug tracking of the caller
  */
 static __always_inline void *__do_kmalloc(size_t size, gfp_t flags,
-					  void *caller)
+					  unsigned long caller)
 {
 	struct kmem_cache *cachep;
 	void *ret;
@@ -3935,7 +3933,7 @@ static __always_inline void *__do_kmalloc(size_t size, gfp_t flags,
 		return cachep;
 	ret = __cache_alloc(cachep, flags, caller);
 
-	trace_kmalloc((unsigned long) caller, ret,
+	trace_kmalloc(caller, ret,
 		      size, cachep->size, flags);
 
 	return ret;
@@ -3945,20 +3943,20 @@ static __always_inline void *__do_kmalloc(size_t size, gfp_t flags,
 #if defined(CONFIG_DEBUG_SLAB) || defined(CONFIG_TRACING)
 void *__kmalloc(size_t size, gfp_t flags)
 {
-	return __do_kmalloc(size, flags, __builtin_return_address(0));
+	return __do_kmalloc(size, flags, _RET_IP_);
 }
 EXPORT_SYMBOL(__kmalloc);
 
 void *__kmalloc_track_caller(size_t size, gfp_t flags, unsigned long caller)
 {
-	return __do_kmalloc(size, flags, (void *)caller);
+	return __do_kmalloc(size, flags, caller);
 }
 EXPORT_SYMBOL(__kmalloc_track_caller);
 
 #else
 void *__kmalloc(size_t size, gfp_t flags)
 {
-	return __do_kmalloc(size, flags, NULL);
+	return __do_kmalloc(size, flags, 0);
 }
 EXPORT_SYMBOL(__kmalloc);
 #endif
@@ -3979,7 +3977,7 @@ void kmem_cache_free(struct kmem_cache *cachep, void *objp)
 	debug_check_no_locks_freed(objp, cachep->object_size);
 	if (!(cachep->flags & SLAB_DEBUG_OBJECTS))
 		debug_check_no_obj_freed(objp, cachep->object_size);
-	__cache_free(cachep, objp, __builtin_return_address(0));
+	__cache_free(cachep, objp, _RET_IP_);
 	local_irq_restore(flags);
 
 	trace_kmem_cache_free(_RET_IP_, objp);
@@ -4010,7 +4008,7 @@ void kfree(const void *objp)
 	debug_check_no_locks_freed(objp, c->object_size);
 
 	debug_check_no_obj_freed(objp, c->object_size);
-	__cache_free(c, (void *)objp, __builtin_return_address(0));
+	__cache_free(c, (void *)objp, _RET_IP_);
 	local_irq_restore(flags);
 }
 EXPORT_SYMBOL(kfree);
-- 
1.7.8.6


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

* [PATCH 07/10] mm, slab: Match SLAB and SLUB kmem_cache_alloc_xxx_trace() prototype
  2012-09-08 20:47 [PATCH 01/10] Makefile: Add option CONFIG_DISABLE_GCC_AUTOMATIC_INLINING Ezequiel Garcia
                   ` (4 preceding siblings ...)
  2012-09-08 20:47 ` [PATCH 06/10] mm, slab: Replace 'caller' type, void* -> unsigned long Ezequiel Garcia
@ 2012-09-08 20:47 ` Ezequiel Garcia
  2012-09-08 20:47 ` [PATCH 08/10] mm, slab: Rename __cache_alloc() -> slab_alloc() Ezequiel Garcia
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 27+ messages in thread
From: Ezequiel Garcia @ 2012-09-08 20:47 UTC (permalink / raw)
  To: linux-kernel, linux-mm; +Cc: Ezequiel Garcia, Pekka Enberg, Christoph Lameter

This long (seemingly unnecessary) patch does not fix anything and
its only goal is to produce common code between SLAB and SLUB.

Cc: Pekka Enberg <penberg@kernel.org>
Cc: Christoph Lameter <cl@linux.com>
Signed-off-by: Ezequiel Garcia <elezegarcia@gmail.com>
---
 include/linux/slab_def.h |    7 +++----
 mm/slab.c                |   10 +++++-----
 2 files changed, 8 insertions(+), 9 deletions(-)

diff --git a/include/linux/slab_def.h b/include/linux/slab_def.h
index 604ebc8..e98caeb 100644
--- a/include/linux/slab_def.h
+++ b/include/linux/slab_def.h
@@ -111,11 +111,10 @@ void *kmem_cache_alloc(struct kmem_cache *, gfp_t);
 void *__kmalloc(size_t size, gfp_t flags);
 
 #ifdef CONFIG_TRACING
-extern void *kmem_cache_alloc_trace(size_t size,
-				    struct kmem_cache *cachep, gfp_t flags);
+extern void *kmem_cache_alloc_trace(struct kmem_cache *, gfp_t, size_t);
 #else
 static __always_inline void *
-kmem_cache_alloc_trace(size_t size, struct kmem_cache *cachep, gfp_t flags)
+kmem_cache_alloc_trace(struct kmem_cache *cachep, gfp_t flags, size_t size)
 {
 	return kmem_cache_alloc(cachep, flags);
 }
@@ -148,7 +147,7 @@ found:
 #endif
 			cachep = malloc_sizes[i].cs_cachep;
 
-		ret = kmem_cache_alloc_trace(size, cachep, flags);
+		ret = kmem_cache_alloc_trace(cachep, flags, size);
 
 		return ret;
 	}
diff --git a/mm/slab.c b/mm/slab.c
index bc342d1..47cb03c 100644
--- a/mm/slab.c
+++ b/mm/slab.c
@@ -3834,7 +3834,7 @@ EXPORT_SYMBOL(kmem_cache_alloc);
 
 #ifdef CONFIG_TRACING
 void *
-kmem_cache_alloc_trace(size_t size, struct kmem_cache *cachep, gfp_t flags)
+kmem_cache_alloc_trace(struct kmem_cache *cachep, gfp_t flags, size_t size)
 {
 	void *ret;
 
@@ -3861,10 +3861,10 @@ void *kmem_cache_alloc_node(struct kmem_cache *cachep, gfp_t flags, int nodeid)
 EXPORT_SYMBOL(kmem_cache_alloc_node);
 
 #ifdef CONFIG_TRACING
-void *kmem_cache_alloc_node_trace(size_t size,
-				  struct kmem_cache *cachep,
+void *kmem_cache_alloc_node_trace(struct kmem_cache *cachep,
 				  gfp_t flags,
-				  int nodeid)
+				  int nodeid,
+				  size_t size)
 {
 	void *ret;
 
@@ -3886,7 +3886,7 @@ __do_kmalloc_node(size_t size, gfp_t flags, int node, unsigned long caller)
 	cachep = kmem_find_general_cachep(size, flags);
 	if (unlikely(ZERO_OR_NULL_PTR(cachep)))
 		return cachep;
-	return kmem_cache_alloc_node_trace(size, cachep, flags, node);
+	return kmem_cache_alloc_node_trace(cachep, flags, node, size);
 }
 
 #if defined(CONFIG_DEBUG_SLAB) || defined(CONFIG_TRACING)
-- 
1.7.8.6


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

* [PATCH 08/10] mm, slab: Rename __cache_alloc() -> slab_alloc()
  2012-09-08 20:47 [PATCH 01/10] Makefile: Add option CONFIG_DISABLE_GCC_AUTOMATIC_INLINING Ezequiel Garcia
                   ` (5 preceding siblings ...)
  2012-09-08 20:47 ` [PATCH 07/10] mm, slab: Match SLAB and SLUB kmem_cache_alloc_xxx_trace() prototype Ezequiel Garcia
@ 2012-09-08 20:47 ` Ezequiel Garcia
  2012-09-08 20:47 ` [PATCH 09/10] mm, slub: Rename slab_alloc() -> slab_alloc_node() to match SLAB Ezequiel Garcia
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 27+ messages in thread
From: Ezequiel Garcia @ 2012-09-08 20:47 UTC (permalink / raw)
  To: linux-kernel, linux-mm; +Cc: Ezequiel Garcia, Pekka Enberg, Christoph Lameter

This patch does not fix anything and its only goal is to
produce common code between SLAB and SLUB.

Cc: Pekka Enberg <penberg@kernel.org>
Cc: Christoph Lameter <cl@linux.com>
Signed-off-by: Ezequiel Garcia <elezegarcia@gmail.com>
---
 mm/slab.c |   14 +++++++-------
 1 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/mm/slab.c b/mm/slab.c
index 47cb03c..57094ee 100644
--- a/mm/slab.c
+++ b/mm/slab.c
@@ -3560,7 +3560,7 @@ done:
  * Fallback to other node is possible if __GFP_THISNODE is not set.
  */
 static __always_inline void *
-__cache_alloc_node(struct kmem_cache *cachep, gfp_t flags, int nodeid,
+slab_alloc_node(struct kmem_cache *cachep, gfp_t flags, int nodeid,
 		   unsigned long caller)
 {
 	unsigned long save_flags;
@@ -3647,7 +3647,7 @@ __do_cache_alloc(struct kmem_cache *cachep, gfp_t flags)
 #endif /* CONFIG_NUMA */
 
 static __always_inline void *
-__cache_alloc(struct kmem_cache *cachep, gfp_t flags, unsigned long caller)
+slab_alloc(struct kmem_cache *cachep, gfp_t flags, unsigned long caller)
 {
 	unsigned long save_flags;
 	void *objp;
@@ -3823,7 +3823,7 @@ static inline void __cache_free(struct kmem_cache *cachep, void *objp,
  */
 void *kmem_cache_alloc(struct kmem_cache *cachep, gfp_t flags)
 {
-	void *ret = __cache_alloc(cachep, flags, _RET_IP_);
+	void *ret = slab_alloc(cachep, flags, _RET_IP_);
 
 	trace_kmem_cache_alloc(_RET_IP_, ret,
 			       cachep->object_size, cachep->size, flags);
@@ -3838,7 +3838,7 @@ kmem_cache_alloc_trace(struct kmem_cache *cachep, gfp_t flags, size_t size)
 {
 	void *ret;
 
-	ret = __cache_alloc(cachep, flags, _RET_IP_);
+	ret = slab_alloc(cachep, flags, _RET_IP_);
 
 	trace_kmalloc(_RET_IP_, ret,
 		      size, cachep->size, flags);
@@ -3850,7 +3850,7 @@ EXPORT_SYMBOL(kmem_cache_alloc_trace);
 #ifdef CONFIG_NUMA
 void *kmem_cache_alloc_node(struct kmem_cache *cachep, gfp_t flags, int nodeid)
 {
-	void *ret = __cache_alloc_node(cachep, flags, nodeid, _RET_IP_);
+	void *ret = slab_alloc_node(cachep, flags, nodeid, _RET_IP_);
 
 	trace_kmem_cache_alloc_node(_RET_IP_, ret,
 				    cachep->object_size, cachep->size,
@@ -3868,7 +3868,7 @@ void *kmem_cache_alloc_node_trace(struct kmem_cache *cachep,
 {
 	void *ret;
 
-	ret = __cache_alloc_node(cachep, flags, nodeid, _RET_IP);
+	ret = slab_alloc_node(cachep, flags, nodeid, _RET_IP);
 
 	trace_kmalloc_node(_RET_IP_, ret,
 			   size, cachep->size,
@@ -3931,7 +3931,7 @@ static __always_inline void *__do_kmalloc(size_t size, gfp_t flags,
 	cachep = __find_general_cachep(size, flags);
 	if (unlikely(ZERO_OR_NULL_PTR(cachep)))
 		return cachep;
-	ret = __cache_alloc(cachep, flags, caller);
+	ret = slab_alloc(cachep, flags, caller);
 
 	trace_kmalloc(caller, ret,
 		      size, cachep->size, flags);
-- 
1.7.8.6


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

* [PATCH 09/10] mm, slub: Rename slab_alloc() -> slab_alloc_node() to match SLAB
  2012-09-08 20:47 [PATCH 01/10] Makefile: Add option CONFIG_DISABLE_GCC_AUTOMATIC_INLINING Ezequiel Garcia
                   ` (6 preceding siblings ...)
  2012-09-08 20:47 ` [PATCH 08/10] mm, slab: Rename __cache_alloc() -> slab_alloc() Ezequiel Garcia
@ 2012-09-08 20:47 ` Ezequiel Garcia
  2012-09-08 20:47 ` [PATCH 10/10] mm: Factor SLAB and SLUB common code Ezequiel Garcia
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 27+ messages in thread
From: Ezequiel Garcia @ 2012-09-08 20:47 UTC (permalink / raw)
  To: linux-kernel, linux-mm; +Cc: Ezequiel Garcia, Christoph Lameter

This patch does not fix anything, and its only goal is to enable us
to obtain some common code between SLAB and SLUB.
Neither behavior nor produced code is affected.

Cc: Christoph Lameter <cl@linux.com>
Signed-off-by: Ezequiel Garcia <elezegarcia@gmail.com>
---
 mm/slub.c |   24 +++++++++++++++---------
 1 files changed, 15 insertions(+), 9 deletions(-)

diff --git a/mm/slub.c b/mm/slub.c
index c67bd0a..786a181 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -2311,7 +2311,7 @@ new_slab:
  *
  * Otherwise we can simply pick the next object from the lockless free list.
  */
-static __always_inline void *slab_alloc(struct kmem_cache *s,
+static __always_inline void *slab_alloc_node(struct kmem_cache *s,
 		gfp_t gfpflags, int node, unsigned long addr)
 {
 	void **object;
@@ -2381,9 +2381,15 @@ redo:
 	return object;
 }
 
+static __always_inline void *slab_alloc(struct kmem_cache *s,
+		gfp_t gfpflags, unsigned long addr)
+{
+	return slab_alloc_node(s, gfpflags, NUMA_NO_NODE, addr);
+}
+
 void *kmem_cache_alloc(struct kmem_cache *s, gfp_t gfpflags)
 {
-	void *ret = slab_alloc(s, gfpflags, NUMA_NO_NODE, _RET_IP_);
+	void *ret = slab_alloc(s, gfpflags, _RET_IP_);
 
 	trace_kmem_cache_alloc(_RET_IP_, ret, s->object_size, s->size, gfpflags);
 
@@ -2394,7 +2400,7 @@ EXPORT_SYMBOL(kmem_cache_alloc);
 #ifdef CONFIG_TRACING
 void *kmem_cache_alloc_trace(struct kmem_cache *s, gfp_t gfpflags, size_t size)
 {
-	void *ret = slab_alloc(s, gfpflags, NUMA_NO_NODE, _RET_IP_);
+	void *ret = slab_alloc(s, gfpflags, _RET_IP_);
 	trace_kmalloc(_RET_IP_, ret, size, s->size, gfpflags);
 	return ret;
 }
@@ -2412,7 +2418,7 @@ EXPORT_SYMBOL(kmalloc_order_trace);
 #ifdef CONFIG_NUMA
 void *kmem_cache_alloc_node(struct kmem_cache *s, gfp_t gfpflags, int node)
 {
-	void *ret = slab_alloc(s, gfpflags, node, _RET_IP_);
+	void *ret = slab_alloc_node(s, gfpflags, node, _RET_IP_);
 
 	trace_kmem_cache_alloc_node(_RET_IP_, ret,
 				    s->object_size, s->size, gfpflags, node);
@@ -2426,7 +2432,7 @@ void *kmem_cache_alloc_node_trace(struct kmem_cache *s,
 				    gfp_t gfpflags,
 				    int node, size_t size)
 {
-	void *ret = slab_alloc(s, gfpflags, node, _RET_IP_);
+	void *ret = slab_alloc_node(s, gfpflags, node, _RET_IP_);
 
 	trace_kmalloc_node(_RET_IP_, ret,
 			   size, s->size, gfpflags, node);
@@ -3364,7 +3370,7 @@ void *__kmalloc(size_t size, gfp_t flags)
 	if (unlikely(ZERO_OR_NULL_PTR(s)))
 		return s;
 
-	ret = slab_alloc(s, flags, NUMA_NO_NODE, _RET_IP_);
+	ret = slab_alloc(s, flags, _RET_IP_);
 
 	trace_kmalloc(_RET_IP_, ret, size, s->size, flags);
 
@@ -3407,7 +3413,7 @@ void *__kmalloc_node(size_t size, gfp_t flags, int node)
 	if (unlikely(ZERO_OR_NULL_PTR(s)))
 		return s;
 
-	ret = slab_alloc(s, flags, node, _RET_IP_);
+	ret = slab_alloc_node(s, flags, node, _RET_IP_);
 
 	trace_kmalloc_node(_RET_IP_, ret, size, s->size, flags, node);
 
@@ -4035,7 +4041,7 @@ void *__kmalloc_track_caller(size_t size, gfp_t gfpflags, unsigned long caller)
 	if (unlikely(ZERO_OR_NULL_PTR(s)))
 		return s;
 
-	ret = slab_alloc(s, gfpflags, NUMA_NO_NODE, caller);
+	ret = slab_alloc(s, gfpflags, caller);
 
 	/* Honor the call site pointer we received. */
 	trace_kmalloc(caller, ret, size, s->size, gfpflags);
@@ -4065,7 +4071,7 @@ void *__kmalloc_node_track_caller(size_t size, gfp_t gfpflags,
 	if (unlikely(ZERO_OR_NULL_PTR(s)))
 		return s;
 
-	ret = slab_alloc(s, gfpflags, node, caller);
+	ret = slab_alloc_node(s, gfpflags, node, caller);
 
 	/* Honor the call site pointer we received. */
 	trace_kmalloc_node(caller, ret, size, s->size, gfpflags, node);
-- 
1.7.8.6


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

* [PATCH 10/10] mm: Factor SLAB and SLUB common code
  2012-09-08 20:47 [PATCH 01/10] Makefile: Add option CONFIG_DISABLE_GCC_AUTOMATIC_INLINING Ezequiel Garcia
                   ` (7 preceding siblings ...)
  2012-09-08 20:47 ` [PATCH 09/10] mm, slub: Rename slab_alloc() -> slab_alloc_node() to match SLAB Ezequiel Garcia
@ 2012-09-08 20:47 ` Ezequiel Garcia
  2012-09-08 21:43 ` [PATCH 01/10] Makefile: Add option CONFIG_DISABLE_GCC_AUTOMATIC_INLINING Sam Ravnborg
  2012-09-09 21:25 ` David Rientjes
  10 siblings, 0 replies; 27+ messages in thread
From: Ezequiel Garcia @ 2012-09-08 20:47 UTC (permalink / raw)
  To: linux-kernel, linux-mm; +Cc: Ezequiel Garcia, Pekka Enberg, Christoph Lameter

Several interfaces are now exactly the same in both SLAB and SLUB,
and it's very easy to factor them out.
To make this work slab_alloc() and slab_alloc_node() are made non-static.

This has the benefit of putting most of the tracing code in mm/slab_common.c,
making it harder to produce inconsistent trace behavior.

Cc: Pekka Enberg <penberg@kernel.org>
Cc: Christoph Lameter <cl@linux.com>
Signed-off-by: Ezequiel Garcia <elezegarcia@gmail.com>
---
 include/linux/slab_def.h |    2 +
 include/linux/slub_def.h |    2 +
 mm/slab.c                |   68 +--------------------------------------------
 mm/slab_common.c         |   57 ++++++++++++++++++++++++++++++++++++++
 mm/slub.c                |   49 +-------------------------------
 5 files changed, 65 insertions(+), 113 deletions(-)

diff --git a/include/linux/slab_def.h b/include/linux/slab_def.h
index e98caeb..021c162 100644
--- a/include/linux/slab_def.h
+++ b/include/linux/slab_def.h
@@ -107,6 +107,8 @@ struct cache_sizes {
 };
 extern struct cache_sizes malloc_sizes[];
 
+void *slab_alloc(struct kmem_cache *, gfp_t, unsigned long);
+void *slab_alloc_node(struct kmem_cache *, gfp_t, int, unsigned long);
 void *kmem_cache_alloc(struct kmem_cache *, gfp_t);
 void *__kmalloc(size_t size, gfp_t flags);
 
diff --git a/include/linux/slub_def.h b/include/linux/slub_def.h
index df448ad..d94f457 100644
--- a/include/linux/slub_def.h
+++ b/include/linux/slub_def.h
@@ -216,6 +216,8 @@ static __always_inline struct kmem_cache *kmalloc_slab(size_t size)
 	return kmalloc_caches[index];
 }
 
+void *slab_alloc(struct kmem_cache *, gfp_t, unsigned long);
+void *slab_alloc_node(struct kmem_cache *, gfp_t, int, unsigned long);
 void *kmem_cache_alloc(struct kmem_cache *, gfp_t);
 void *__kmalloc(size_t size, gfp_t flags);
 
diff --git a/mm/slab.c b/mm/slab.c
index 57094ee..d7f8466 100644
--- a/mm/slab.c
+++ b/mm/slab.c
@@ -3559,7 +3559,7 @@ done:
  *
  * Fallback to other node is possible if __GFP_THISNODE is not set.
  */
-static __always_inline void *
+__always_inline void *
 slab_alloc_node(struct kmem_cache *cachep, gfp_t flags, int nodeid,
 		   unsigned long caller)
 {
@@ -3646,7 +3646,7 @@ __do_cache_alloc(struct kmem_cache *cachep, gfp_t flags)
 
 #endif /* CONFIG_NUMA */
 
-static __always_inline void *
+__always_inline void *
 slab_alloc(struct kmem_cache *cachep, gfp_t flags, unsigned long caller)
 {
 	unsigned long save_flags;
@@ -3813,71 +3813,7 @@ static inline void __cache_free(struct kmem_cache *cachep, void *objp,
 	ac_put_obj(cachep, ac, objp);
 }
 
-/**
- * kmem_cache_alloc - Allocate an object
- * @cachep: The cache to allocate from.
- * @flags: See kmalloc().
- *
- * Allocate an object from this cache.  The flags are only relevant
- * if the cache has no available objects.
- */
-void *kmem_cache_alloc(struct kmem_cache *cachep, gfp_t flags)
-{
-	void *ret = slab_alloc(cachep, flags, _RET_IP_);
-
-	trace_kmem_cache_alloc(_RET_IP_, ret,
-			       cachep->object_size, cachep->size, flags);
-
-	return ret;
-}
-EXPORT_SYMBOL(kmem_cache_alloc);
-
-#ifdef CONFIG_TRACING
-void *
-kmem_cache_alloc_trace(struct kmem_cache *cachep, gfp_t flags, size_t size)
-{
-	void *ret;
-
-	ret = slab_alloc(cachep, flags, _RET_IP_);
-
-	trace_kmalloc(_RET_IP_, ret,
-		      size, cachep->size, flags);
-	return ret;
-}
-EXPORT_SYMBOL(kmem_cache_alloc_trace);
-#endif
-
 #ifdef CONFIG_NUMA
-void *kmem_cache_alloc_node(struct kmem_cache *cachep, gfp_t flags, int nodeid)
-{
-	void *ret = slab_alloc_node(cachep, flags, nodeid, _RET_IP_);
-
-	trace_kmem_cache_alloc_node(_RET_IP_, ret,
-				    cachep->object_size, cachep->size,
-				    flags, nodeid);
-
-	return ret;
-}
-EXPORT_SYMBOL(kmem_cache_alloc_node);
-
-#ifdef CONFIG_TRACING
-void *kmem_cache_alloc_node_trace(struct kmem_cache *cachep,
-				  gfp_t flags,
-				  int nodeid,
-				  size_t size)
-{
-	void *ret;
-
-	ret = slab_alloc_node(cachep, flags, nodeid, _RET_IP);
-
-	trace_kmalloc_node(_RET_IP_, ret,
-			   size, cachep->size,
-			   flags, nodeid);
-	return ret;
-}
-EXPORT_SYMBOL(kmem_cache_alloc_node_trace);
-#endif
-
 static __always_inline void *
 __do_kmalloc_node(size_t size, gfp_t flags, int node, unsigned long caller)
 {
diff --git a/mm/slab_common.c b/mm/slab_common.c
index 8cf8b49..5fc0da0 100644
--- a/mm/slab_common.c
+++ b/mm/slab_common.c
@@ -17,6 +17,7 @@
 #include <asm/tlbflush.h>
 #include <asm/page.h>
 
+#include <trace/events/kmem.h>
 #include "slab.h"
 
 enum slab_state slab_state;
@@ -113,6 +114,62 @@ struct kmem_cache *kmem_cache_create(const char *name, size_t size, size_t align
 }
 EXPORT_SYMBOL(kmem_cache_create);
 
+#if defined(CONFIG_SLAB) || defined(CONFIG_SLUB)
+/**
+ * kmem_cache_alloc - Allocate an object
+ * @cachep: The cache to allocate from.
+ * @flags: See kmalloc().
+ *
+ * Allocate an object from this cache.  The flags are only relevant
+ * if the cache has no available objects.
+ */
+void *kmem_cache_alloc(struct kmem_cache *s, gfp_t flags)
+{
+	void *ret = slab_alloc(s, flags, _RET_IP_);
+
+	trace_kmem_cache_alloc(_RET_IP_, ret, s->object_size, s->size, flags);
+
+	return ret;
+}
+EXPORT_SYMBOL(kmem_cache_alloc);
+
+#ifdef CONFIG_TRACING
+void *kmem_cache_alloc_trace(struct kmem_cache *s, gfp_t gfpflags, size_t size)
+{
+	void *ret = slab_alloc(s, gfpflags, _RET_IP_);
+	trace_kmalloc(_RET_IP_, ret, size, s->size, gfpflags);
+	return ret;
+}
+EXPORT_SYMBOL(kmem_cache_alloc_trace);
+#endif
+
+#ifdef CONFIG_NUMA
+void *kmem_cache_alloc_node(struct kmem_cache *s, gfp_t gfpflags, int node)
+{
+	void *ret = slab_alloc_node(s, gfpflags, node, _RET_IP_);
+
+	trace_kmem_cache_alloc_node(_RET_IP_, ret,
+				    s->object_size, s->size, gfpflags, node);
+
+	return ret;
+}
+EXPORT_SYMBOL(kmem_cache_alloc_node);
+
+#ifdef CONFIG_TRACING
+void *kmem_cache_alloc_node_trace(struct kmem_cache *s,
+				    gfp_t gfpflags,
+				    int node, size_t size)
+{
+	void *ret = slab_alloc_node(s, gfpflags, node, _RET_IP_);
+
+	trace_kmalloc_node(_RET_IP_, ret, size, s->size, gfpflags, node);
+	return ret;
+}
+EXPORT_SYMBOL(kmem_cache_alloc_node_trace);
+#endif /* CONFIG_TRACING */
+#endif /* CONFIG_NUMA */
+#endif /* CONFIG_SLAB || CONFIG_SLUB */
+
 int slab_is_available(void)
 {
 	return slab_state >= UP;
diff --git a/mm/slub.c b/mm/slub.c
index 786a181..fa0fb4a 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -2311,7 +2311,7 @@ new_slab:
  *
  * Otherwise we can simply pick the next object from the lockless free list.
  */
-static __always_inline void *slab_alloc_node(struct kmem_cache *s,
+__always_inline void *slab_alloc_node(struct kmem_cache *s,
 		gfp_t gfpflags, int node, unsigned long addr)
 {
 	void **object;
@@ -2381,31 +2381,13 @@ redo:
 	return object;
 }
 
-static __always_inline void *slab_alloc(struct kmem_cache *s,
+__always_inline void *slab_alloc(struct kmem_cache *s,
 		gfp_t gfpflags, unsigned long addr)
 {
 	return slab_alloc_node(s, gfpflags, NUMA_NO_NODE, addr);
 }
 
-void *kmem_cache_alloc(struct kmem_cache *s, gfp_t gfpflags)
-{
-	void *ret = slab_alloc(s, gfpflags, _RET_IP_);
-
-	trace_kmem_cache_alloc(_RET_IP_, ret, s->object_size, s->size, gfpflags);
-
-	return ret;
-}
-EXPORT_SYMBOL(kmem_cache_alloc);
-
 #ifdef CONFIG_TRACING
-void *kmem_cache_alloc_trace(struct kmem_cache *s, gfp_t gfpflags, size_t size)
-{
-	void *ret = slab_alloc(s, gfpflags, _RET_IP_);
-	trace_kmalloc(_RET_IP_, ret, size, s->size, gfpflags);
-	return ret;
-}
-EXPORT_SYMBOL(kmem_cache_alloc_trace);
-
 void *kmalloc_order_trace(size_t size, gfp_t flags, unsigned int order)
 {
 	void *ret = kmalloc_order(size, flags, order);
@@ -2415,33 +2397,6 @@ void *kmalloc_order_trace(size_t size, gfp_t flags, unsigned int order)
 EXPORT_SYMBOL(kmalloc_order_trace);
 #endif
 
-#ifdef CONFIG_NUMA
-void *kmem_cache_alloc_node(struct kmem_cache *s, gfp_t gfpflags, int node)
-{
-	void *ret = slab_alloc_node(s, gfpflags, node, _RET_IP_);
-
-	trace_kmem_cache_alloc_node(_RET_IP_, ret,
-				    s->object_size, s->size, gfpflags, node);
-
-	return ret;
-}
-EXPORT_SYMBOL(kmem_cache_alloc_node);
-
-#ifdef CONFIG_TRACING
-void *kmem_cache_alloc_node_trace(struct kmem_cache *s,
-				    gfp_t gfpflags,
-				    int node, size_t size)
-{
-	void *ret = slab_alloc_node(s, gfpflags, node, _RET_IP_);
-
-	trace_kmalloc_node(_RET_IP_, ret,
-			   size, s->size, gfpflags, node);
-	return ret;
-}
-EXPORT_SYMBOL(kmem_cache_alloc_node_trace);
-#endif
-#endif
-
 /*
  * Slow patch handling. This may still be called frequently since objects
  * have a longer lifetime than the cpu slabs in most processing loads.
-- 
1.7.8.6


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

* Re: [PATCH 01/10] Makefile: Add option CONFIG_DISABLE_GCC_AUTOMATIC_INLINING
  2012-09-08 20:47 [PATCH 01/10] Makefile: Add option CONFIG_DISABLE_GCC_AUTOMATIC_INLINING Ezequiel Garcia
                   ` (8 preceding siblings ...)
  2012-09-08 20:47 ` [PATCH 10/10] mm: Factor SLAB and SLUB common code Ezequiel Garcia
@ 2012-09-08 21:43 ` Sam Ravnborg
  2012-09-09 21:25 ` David Rientjes
  10 siblings, 0 replies; 27+ messages in thread
From: Sam Ravnborg @ 2012-09-08 21:43 UTC (permalink / raw)
  To: Ezequiel Garcia; +Cc: linux-kernel, linux-mm, Michal Marek

On Sat, Sep 08, 2012 at 05:47:50PM -0300, Ezequiel Garcia wrote:
> As its name suggest this option prevents gcc from auto inlining
> small functions. This is very important if one wants to obtain
> traces with accurate call sites.
> 
> Without this option, gcc will collapse some small functions,
> even when not marked as 'inline' thus making impossible to
> correlate the trace caller address to the real function it belongs.
> 
> Of course, the resultant kernel is slower and slightly smaller,
> but that's not an issue if the focus is on tracing accuracy.
> 
> Cc: Michal Marek <mmarek@suse.cz>
> Signed-off-by: Ezequiel Garcia <elezegarcia@gmail.com>
> ---
>  Makefile          |    4 ++++
>  lib/Kconfig.debug |   11 +++++++++++
>  2 files changed, 15 insertions(+), 0 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index ddf5be9..df6045a 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -561,6 +561,10 @@ else
>  KBUILD_CFLAGS	+= -O2
>  endif
>  
> +ifdef CONFIG_DISABLE_GCC_AUTOMATIC_INLINING
> +KBUILD_CFLAGS	+= -fno-inline-small-functions
> +endif
> +
>  include $(srctree)/arch/$(SRCARCH)/Makefile
>  
>  ifdef CONFIG_READABLE_ASM
> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index 2403a63..c8fd50f 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -1265,6 +1265,17 @@ config LATENCYTOP
>  source mm/Kconfig.debug
>  source kernel/trace/Kconfig
>  
> +config DISABLE_GCC_AUTOMATIC_INLINING
> +	bool "Disable gcc automatic inlining"

Could we please call this option for:
config CC_DISABLE_AUTOMATIC_INLINING

We have at least a few other options following that naming scheme.
And today we have no options named *GCC_*

	Sam

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

* Re: [PATCH 01/10] Makefile: Add option CONFIG_DISABLE_GCC_AUTOMATIC_INLINING
  2012-09-08 20:47 [PATCH 01/10] Makefile: Add option CONFIG_DISABLE_GCC_AUTOMATIC_INLINING Ezequiel Garcia
                   ` (9 preceding siblings ...)
  2012-09-08 21:43 ` [PATCH 01/10] Makefile: Add option CONFIG_DISABLE_GCC_AUTOMATIC_INLINING Sam Ravnborg
@ 2012-09-09 21:25 ` David Rientjes
  2012-09-13  0:30   ` Ezequiel Garcia
  10 siblings, 1 reply; 27+ messages in thread
From: David Rientjes @ 2012-09-09 21:25 UTC (permalink / raw)
  To: Ezequiel Garcia; +Cc: linux-kernel, linux-mm, Michal Marek

On Sat, 8 Sep 2012, Ezequiel Garcia wrote:

> diff --git a/Makefile b/Makefile
> index ddf5be9..df6045a 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -561,6 +561,10 @@ else
>  KBUILD_CFLAGS	+= -O2
>  endif
>  
> +ifdef CONFIG_DISABLE_GCC_AUTOMATIC_INLINING
> +KBUILD_CFLAGS	+= -fno-inline-small-functions

This isn't the only option that controls automatic inlining of functions, 
see indirect-inlining, inline-functions, and inline-functions-called-once.

> +endif
> +
>  include $(srctree)/arch/$(SRCARCH)/Makefile
>  
>  ifdef CONFIG_READABLE_ASM
> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index 2403a63..c8fd50f 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -1265,6 +1265,17 @@ config LATENCYTOP
>  source mm/Kconfig.debug
>  source kernel/trace/Kconfig
>  
> +config DISABLE_GCC_AUTOMATIC_INLINING
> +	bool "Disable gcc automatic inlining"
> +	depends on TRACING
> +	help
> +	  This option tells gcc he's not allowed to inline functions automatically,
> +	  when they are not marked as 'inline'.
> +	  In turn, this enables to trace an event with an accurate call site.
> +	  Of course, the resultant kernel is slower and slightly smaller.
> +
> +	  Select this option only if you want to trace call sites accurately.
> +
>  config PROVIDE_OHCI1394_DMA_INIT
>  	bool "Remote debugging over FireWire early on boot"
>  	depends on PCI && X86
> -- 
> 1.7.8.6
> 
> --
> 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] 27+ messages in thread

* Re: [PATCH 02/10] mm, slob: Use NUMA_NO_NODE instead of -1
  2012-09-08 20:47 ` [PATCH 02/10] mm, slob: Use NUMA_NO_NODE instead of -1 Ezequiel Garcia
@ 2012-09-09 21:27   ` David Rientjes
  2012-09-24 17:13     ` Ezequiel Garcia
  0 siblings, 1 reply; 27+ messages in thread
From: David Rientjes @ 2012-09-09 21:27 UTC (permalink / raw)
  To: Ezequiel Garcia; +Cc: linux-kernel, linux-mm, Pekka Enberg

On Sat, 8 Sep 2012, Ezequiel Garcia wrote:

> Cc: Pekka Enberg <penberg@kernel.org>
> Signed-off-by: Ezequiel Garcia <elezegarcia@gmail.com>

Acked-by: David Rientjes <rientjes@google.com>

Please cc Matt Mackall <mpm@selenic.com> on all slob patches in the 
future.

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

* Re: [PATCH 03/10] mm, slab: Remove silly function slab_buffer_size()
  2012-09-08 20:47 ` [PATCH 03/10] mm, slab: Remove silly function slab_buffer_size() Ezequiel Garcia
@ 2012-09-09 21:28   ` David Rientjes
  0 siblings, 0 replies; 27+ messages in thread
From: David Rientjes @ 2012-09-09 21:28 UTC (permalink / raw)
  To: Ezequiel Garcia; +Cc: linux-kernel, linux-mm, Pekka Enberg

On Sat, 8 Sep 2012, Ezequiel Garcia wrote:

> This function is seldom used, and can be simply replaced with cachep->size.
> 
> Cc: Pekka Enberg <penberg@kernel.org>
> Signed-off-by: Ezequiel Garcia <elezegarcia@gmail.com>

Acked-by: David Rientjes <rientjes@google.com>

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

* Re: [PATCH 01/10] Makefile: Add option CONFIG_DISABLE_GCC_AUTOMATIC_INLINING
  2012-09-09 21:25 ` David Rientjes
@ 2012-09-13  0:30   ` Ezequiel Garcia
  2012-09-13  7:17     ` Michal Marek
  0 siblings, 1 reply; 27+ messages in thread
From: Ezequiel Garcia @ 2012-09-13  0:30 UTC (permalink / raw)
  To: David Rientjes, sam; +Cc: linux-kernel, linux-mm, Michal Marek

Hi,

On Sun, Sep 9, 2012 at 6:25 PM, David Rientjes <rientjes@google.com> wrote:
> On Sat, 8 Sep 2012, Ezequiel Garcia wrote:
>
>> diff --git a/Makefile b/Makefile
>> index ddf5be9..df6045a 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -561,6 +561,10 @@ else
>>  KBUILD_CFLAGS        += -O2
>>  endif
>>
>> +ifdef CONFIG_DISABLE_GCC_AUTOMATIC_INLINING
>> +KBUILD_CFLAGS        += -fno-inline-small-functions
>
> This isn't the only option that controls automatic inlining of functions,
> see indirect-inlining, inline-functions, and inline-functions-called-once.
>

I'll check about this gcc options and re-send, renamed as:
CONFIG_DISABLE_CC_AUTOMATIC_INLINING

Thanks for both your feedback,
Ezequiel.

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

* Re: [PATCH 01/10] Makefile: Add option CONFIG_DISABLE_GCC_AUTOMATIC_INLINING
  2012-09-13  0:30   ` Ezequiel Garcia
@ 2012-09-13  7:17     ` Michal Marek
  2012-09-13  9:16       ` Ezequiel Garcia
  0 siblings, 1 reply; 27+ messages in thread
From: Michal Marek @ 2012-09-13  7:17 UTC (permalink / raw)
  To: Ezequiel Garcia; +Cc: David Rientjes, sam, linux-kernel, linux-mm

Dne 13.9.2012 02:30, Ezequiel Garcia napsal(a):
> Hi,
> 
> On Sun, Sep 9, 2012 at 6:25 PM, David Rientjes <rientjes@google.com> wrote:
>> On Sat, 8 Sep 2012, Ezequiel Garcia wrote:
>>
>>> diff --git a/Makefile b/Makefile
>>> index ddf5be9..df6045a 100644
>>> --- a/Makefile
>>> +++ b/Makefile
>>> @@ -561,6 +561,10 @@ else
>>>  KBUILD_CFLAGS        += -O2
>>>  endif
>>>
>>> +ifdef CONFIG_DISABLE_GCC_AUTOMATIC_INLINING
>>> +KBUILD_CFLAGS        += -fno-inline-small-functions
>>
>> This isn't the only option that controls automatic inlining of functions,
>> see indirect-inlining, inline-functions, and inline-functions-called-once.
>>
> 
> I'll check about this gcc options and re-send, renamed as:
> CONFIG_DISABLE_CC_AUTOMATIC_INLINING

Please name it CONFIG_CC_<something>

Michal


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

* Re: [PATCH 01/10] Makefile: Add option CONFIG_DISABLE_GCC_AUTOMATIC_INLINING
  2012-09-13  7:17     ` Michal Marek
@ 2012-09-13  9:16       ` Ezequiel Garcia
  0 siblings, 0 replies; 27+ messages in thread
From: Ezequiel Garcia @ 2012-09-13  9:16 UTC (permalink / raw)
  To: Michal Marek; +Cc: David Rientjes, sam, linux-kernel, linux-mm

On Thu, Sep 13, 2012 at 4:17 AM, Michal Marek <mmarek@suse.cz> wrote:
> Dne 13.9.2012 02:30, Ezequiel Garcia napsal(a):
>> Hi,
>>
>> On Sun, Sep 9, 2012 at 6:25 PM, David Rientjes <rientjes@google.com> wrote:
>>> On Sat, 8 Sep 2012, Ezequiel Garcia wrote:
>>>
>>>> diff --git a/Makefile b/Makefile
>>>> index ddf5be9..df6045a 100644
>>>> --- a/Makefile
>>>> +++ b/Makefile
>>>> @@ -561,6 +561,10 @@ else
>>>>  KBUILD_CFLAGS        += -O2
>>>>  endif
>>>>
>>>> +ifdef CONFIG_DISABLE_GCC_AUTOMATIC_INLINING
>>>> +KBUILD_CFLAGS        += -fno-inline-small-functions
>>>
>>> This isn't the only option that controls automatic inlining of functions,
>>> see indirect-inlining, inline-functions, and inline-functions-called-once.
>>>
>>
>> I'll check about this gcc options and re-send, renamed as:
>> CONFIG_DISABLE_CC_AUTOMATIC_INLINING
>
> Please name it CONFIG_CC_<something>
>

Yeah, that's what I meant, sorry.

Ezequiel.

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

* Re: [PATCH 02/10] mm, slob: Use NUMA_NO_NODE instead of -1
  2012-09-09 21:27   ` David Rientjes
@ 2012-09-24 17:13     ` Ezequiel Garcia
  2012-09-25  7:37       ` Pekka Enberg
  0 siblings, 1 reply; 27+ messages in thread
From: Ezequiel Garcia @ 2012-09-24 17:13 UTC (permalink / raw)
  To: Pekka Enberg; +Cc: linux-kernel, linux-mm, David Rientjes, Matt Mackall

Pekka,

On Sun, Sep 9, 2012 at 6:27 PM, David Rientjes <rientjes@google.com> wrote:
> On Sat, 8 Sep 2012, Ezequiel Garcia wrote:
>
>> Cc: Pekka Enberg <penberg@kernel.org>
>> Signed-off-by: Ezequiel Garcia <elezegarcia@gmail.com>
>
> Acked-by: David Rientjes <rientjes@google.com>
>

Will you pick this (and the rest of cleanup patches)
for v3.7 pull request?
Or is there anything for me to redo?

Thanks,
Ezequiel.

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

* Re: [PATCH 05/10] mm, util: Use dup_user to duplicate user memory
  2012-09-08 20:47 ` [PATCH 05/10] mm, util: Use dup_user to duplicate user memory Ezequiel Garcia
@ 2012-09-25  7:15   ` Pekka Enberg
  2012-09-25 21:29   ` Andrew Morton
  1 sibling, 0 replies; 27+ messages in thread
From: Pekka Enberg @ 2012-09-25  7:15 UTC (permalink / raw)
  To: Ezequiel Garcia
  Cc: linux-kernel, linux-mm, Andrew Morton, Christoph Lameter, David Rientjes

On Sat, Sep 8, 2012 at 11:47 PM, Ezequiel Garcia <elezegarcia@gmail.com> wrote:
> Previously the strndup_user allocation was being done through memdup_user,
> and the caller was wrongly traced as being strndup_user
> (the correct trace must report the caller of strndup_user).
>
> This is a common problem: in order to get accurate callsite tracing,
> a utils function can't allocate through another utils function,
> but instead do the allocation himself (or inlined).
>
> Here we fix this by creating an always inlined dup_user() function to
> performed the real allocation and to be used by memdup_user and strndup_user.
>
> Cc: Pekka Enberg <penberg@kernel.org>
> Signed-off-by: Ezequiel Garcia <elezegarcia@gmail.com>
> ---
>  mm/util.c |   11 ++++++++---
>  1 files changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/mm/util.c b/mm/util.c
> index dc3036c..48d3ff8b 100644
> --- a/mm/util.c
> +++ b/mm/util.c
> @@ -76,14 +76,14 @@ void *kmemdup(const void *src, size_t len, gfp_t gfp)
>  EXPORT_SYMBOL(kmemdup);
>
>  /**
> - * memdup_user - duplicate memory region from user space
> + * dup_user - duplicate memory region from user space
>   *
>   * @src: source address in user space
>   * @len: number of bytes to copy
>   *
>   * Returns an ERR_PTR() on failure.
>   */
> -void *memdup_user(const void __user *src, size_t len)
> +static __always_inline void *dup_user(const void __user *src, size_t len)
>  {
>         void *p;
>
> @@ -103,6 +103,11 @@ void *memdup_user(const void __user *src, size_t len)
>
>         return p;
>  }
> +
> +void *memdup_user(const void __user *src, size_t len)
> +{
> +       return dup_user(src, len);
> +}
>  EXPORT_SYMBOL(memdup_user);
>
>  static __always_inline void *__do_krealloc(const void *p, size_t new_size,
> @@ -214,7 +219,7 @@ char *strndup_user(const char __user *s, long n)
>         if (length > n)
>                 return ERR_PTR(-EINVAL);
>
> -       p = memdup_user(s, length);
> +       p = dup_user(s, length);
>
>         if (IS_ERR(p))
>                 return p;

Looks good to me. Andrew, do you want to pick this up?

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

* Re: [PATCH 02/10] mm, slob: Use NUMA_NO_NODE instead of -1
  2012-09-24 17:13     ` Ezequiel Garcia
@ 2012-09-25  7:37       ` Pekka Enberg
  2012-09-25 10:13         ` Ezequiel Garcia
  0 siblings, 1 reply; 27+ messages in thread
From: Pekka Enberg @ 2012-09-25  7:37 UTC (permalink / raw)
  To: Ezequiel Garcia
  Cc: linux-kernel, linux-mm, David Rientjes, Matt Mackall,
	Christoph Lameter, Andrew Morton

On Mon, Sep 24, 2012 at 8:13 PM, Ezequiel Garcia <elezegarcia@gmail.com> wrote:
> Will you pick this (and the rest of cleanup patches)
> for v3.7 pull request?
> Or is there anything for me to redo?

I merged all the patches except for the last one which conflicts with
the kmem/memcg changes.

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

* Re: [PATCH 02/10] mm, slob: Use NUMA_NO_NODE instead of -1
  2012-09-25  7:37       ` Pekka Enberg
@ 2012-09-25 10:13         ` Ezequiel Garcia
  0 siblings, 0 replies; 27+ messages in thread
From: Ezequiel Garcia @ 2012-09-25 10:13 UTC (permalink / raw)
  To: Pekka Enberg
  Cc: linux-kernel, linux-mm, David Rientjes, Matt Mackall,
	Christoph Lameter, Andrew Morton

On Tue, Sep 25, 2012 at 4:37 AM, Pekka Enberg <penberg@kernel.org> wrote:
> On Mon, Sep 24, 2012 at 8:13 PM, Ezequiel Garcia <elezegarcia@gmail.com> wrote:
>> Will you pick this (and the rest of cleanup patches)
>> for v3.7 pull request?
>> Or is there anything for me to redo?
>
> I merged all the patches except for the last one which conflicts with
> the kmem/memcg changes.

Ok, great. We can work on those later, after kmem/memcg is settled.

Thanks,
Ezequiel.

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

* [patch slab/next] mm, slob: fix build breakage in __kmalloc_node_track_caller
  2012-09-08 20:47 ` [PATCH 04/10] mm, slob: Add support for kmalloc_track_caller() Ezequiel Garcia
@ 2012-09-25 19:53   ` David Rientjes
  2012-09-25 19:55     ` Ezequiel Garcia
  0 siblings, 1 reply; 27+ messages in thread
From: David Rientjes @ 2012-09-25 19:53 UTC (permalink / raw)
  To: Ezequiel Garcia, Pekka Enberg; +Cc: linux-kernel, linux-mm

On Sat, 8 Sep 2012, Ezequiel Garcia wrote:

> @@ -454,15 +455,35 @@ void *__kmalloc_node(size_t size, gfp_t gfp, int node)
>  			gfp |= __GFP_COMP;
>  		ret = slob_new_pages(gfp, order, node);
>  
> -		trace_kmalloc_node(_RET_IP_, ret,
> +		trace_kmalloc_node(caller, ret,
>  				   size, PAGE_SIZE << order, gfp, node);
>  	}
>  
>  	kmemleak_alloc(ret, size, 1, gfp);
>  	return ret;
>  }
> +
> +void *__kmalloc_node(size_t size, gfp_t gfp, int node)
> +{
> +	return __do_kmalloc_node(size, gfp, node, _RET_IP_);
> +}
>  EXPORT_SYMBOL(__kmalloc_node);
>  
> +#ifdef CONFIG_TRACING
> +void *__kmalloc_track_caller(size_t size, gfp_t gfp, unsigned long caller)
> +{
> +	return __do_kmalloc_node(size, gfp, NUMA_NO_NODE, caller);
> +}
> +
> +#ifdef CONFIG_NUMA
> +void *__kmalloc_node_track_caller(size_t size, gfp_t gfpflags,
> +					int node, unsigned long caller)
> +{
> +	return __do_kmalloc_node(size, gfp, node, caller);
> +}
> +#endif

This breaks Pekka's slab/next tree with this:

mm/slob.c: In function '__kmalloc_node_track_caller':
mm/slob.c:488: error: 'gfp' undeclared (first use in this function)
mm/slob.c:488: error: (Each undeclared identifier is reported only once
mm/slob.c:488: error: for each function it appears in.)


mm, slob: fix build breakage in __kmalloc_node_track_caller

"mm, slob: Add support for kmalloc_track_caller()" breaks the build 
because gfp is undeclared.  Fix it.

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

diff --git a/mm/slob.c b/mm/slob.c
--- a/mm/slob.c
+++ b/mm/slob.c
@@ -482,7 +482,7 @@ void *__kmalloc_track_caller(size_t size, gfp_t gfp, unsigned long caller)
 }
 
 #ifdef CONFIG_NUMA
-void *__kmalloc_node_track_caller(size_t size, gfp_t gfpflags,
+void *__kmalloc_node_track_caller(size_t size, gfp_t gfp,
 					int node, unsigned long caller)
 {
 	return __do_kmalloc_node(size, gfp, node, caller);

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

* Re: [patch slab/next] mm, slob: fix build breakage in __kmalloc_node_track_caller
  2012-09-25 19:53   ` [patch slab/next] mm, slob: fix build breakage in __kmalloc_node_track_caller David Rientjes
@ 2012-09-25 19:55     ` Ezequiel Garcia
  0 siblings, 0 replies; 27+ messages in thread
From: Ezequiel Garcia @ 2012-09-25 19:55 UTC (permalink / raw)
  To: David Rientjes; +Cc: Pekka Enberg, linux-kernel, linux-mm

On Tue, Sep 25, 2012 at 4:53 PM, David Rientjes <rientjes@google.com> wrote:
> On Sat, 8 Sep 2012, Ezequiel Garcia wrote:
>
>> @@ -454,15 +455,35 @@ void *__kmalloc_node(size_t size, gfp_t gfp, int node)
>>                       gfp |= __GFP_COMP;
>>               ret = slob_new_pages(gfp, order, node);
>>
>> -             trace_kmalloc_node(_RET_IP_, ret,
>> +             trace_kmalloc_node(caller, ret,
>>                                  size, PAGE_SIZE << order, gfp, node);
>>       }
>>
>>       kmemleak_alloc(ret, size, 1, gfp);
>>       return ret;
>>  }
>> +
>> +void *__kmalloc_node(size_t size, gfp_t gfp, int node)
>> +{
>> +     return __do_kmalloc_node(size, gfp, node, _RET_IP_);
>> +}
>>  EXPORT_SYMBOL(__kmalloc_node);
>>
>> +#ifdef CONFIG_TRACING
>> +void *__kmalloc_track_caller(size_t size, gfp_t gfp, unsigned long caller)
>> +{
>> +     return __do_kmalloc_node(size, gfp, NUMA_NO_NODE, caller);
>> +}
>> +
>> +#ifdef CONFIG_NUMA
>> +void *__kmalloc_node_track_caller(size_t size, gfp_t gfpflags,
>> +                                     int node, unsigned long caller)
>> +{
>> +     return __do_kmalloc_node(size, gfp, node, caller);
>> +}
>> +#endif
>
> This breaks Pekka's slab/next tree with this:
>
> mm/slob.c: In function '__kmalloc_node_track_caller':
> mm/slob.c:488: error: 'gfp' undeclared (first use in this function)
> mm/slob.c:488: error: (Each undeclared identifier is reported only once
> mm/slob.c:488: error: for each function it appears in.)
>
>
> mm, slob: fix build breakage in __kmalloc_node_track_caller
>
> "mm, slob: Add support for kmalloc_track_caller()" breaks the build
> because gfp is undeclared.  Fix it.
>
> Signed-off-by: David Rientjes <rientjes@google.com>
> ---
>  mm/slob.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/mm/slob.c b/mm/slob.c
> --- a/mm/slob.c
> +++ b/mm/slob.c
> @@ -482,7 +482,7 @@ void *__kmalloc_track_caller(size_t size, gfp_t gfp, unsigned long caller)
>  }
>
>  #ifdef CONFIG_NUMA
> -void *__kmalloc_node_track_caller(size_t size, gfp_t gfpflags,
> +void *__kmalloc_node_track_caller(size_t size, gfp_t gfp,
>                                         int node, unsigned long caller)
>  {
>         return __do_kmalloc_node(size, gfp, node, caller);

Acked-by: Ezequiel Garcia <elezegarcia@gmail.com>

Thanks,
Ezequiel.

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

* Re: [PATCH 05/10] mm, util: Use dup_user to duplicate user memory
  2012-09-08 20:47 ` [PATCH 05/10] mm, util: Use dup_user to duplicate user memory Ezequiel Garcia
  2012-09-25  7:15   ` Pekka Enberg
@ 2012-09-25 21:29   ` Andrew Morton
  2012-09-26  1:15     ` Ezequiel Garcia
  1 sibling, 1 reply; 27+ messages in thread
From: Andrew Morton @ 2012-09-25 21:29 UTC (permalink / raw)
  To: Ezequiel Garcia; +Cc: linux-kernel, linux-mm, Pekka Enberg

On Sat,  8 Sep 2012 17:47:54 -0300
Ezequiel Garcia <elezegarcia@gmail.com> wrote:

> Previously the strndup_user allocation was being done through memdup_user,
> and the caller was wrongly traced as being strndup_user
> (the correct trace must report the caller of strndup_user).
> 
> This is a common problem: in order to get accurate callsite tracing,
> a utils function can't allocate through another utils function,
> but instead do the allocation himself (or inlined).
> 
> Here we fix this by creating an always inlined dup_user() function to
> performed the real allocation and to be used by memdup_user and strndup_user.

This patch increases util.o's text size by 238 bytes.  A larger kernel
with a worsened cache footprint.

And we did this to get marginally improved tracing output?  This sounds
like a bad tradeoff to me.


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

* Re: [PATCH 05/10] mm, util: Use dup_user to duplicate user memory
  2012-09-25 21:29   ` Andrew Morton
@ 2012-09-26  1:15     ` Ezequiel Garcia
  2012-09-26 21:42       ` Andrew Morton
  0 siblings, 1 reply; 27+ messages in thread
From: Ezequiel Garcia @ 2012-09-26  1:15 UTC (permalink / raw)
  To: Andrew Morton; +Cc: linux-kernel, linux-mm, Pekka Enberg

Hi Andrew,

On Tue, Sep 25, 2012 at 6:29 PM, Andrew Morton
<akpm@linux-foundation.org> wrote:
> On Sat,  8 Sep 2012 17:47:54 -0300
> Ezequiel Garcia <elezegarcia@gmail.com> wrote:
>
>> Previously the strndup_user allocation was being done through memdup_user,
>> and the caller was wrongly traced as being strndup_user
>> (the correct trace must report the caller of strndup_user).
>>
>> This is a common problem: in order to get accurate callsite tracing,
>> a utils function can't allocate through another utils function,
>> but instead do the allocation himself (or inlined).
>>
>> Here we fix this by creating an always inlined dup_user() function to
>> performed the real allocation and to be used by memdup_user and strndup_user.
>
> This patch increases util.o's text size by 238 bytes.  A larger kernel
> with a worsened cache footprint.
>
> And we did this to get marginally improved tracing output?  This sounds
> like a bad tradeoff to me.
>

Mmm, that's bad tradeoff indeed.
It's certainly odd since the patch shouldn't increase the text size
*that* much.
Is it too much to ask that you send your kernel config and gcc version.

My compilation (x86 kernel in gcc 4.7.1) shows a kernel less bloated:

$ readelf -s util-dup-user.o | grep dup_user
   161: 00001c10   108 FUNC    GLOBAL DEFAULT    1 memdup_user
   169: 00001df0   159 FUNC    GLOBAL DEFAULT    1 strndup_user
$ readelf -s util.o | grep dup_user
   161: 00001c10   108 FUNC    GLOBAL DEFAULT    1 memdup_user
   169: 00001df0    98 FUNC    GLOBAL DEFAULT    1 strndup_user

$ size util.o
   text	   data	    bss	    dec	    hex	filename
  18319	   2077	      0	  20396	   4fac	util.o
$ size util-dup-user.o
   text	   data	    bss	    dec	    hex	filename
  18367	   2077	      0	  20444	   4fdc	util-dup-user.o

Am I doing anything wrong?
If you still feel this is unnecessary bloatness, perhaps I could think of
something depending on CONFIG_TRACING (though I know
we all hate those nasty ifdefs).

Anyway, thanks for the review,
Ezequiel.

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

* Re: [PATCH 05/10] mm, util: Use dup_user to duplicate user memory
  2012-09-26  1:15     ` Ezequiel Garcia
@ 2012-09-26 21:42       ` Andrew Morton
  2012-09-26 21:51         ` Ezequiel Garcia
  0 siblings, 1 reply; 27+ messages in thread
From: Andrew Morton @ 2012-09-26 21:42 UTC (permalink / raw)
  To: Ezequiel Garcia; +Cc: linux-kernel, linux-mm, Pekka Enberg

On Tue, 25 Sep 2012 22:15:38 -0300
Ezequiel Garcia <elezegarcia@gmail.com> wrote:

> > This patch increases util.o's text size by 238 bytes.  A larger kernel
> > with a worsened cache footprint.
> >
> > And we did this to get marginally improved tracing output?  This sounds
> > like a bad tradeoff to me.
> >
> 
> Mmm, that's bad tradeoff indeed.
> It's certainly odd since the patch shouldn't increase the text size
> *that* much.
> Is it too much to ask that you send your kernel config and gcc version.

x86_64 allmodconfig with CONFIG_DEBUG_INFO=n,
CONFIG_ENABLE_MUST_CHECK=n. gcc-4.4.4.

> My compilation (x86 kernel in gcc 4.7.1) shows a kernel less bloated:
> 
> $ readelf -s util-dup-user.o | grep dup_user
>    161: 00001c10   108 FUNC    GLOBAL DEFAULT    1 memdup_user
>    169: 00001df0   159 FUNC    GLOBAL DEFAULT    1 strndup_user
> $ readelf -s util.o | grep dup_user
>    161: 00001c10   108 FUNC    GLOBAL DEFAULT    1 memdup_user
>    169: 00001df0    98 FUNC    GLOBAL DEFAULT    1 strndup_user
> 
> $ size util.o
>    text	   data	    bss	    dec	    hex	filename
>   18319	   2077	      0	  20396	   4fac	util.o
> $ size util-dup-user.o
>    text	   data	    bss	    dec	    hex	filename
>   18367	   2077	      0	  20444	   4fdc	util-dup-user.o
> 
> Am I doing anything wrong?

Dunno - it could be a config thing.

> If you still feel this is unnecessary bloatness, perhaps I could think of
> something depending on CONFIG_TRACING (though I know
> we all hate those nasty ifdefs).

hm.  Perhaps we could add an __always_inline_for_tracing.  But that
wouldn't help a lot - a CONFIG_TRACING_SUPPORT=y kernel would still be
impacted even if the user is never using tracing.


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

* Re: [PATCH 05/10] mm, util: Use dup_user to duplicate user memory
  2012-09-26 21:42       ` Andrew Morton
@ 2012-09-26 21:51         ` Ezequiel Garcia
  0 siblings, 0 replies; 27+ messages in thread
From: Ezequiel Garcia @ 2012-09-26 21:51 UTC (permalink / raw)
  To: Andrew Morton; +Cc: linux-kernel, linux-mm, Pekka Enberg

Andrew,

On Wed, Sep 26, 2012 at 6:42 PM, Andrew Morton
<akpm@linux-foundation.org> wrote:
> On Tue, 25 Sep 2012 22:15:38 -0300
> Ezequiel Garcia <elezegarcia@gmail.com> wrote:
>
>> > This patch increases util.o's text size by 238 bytes.  A larger kernel
>> > with a worsened cache footprint.
>> >
>> > And we did this to get marginally improved tracing output?  This sounds
>> > like a bad tradeoff to me.
>> >
>>
>> Mmm, that's bad tradeoff indeed.
>> It's certainly odd since the patch shouldn't increase the text size
>> *that* much.
>> Is it too much to ask that you send your kernel config and gcc version.
>
> x86_64 allmodconfig with CONFIG_DEBUG_INFO=n,
> CONFIG_ENABLE_MUST_CHECK=n. gcc-4.4.4.
>

I'll try that.


>> My compilation (x86 kernel in gcc 4.7.1) shows a kernel less bloated:
>>
>> $ readelf -s util-dup-user.o | grep dup_user
>>    161: 00001c10   108 FUNC    GLOBAL DEFAULT    1 memdup_user
>>    169: 00001df0   159 FUNC    GLOBAL DEFAULT    1 strndup_user
>> $ readelf -s util.o | grep dup_user
>>    161: 00001c10   108 FUNC    GLOBAL DEFAULT    1 memdup_user
>>    169: 00001df0    98 FUNC    GLOBAL DEFAULT    1 strndup_user
>>
>> $ size util.o
>>    text          data     bss     dec     hex filename
>>   18319          2077       0   20396    4fac util.o
>> $ size util-dup-user.o
>>    text          data     bss     dec     hex filename
>>   18367          2077       0   20444    4fdc util-dup-user.o
>>
>> Am I doing anything wrong?
>
> Dunno - it could be a config thing.
>

I'm kind of lost. The patch really shouldn't fatten the kernel this way :-(

The patch was meant to improve tracing for memory tracking,
which in turn would be used to reduce memory footprint.
So, definitely I don't want to increase kernel text size.

I'll test that kernel config and see what I can do.

Thanks,
Ezequiel.

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

end of thread, other threads:[~2012-09-26 21:51 UTC | newest]

Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-09-08 20:47 [PATCH 01/10] Makefile: Add option CONFIG_DISABLE_GCC_AUTOMATIC_INLINING Ezequiel Garcia
2012-09-08 20:47 ` [PATCH 02/10] mm, slob: Use NUMA_NO_NODE instead of -1 Ezequiel Garcia
2012-09-09 21:27   ` David Rientjes
2012-09-24 17:13     ` Ezequiel Garcia
2012-09-25  7:37       ` Pekka Enberg
2012-09-25 10:13         ` Ezequiel Garcia
2012-09-08 20:47 ` [PATCH 03/10] mm, slab: Remove silly function slab_buffer_size() Ezequiel Garcia
2012-09-09 21:28   ` David Rientjes
2012-09-08 20:47 ` [PATCH 04/10] mm, slob: Add support for kmalloc_track_caller() Ezequiel Garcia
2012-09-25 19:53   ` [patch slab/next] mm, slob: fix build breakage in __kmalloc_node_track_caller David Rientjes
2012-09-25 19:55     ` Ezequiel Garcia
2012-09-08 20:47 ` [PATCH 05/10] mm, util: Use dup_user to duplicate user memory Ezequiel Garcia
2012-09-25  7:15   ` Pekka Enberg
2012-09-25 21:29   ` Andrew Morton
2012-09-26  1:15     ` Ezequiel Garcia
2012-09-26 21:42       ` Andrew Morton
2012-09-26 21:51         ` Ezequiel Garcia
2012-09-08 20:47 ` [PATCH 06/10] mm, slab: Replace 'caller' type, void* -> unsigned long Ezequiel Garcia
2012-09-08 20:47 ` [PATCH 07/10] mm, slab: Match SLAB and SLUB kmem_cache_alloc_xxx_trace() prototype Ezequiel Garcia
2012-09-08 20:47 ` [PATCH 08/10] mm, slab: Rename __cache_alloc() -> slab_alloc() Ezequiel Garcia
2012-09-08 20:47 ` [PATCH 09/10] mm, slub: Rename slab_alloc() -> slab_alloc_node() to match SLAB Ezequiel Garcia
2012-09-08 20:47 ` [PATCH 10/10] mm: Factor SLAB and SLUB common code Ezequiel Garcia
2012-09-08 21:43 ` [PATCH 01/10] Makefile: Add option CONFIG_DISABLE_GCC_AUTOMATIC_INLINING Sam Ravnborg
2012-09-09 21:25 ` David Rientjes
2012-09-13  0:30   ` Ezequiel Garcia
2012-09-13  7:17     ` Michal Marek
2012-09-13  9:16       ` Ezequiel Garcia

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).