SLUB memory debugging improvements.

mm/slub.c +11 -1

+#ifdef CONFIG_STACKTRACE
+	unsigned long addrs[TRACK_ADDRS_COUNT];	/* Called from address */
+#endif
 	int cpu;		/* Was running on cpu */
 	int pid;		/* Pid context */
 	unsigned long when;	/* When did the operation occur */

@@ -420,7 +424,25 @@ static void set_track(struct kmem_cache *s, void *object,
 	struct track *p = get_track(s, object, alloc);
 
 	if (addr) {
-		p->addr = addr;
+#ifdef CONFIG_STACKTRACE
+		struct stack_trace trace;
+		int i;
+
+		trace.nr_entries = 0;
+		trace.max_entries = TRACK_ADDRS_COUNT;
+		trace.entries = p->addrs;
+		trace.skip = 3;

mm/slub.c static LIST_HEAD(slab_caches); · line 196

Quoted 12 lines · On Tue, 28 Jun 2011, [email protected] wrote:

mm/slub.c +2 -1

index 35f351f..3477ce5 100644
@@ -191,8 +191,12 @@ static LIST_HEAD(slab_caches);
  /*
   * Tracking user of a slab.
   */
+#define TRACK_ADDRS_COUNT 16
  struct track {
-	unsigned long addr;	/* Called from address */
+	unsigned long caddr;

Keep the name the same since its role does not change. That makes the patch touch less code.

Quoted 24 lines

mm/slub.c +16 -1

+#ifdef CONFIG_STACKTRACE
+	unsigned long addrs[TRACK_ADDRS_COUNT];	/* Called from address */
+#endif
  	int cpu;		/* Was running on cpu */
  	int pid;		/* Pid context */
  	unsigned long when;	/* When did the operation occur */

@@ -420,7 +424,25 @@ static void set_track(struct kmem_cache *s, void *object,
  	struct track *p = get_track(s, object, alloc);
 
  	if (addr) {
-		p->addr = addr;
+#ifdef CONFIG_STACKTRACE
+		struct stack_trace trace;
+		int i;
+
+		trace.nr_entries = 0;
+		trace.max_entries = TRACK_ADDRS_COUNT;
+		trace.entries = p->addrs;
+		trace.skip = 3;
+		save_stack_trace(&trace);
+
+		/* See rant in lockdep.c */
+		if (trace.nr_entries != 0&&
+		    trace.entries[trace.nr_entries - 1] == ULONG_MAX)

mm/slub.c static void set_track(struct kmem_cache *s, void *object, · line 444

Quoted 5 lines

mm/slub.c +5 -0

+			trace.nr_entries--;
+
+		for (i = trace.nr_entries; i<  TRACK_ADDRS_COUNT; i++)
+			p->addrs[i] = 0;
+#endif

Hmmm... Is this really working on all architectures? I remember from years past that we had issues with adding the same functionality to slab.

Otherwise the patch is quite straightforward (and it will be much cleaner if you do not change the name of "addr").

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

include/linux/slab.h +3 -0

index ad4dd1c..916ce0f 100644
@@ -302,6 +302,19 @@ extern void *__kmalloc_node_track_caller(size_t, gfp_t, int, unsigned long);
 
 #endif /* CONFIG_NUMA */
 
+/**
+ * Calling this on allocated memory will check that the memory
+ * is expected to be in use, and print warnings if not.

mm/slub.c +9 -0

index 3477ce5..f64caca 100644
@@ -2958,6 +2958,42 @@ size_t ksize(const void *object)
 }
 EXPORT_SYMBOL(ksize);
 
+#ifdef CONFIG_SLUB_DEBUG
+bool verify_mem_not_deleted(const void *x)
+{
+	struct page *page;
+	void *object = (void *)x;
+	unsigned long flags;
+	bool rv;
+
+	if (unlikely(ZERO_OR_NULL_PTR(x)))

Quoted 8 lines · On Tue, 28 Jun 2011, [email protected] wrote:

include/linux/slab.h +0 -0

index ad4dd1c..916ce0f 100644
@@ -302,6 +302,19 @@ extern void *__kmalloc_node_track_caller(size_t, gfp_t, int, unsigned long);
 
  #endif /* CONFIG_NUMA */
 

include/linux/slab.h extern void *__kmalloc_node_track_caller(size_t, gfp_t, int, unsigned long); · line 317

Quoted 13 lines

include/linux/slab.h +13 -0

+/**
+ * Calling this on allocated memory will check that the memory
+ * is expected to be in use, and print warnings if not.
+ */
+#if defined(CONFIG_SLUB)&&  defined(CONFIG_SLUB_DEBUG)
+extern bool verify_mem_not_deleted(const void *x);
+#else
+static inline bool verify_mem_not_deleted(const void *x)
+{
+	return true;
+}
+#endif
+

Why is this in slab.h and not slub_def.h?

Quoted 23 lines

mm/slub.c +14 -0

index 3477ce5..f64caca 100644
@@ -2958,6 +2958,42 @@ size_t ksize(const void *object)
  }
  EXPORT_SYMBOL(ksize);
 
+#ifdef CONFIG_SLUB_DEBUG
+bool verify_mem_not_deleted(const void *x)
+{
+	struct page *page;
+	void *object = (void *)x;
+	unsigned long flags;
+	bool rv;
+
+	if (unlikely(ZERO_OR_NULL_PTR(x)))
+		return false;
+
+	local_irq_save(flags);
+
+	page = virt_to_head_page(x);

mm/slub.c size_t ksize(const void *object) · line 2979

Quoted 5 lines

mm/slub.c +5 -0

+	if (unlikely(!PageSlab(page))) {
+		/* maybe it was from stack? */
+		rv = true;
+		goto out_unlock;
+	}

The above check is problematic for slabs that have SLAB_DESTROY_BY_RCU set. PageSlab(page) may be true to the next rcu interval.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Quoted 14 lines

mm/slub.c +14 -0

+#ifdef CONFIG_SLUB_DEBUG
+bool verify_mem_not_deleted(const void *x)
+{
+	struct page *page;
+	void *object = (void *)x;
+	unsigned long flags;
+	bool rv;
+
+	if (unlikely(ZERO_OR_NULL_PTR(x)))
+		return false;
+
+	local_irq_save(flags);
+
+	page = virt_to_head_page(x);

mm/slub.c size_t ksize(const void *object) · line 2979

Quoted 5 lines

mm/slub.c +5 -0

+	if (unlikely(!PageSlab(page))) {
+		/* maybe it was from stack? */
+		rv = true;
+		goto out_unlock;
+	}

The above check is problematic for slabs that have SLAB_DESTROY_BY_RCU set. PageSlab(page) may be true to the next rcu interval.