Re: [PATCH 3/5] SLUB: Replace __builtin_return_address(0) with _RET_IP_.

[Christoph Lameter <cl@linux-foundation.org>]

Eduard - Gabriel Munteanu wrote:
> On Tue, Aug 19, 2008 at 01:14:01PM -0500, Christoph Lameter wrote:
>> Eduard - Gabriel Munteanu wrote:
>>
>>>  void *kmem_cache_alloc(struct kmem_cache *s, gfp_t gfpflags)
>>>  {
>>> -	return slab_alloc(s, gfpflags, -1, __builtin_return_address(0));
>>> +	return slab_alloc(s, gfpflags, -1, (void *) _RET_IP_);
>>>  }
>> Could you get rid of the casts by changing the type of parameter of slab_alloc()?
> 
> I just looked at it and it isn't a trivial change. slab_alloc() calls
> other functions which expect a void ptr. Even if slab_alloc() were to
> take an unsigned long and then cast it to a void ptr, other functions do
> call slab_alloc() with void ptr arguments (so the casts would move
> there).
> 
> I'd rather have this merged as it is and change things later, so that
> kmemtrace gets some testing from Pekka and others. 
> 

Well maybe this patch will do it then:

Subject: slub: Use _RET_IP and use "unsigned long" for kernel text addresses

Use _RET_IP_ instead of buildint_return_address() and make slub use unsigned long
instead of void * for addresses.

Signed-off-by: Christoph Lameter <cl@linux-foundation.org>

---
 mm/slub.c |   46 +++++++++++++++++++++++-----------------------
 1 file changed, 23 insertions(+), 23 deletions(-)

Index: linux-2.6/mm/slub.c
===================================================================
--- linux-2.6.orig/mm/slub.c	2008-08-19 11:40:52.332357770 -0700
+++ linux-2.6/mm/slub.c	2008-08-19 11:52:17.479064425 -0700
@@ -177,7 +177,7 @@ static LIST_HEAD(slab_caches);
  * Tracking user of a slab.
  */
 struct track {
-	void *addr;		/* Called from address */
+	unsigned long addr;	/* Called from address */
 	int cpu;		/* Was running on cpu */
 	int pid;		/* Pid context */
 	unsigned long when;	/* When did the operation occur */
@@ -366,7 +366,7 @@ static struct track *get_track(struct km
 }

 static void set_track(struct kmem_cache *s, void *object,
-				enum track_item alloc, void *addr)
+				enum track_item alloc, unsigned long addr)
 {
 	struct track *p;

@@ -390,8 +390,8 @@ static void init_tracking(struct kmem_ca
 	if (!(s->flags & SLAB_STORE_USER))
 		return;

-	set_track(s, object, TRACK_FREE, NULL);
-	set_track(s, object, TRACK_ALLOC, NULL);
+	set_track(s, object, TRACK_FREE, 0L);
+	set_track(s, object, TRACK_ALLOC, 0L);
 }

 static void print_track(const char *s, struct track *t)
@@ -399,7 +399,7 @@ static void print_track(const char *s, s
 	if (!t->addr)
 		return;

-	printk(KERN_ERR "INFO: %s in %pS age=%lu cpu=%u pid=%d\n",
+	printk(KERN_ERR "INFO: %s in %lxS age=%lu cpu=%u pid=%d\n",
 		s, t->addr, jiffies - t->when, t->cpu, t->pid);
 }

@@ -865,7 +865,7 @@ static void setup_object_debug(struct km
 }

 static int alloc_debug_processing(struct kmem_cache *s, struct page *page,
-						void *object, void *addr)
+						void *object, unsigned long addr)
 {
 	if (!check_slab(s, page))
 		goto bad;
@@ -905,7 +905,7 @@ bad:
 }

 static int free_debug_processing(struct kmem_cache *s, struct page *page,
-						void *object, void *addr)
+						void *object, unsigned long addr)
 {
 	if (!check_slab(s, page))
 		goto fail;
@@ -1028,10 +1028,10 @@ static inline void setup_object_debug(st
 			struct page *page, void *object) {}

 static inline int alloc_debug_processing(struct kmem_cache *s,
-	struct page *page, void *object, void *addr) { return 0; }
+	struct page *page, void *object, unsigned long addr) { return 0; }

 static inline int free_debug_processing(struct kmem_cache *s,
-	struct page *page, void *object, void *addr) { return 0; }
+	struct page *page, void *object, unsigned long) { return 0; }

 static inline int slab_pad_check(struct kmem_cache *s, struct page *page)
 			{ return 1; }
@@ -1499,7 +1499,7 @@ static inline int node_match(struct kmem
  * a call to the page allocator and the setup of a new slab.
  */
 static void *__slab_alloc(struct kmem_cache *s,
-		gfp_t gfpflags, int node, void *addr, struct kmem_cache_cpu *c)
+		gfp_t gfpflags, int node, unsigned long addr, struct kmem_cache_cpu *c)
 {
 	void **object;
 	struct page *new;
@@ -1583,7 +1583,7 @@ debug:
  * Otherwise we can simply pick the next object from the lockless free list.
  */
 static __always_inline void *slab_alloc(struct kmem_cache *s,
-		gfp_t gfpflags, int node, void *addr)
+		gfp_t gfpflags, int node, unsigned long addr)
 {
 	void **object;
 	struct kmem_cache_cpu *c;
@@ -1612,14 +1612,14 @@ static __always_inline void *slab_alloc(

 void *kmem_cache_alloc(struct kmem_cache *s, gfp_t gfpflags)
 {
-	return slab_alloc(s, gfpflags, -1, __builtin_return_address(0));
+	return slab_alloc(s, gfpflags, -1, _RET_IP_);
 }
 EXPORT_SYMBOL(kmem_cache_alloc);

 #ifdef CONFIG_NUMA
 void *kmem_cache_alloc_node(struct kmem_cache *s, gfp_t gfpflags, int node)
 {
-	return slab_alloc(s, gfpflags, node, __builtin_return_address(0));
+	return slab_alloc(s, gfpflags, node, _RET_IP_);
 }
 EXPORT_SYMBOL(kmem_cache_alloc_node);
 #endif
@@ -1633,7 +1633,7 @@ EXPORT_SYMBOL(kmem_cache_alloc_node);
  * handling required then we can return immediately.
  */
 static void __slab_free(struct kmem_cache *s, struct page *page,
-				void *x, void *addr, unsigned int offset)
+				void *x, unsigned long addr, unsigned int offset)
 {
 	void *prior;
 	void **object = (void *)x;
@@ -1703,7 +1703,7 @@ debug:
  * with all sorts of special processing.
  */
 static __always_inline void slab_free(struct kmem_cache *s,
-			struct page *page, void *x, void *addr)
+			struct page *page, void *x, unsigned long addr)
 {
 	void **object = (void *)x;
 	struct kmem_cache_cpu *c;
@@ -1730,7 +1730,7 @@ void kmem_cache_free(struct kmem_cache *

 	page = virt_to_head_page(x);

-	slab_free(s, page, x, __builtin_return_address(0));
+	slab_free(s, page, x, _RET_IP_);
 }
 EXPORT_SYMBOL(kmem_cache_free);

@@ -2657,7 +2657,7 @@ void *__kmalloc(size_t size, gfp_t flags
 	if (unlikely(ZERO_OR_NULL_PTR(s)))
 		return s;

-	return slab_alloc(s, flags, -1, __builtin_return_address(0));
+	return slab_alloc(s, flags, -1, _RET_IP_);
 }
 EXPORT_SYMBOL(__kmalloc);

@@ -2685,7 +2685,7 @@ void *__kmalloc_node(size_t size, gfp_t
 	if (unlikely(ZERO_OR_NULL_PTR(s)))
 		return s;

-	return slab_alloc(s, flags, node, __builtin_return_address(0));
+	return slab_alloc(s, flags, node, _RET_IP_);
 }
 EXPORT_SYMBOL(__kmalloc_node);
 #endif
@@ -2742,7 +2742,7 @@ void kfree(const void *x)
 		put_page(page);
 		return;
 	}
-	slab_free(page->slab, page, object, __builtin_return_address(0));
+	slab_free(page->slab, page, object, _RET_IP_);
 }
 EXPORT_SYMBOL(kfree);

@@ -3210,7 +3210,7 @@ void *__kmalloc_track_caller(size_t size
 	if (unlikely(ZERO_OR_NULL_PTR(s)))
 		return s;

-	return slab_alloc(s, gfpflags, -1, caller);
+	return slab_alloc(s, gfpflags, -1, (unsigned long)caller);
 }

 void *__kmalloc_node_track_caller(size_t size, gfp_t gfpflags,
@@ -3226,7 +3226,7 @@ void *__kmalloc_node_track_caller(size_t
 	if (unlikely(ZERO_OR_NULL_PTR(s)))
 		return s;

-	return slab_alloc(s, gfpflags, node, caller);
+	return slab_alloc(s, gfpflags, node, (unsigned long)caller);
 }

 #ifdef CONFIG_SLUB_DEBUG
@@ -3425,7 +3425,7 @@ static void resiliency_test(void) {};

 struct location {
 	unsigned long count;
-	void *addr;
+	unsigned long addr;
 	long long sum_time;
 	long min_time;
 	long max_time;
@@ -3473,7 +3473,7 @@ static int add_location(struct loc_track
 {
 	long start, end, pos;
 	struct location *l;
-	void *caddr;
+	unsigned long caddr;
 	unsigned long age = jiffies - track->when;

 	start = -1;