Skip to contentSkip to conversation

object debugging infrastructure V2

PATCH In review Thomas Gleixner 4 participants 17 messages Mar 5, 2008 – Mar 21, 2008

Subsystems (9)
lore
Share & tools
lore
#0 lore raw

cover object debugging infrastructure V2

This is version 2 of the object debugging infrastructure. Changes versus V1:

- review comments processed:
- multiplexing interfaces replaced
- object type is determined by a caller provided pointer to a debug structure which replaces the object type identifier -> No changes to the core code for new users
- new debug state "destroyed" (kobject wishlist)
- various cleanups

- complete detachment of the tracker object from the real object (no change to the real objects data structure necessary anymore)

- improved robustness (object memory management, oom cleanup)

- full set of self tests

- debugfs statistics

- docbook documentation

Thanks,

tglx

#1 lore raw

PATCH 1/5 vmalloc: do not check for freed locks on user maps

User maps do not contain kernel internal objects. No need to check them.

---
 mm/vmalloc.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
mm/vmalloc.c +2 -1
Index: linux-2.6/mm/vmalloc.c
===================================================================
@@ -382,7 +382,8 @@ static void __vunmap(const void *addr, i
 		return;
 	}
 
-	debug_check_no_locks_freed(addr, area->size);
+	if (!(area->flags & VM_USERMAP))
+		debug_check_no_locks_freed(addr, area->size);
 
 	if (deallocate_pages) {
 		int i;
#2 lore raw

Re: [patch 1/5] vmalloc: do not check for freed locks on user maps

Quoted 2 lines · Thomas Gleixner

User maps do not contain kernel internal objects. No need to check them.

Why not? Depends on your definition of kernel internal... and objects ;)

Drivers could create and manage some objects in this vmalloc area. They are no longer internal if you map them to userspace, but I still don't think you want to vunmap it until those object lifetimes are finished.

Quoted 20 lines
---
 mm/vmalloc.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Index: linux-2.6/mm/vmalloc.c
===================================================================

mm/vmalloc.c +2 -1
@@ -382,7 +382,8 @@ static void __vunmap(const void *addr, i
 		return;
 	}
 
-	debug_check_no_locks_freed(addr, area->size);
+	if (!(area->flags & VM_USERMAP))
+		debug_check_no_locks_freed(addr, area->size);
 
 	if (deallocate_pages) {
 		int i;
#3 lore raw

Re: [patch 1/5] vmalloc: do not check for freed locks on user maps

Quoted 11 lines · Nick Piggin
Quoted 2 lines · Thomas Gleixner

User maps do not contain kernel internal objects. No need to check them.

Why not? Depends on your definition of kernel internal... and objects ;)

Drivers could create and manage some objects in this vmalloc area. They are no longer internal if you map them to userspace, but I still don't think you want to vunmap it until those object lifetimes are finished.

Well, in case of the locks I have a hard time to figure out how you use a spinlock/mutex with a user space address. The same applies for timers or other objects used by kernel subsystems. So when the driver writer creates an kernel related object in the vmalloc space, he has to use the kernel mapping which is unmapped separate, right ?

I can see your concern about the infinite stu^H^H^Hcreativity of driver writers, but I prefer not to go down that road and provide debug infrastructure for absurdities.

Thanks,
tglx

Quoted 21 lines
Quoted 20 lines
---
 mm/vmalloc.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Index: linux-2.6/mm/vmalloc.c
===================================================================

mm/vmalloc.c +2 -1
@@ -382,7 +382,8 @@ static void __vunmap(const void *addr, i
 		return;
 	}
 
-	debug_check_no_locks_freed(addr, area->size);
+	if (!(area->flags & VM_USERMAP))
+		debug_check_no_locks_freed(addr, area->size);
 
 	if (deallocate_pages) {
 		int i;
#4 lore raw

Re: [patch 1/5] vmalloc: do not check for freed locks on user maps

Quoted 13 lines · Thomas Gleixner
Quoted 11 lines · Nick Piggin
Quoted 2 lines · Thomas Gleixner

User maps do not contain kernel internal objects. No need to check them.

Why not? Depends on your definition of kernel internal... and objects ;)

Drivers could create and manage some objects in this vmalloc area. They are no longer internal if you map them to userspace, but I still don't think you want to vunmap it until those object lifetimes are finished.

Quoted 5 lines

Well, in case of the locks I have a hard time to figure out how you use a spinlock/mutex with a user space address. The same applies for timers or other objects used by kernel subsystems. So when the driver writer creates an kernel related object in the vmalloc space, he has to use the kernel mapping which is unmapped separate, right ?

This is the kernel mapping. The user mapping is unmapped when the userspace munmaps.

Quoted 3 lines

I can see your concern about the infinite stu^H^H^Hcreativity of driver writers, but I prefer not to go down that road and provide debug infrastructure for absurdities.

#5 lore raw

Re: [patch 1/5] vmalloc: do not check for freed locks on user maps

Quoted 14 lines · Nick Piggin
Quoted 13 lines · Thomas Gleixner
Quoted 11 lines · Nick Piggin
Quoted 2 lines · Thomas Gleixner

User maps do not contain kernel internal objects. No need to check them.

Why not? Depends on your definition of kernel internal... and objects ;)

Drivers could create and manage some objects in this vmalloc area. They are no longer internal if you map them to userspace, but I still don't think you want to vunmap it until those object lifetimes are finished.

Quoted 8 lines
Quoted 5 lines

Well, in case of the locks I have a hard time to figure out how you use a spinlock/mutex with a user space address. The same applies for timers or other objects used by kernel subsystems. So when the driver writer creates an kernel related object in the vmalloc space, he has to use the kernel mapping which is unmapped separate, right ?

This is the kernel mapping. The user mapping is unmapped when the userspace munmaps.

Ok, my misinterpretation of that flag. Is the user space unmap in the same code path ? If yes, how can it be distinguished from the kernel space unmap ?

Thanks,

tglx

#6 lore raw

Re: [patch 1/5] vmalloc: do not check for freed locks on user maps

Quoted 8 lines · Thomas Gleixner
Quoted 2 lines · Nick Piggin

This is the kernel mapping. The user mapping is unmapped when the userspace munmaps.

Ok, my misinterpretation of that flag. Is the user space unmap in the same code path ? If yes, how can it be distinguished from the kernel space unmap ?

The flag I added so that the vmap can be passed to remap_vmalloc_range. There were a couple of places at one point that were not zeroing out the vmalloced memory correctly, so this just helps ensure we don't leak data.

Userspace unmap is unmap_vmas and related code in mm/memory.c.

#7 lore raw

PATCH 2/5 slab: add a flag to prevent debug_free checks on a kmem_cache

In reply to Thomas Gleixner

This is a preperatory patch for the debugobjects infrastructure. The flag prevents debug_free checks on kmem_caches. This is necessary to avoid resursive calls into a debug mechanism which uses a kmem_cache itself.

---
 include/linux/slab.h |    7 +++++++
 1 file changed, 7 insertions(+)
include/linux/slab.h +7 -0
Index: linux-2.6/include/linux/slab.h
===================================================================
@@ -29,6 +29,13 @@
 #define SLAB_MEM_SPREAD		0x00100000UL	/* Spread some memory over cpuset */
 #define SLAB_TRACE		0x00200000UL	/* Trace allocations and frees */
 
+/* Flag to prevent checks on free */
+#ifdef CONFIG_DEBUG_OBJECTS
+# define SLAB_DEBUG_OBJECTS	0x00400000UL
+#else
+# define SLAB_DEBUG_OBJECTS	0x00000000UL
+#endif
+
 /* The following flags affect the page allocator grouping pages by mobility */
 #define SLAB_RECLAIM_ACCOUNT	0x00020000UL		/* Objects are reclaimable */
 #define SLAB_TEMPORARY		SLAB_RECLAIM_ACCOUNT	/* Objects are short-lived */
#8 lore raw

PATCH 3/5 infrastructure to debug (dynamic) objects

In reply to Thomas Gleixner

We can see an ever repeating problem pattern with objects of any kind in the kernel:

1) freeing of active objects
2) reinitialization of active objects

Both problems can be hard to debug because the crash happens at a point where we have no chance to decode the root cause anymore. One problem spot are kernel timers, where the detection of the problem often happens in interrupt context and usually causes the machine to panic.

While working on a timer related bug report I had to hack specialized code into the timer subsystem to get a reasonable hint for the root cause. This debug hack was fine for temporary use, but far from a mergeable solution due to the intrusiveness into the timer code.

The code further lacked the ability to detect and report the root cause instantly and keep the system operational.

Keeping the system operational is important to get hold of the debug information without special debugging aids like serial consoles and special knowledge of the bug reporter.

The problems described above are not restricted to timers, but timers tend to expose it usually in a full system crash. Other objects are less explosive, but the symptoms caused by such mistakes can be even harder to debug.

Instead of creating specialized debugging code for the timer subsystem a generic infrastructure is created which allows developers to verify their code and provides an easy to enable debug facility for users in case of trouble.

The debugobjects core code keeps track of operations on static and dynamic objects by inserting them into a hashed list and sanity checking them on object operations and provides additional checks whenever kernel memory is freed.

The tracked object operations are:
- initializing an object
- adding an object to a subsystem list
- deleting an object from a subsystem list

Each operation is sanity checked before the operation is executed and the subsystem specific code can provide a fixup function which allows to prevent the damage of the operation. When the sanity check triggers a warning message and a stack trace is printed.

The list of operations can be extended if the need arises. For now it's limited to the requirements of the first user (timers).

The core code enqueues the objects into hash buckets. The hash index is generated from the address of the object to simplify the lookup for the check on k/vfree. Each bucket has it's own spinlock to avoid contention on a global lock.

The debug code can be compiled in without being active. The runtime overhead is minimal and could be optimized by asm alternatives. A kernel command line option enables the debugging code.

Thanks to Ingo Molnar for review, suggestions and cleanup patches.

---
 Documentation/kernel-parameters.txt |    2 
 include/linux/debugobjects.h        |   86 +++
 init/main.c                         |    3 
 lib/Kconfig.debug                   |   23 
 lib/Makefile                        |    1 
 lib/debugobjects.c                  |  844 ++++++++++++++++++++++++++++++++++++
 mm/page_alloc.c                     |   10 
 mm/slab.c                           |   10 
 mm/slub.c                           |    3 
 mm/vmalloc.c                        |    5 
 10 files changed, 982 insertions(+), 5 deletions(-)
Documentation/kernel-parameters.txt +2 -0
Index: linux-2.6/Documentation/kernel-parameters.txt
===================================================================
@@ -554,6 +554,8 @@ and is between 256 and 4096 characters. 
 			1 will print _a lot_ more information - normally
 			only useful to kernel developers.
 
+	debug_objects	[KNL] Enable object debugging
+
 	decnet.addr=	[HW,NET]
 			Format: <area>[,<node>]
 			See also Documentation/networking/decnet.txt.
include/linux/debugobjects.h new file +86 -0

86 lines not on this page · raw message

init/main.c +3 -0
Index: linux-2.6/init/main.c
===================================================================
@@ -52,6 +52,7 @@
 #include <linux/unwind.h>
 #include <linux/buffer_head.h>
 #include <linux/debug_locks.h>
+#include <linux/debugobjects.h>
 #include <linux/lockdep.h>
 #include <linux/pid_namespace.h>
 #include <linux/device.h>
@@ -523,6 +524,7 @@ asmlinkage void __init start_kernel(void
 	 */
 	unwind_init();
 	lockdep_init();
+	debug_objects_early_init();
 	cgroup_init_early();
 
 	local_irq_disable();
@@ -616,6 +618,7 @@ asmlinkage void __init start_kernel(void
 	enable_debug_pagealloc();
 	cpu_hotplug_init();
 	kmem_cache_init();
+	debug_objects_mem_init();
 	setup_per_cpu_pageset();
 	numa_policy_init();
 	if (late_time_init)
lib/Kconfig.debug +23 -0
Index: linux-2.6/lib/Kconfig.debug
===================================================================
@@ -183,6 +183,29 @@ config TIMER_STATS
 	  (it defaults to deactivated on bootup and will only be activated
 	  if some application like powertop activates it explicitly).
 
+config DEBUG_OBJECTS
+	bool "Debug object operations"
+	depends on DEBUG_KERNEL
+	help
+	  If you say Y here, additional code will be inserted into the
+	  kernel to track the life time of various objects and validate
+	  the operations on those objects.
+
+config DEBUG_OBJECTS_SELFTEST
+	bool "Debug objects selftest"
+	depends on DEBUG_OBJECTS
+	help
+	  This enables the selftest of the object debug code.
+
+config DEBUG_OBJECTS_FREE
+	bool "Debug objects in freed memory"
+	depends on DEBUG_OBJECTS
+	help
+	  This enables checks whether a k/v free operation frees an area
+	  which contains an object which has not been deactivated
+	  properly. This can make kmalloc/kfree-intensive workloads
+	  much slower.
+
 config DEBUG_SLAB
 	bool "Debug slab memory allocations"
 	depends on DEBUG_KERNEL && SLAB
lib/Makefile +1 -0
Index: linux-2.6/lib/Makefile
===================================================================
@@ -36,6 +36,7 @@ obj-$(CONFIG_LOCK_KERNEL) += kernel_lock
 obj-$(CONFIG_PLIST) += plist.o
 obj-$(CONFIG_DEBUG_PREEMPT) += smp_processor_id.o
 obj-$(CONFIG_DEBUG_LIST) += list_debug.o
+obj-$(CONFIG_DEBUG_OBJECTS) += debugobjects.o
 
 ifneq ($(CONFIG_HAVE_DEC_LOCK),y)
   lib-y += dec_and_lock.o
lib/debugobjects.c new file +844 -0

844 lines not on this page · raw message

mm/page_alloc.c +8 -2
Index: linux-2.6/mm/page_alloc.c
===================================================================
@@ -45,6 +45,7 @@
 #include <linux/fault-inject.h>
 #include <linux/page-isolation.h>
 #include <linux/memcontrol.h>
+#include <linux/debugobjects.h>
 
 #include <asm/tlbflush.h>
 #include <asm/div64.h>
@@ -532,8 +533,11 @@ static void __free_pages_ok(struct page 
 	if (reserved)
 		return;
 
-	if (!PageHighMem(page))
+	if (!PageHighMem(page)) {
 		debug_check_no_locks_freed(page_address(page),PAGE_SIZE<<order);
+		debug_check_no_obj_freed(page_address(page),
+					   PAGE_SIZE << order);
+	}
 	arch_free_page(page, order);
 	kernel_map_pages(page, 1 << order, 0);
 
@@ -995,8 +999,10 @@ static void free_hot_cold_page(struct pa
 	if (free_pages_check(page))
 		return;
 
-	if (!PageHighMem(page))
+	if (!PageHighMem(page)) {
 		debug_check_no_locks_freed(page_address(page), PAGE_SIZE);
+		debug_check_no_obj_freed(page_address(page), PAGE_SIZE);
+	}
 	arch_free_page(page, 0);
 	kernel_map_pages(page, 1, 0);
 
mm/slab.c +8 -2
Index: linux-2.6/mm/slab.c
===================================================================
@@ -110,6 +110,7 @@
 #include	<linux/fault-inject.h>
 #include	<linux/rtmutex.h>
 #include	<linux/reciprocal_div.h>
+#include	<linux/debugobjects.h>
 
 #include	<asm/cacheflush.h>
 #include	<asm/tlbflush.h>
@@ -178,12 +179,14 @@
 			 SLAB_CACHE_DMA | \
 			 SLAB_STORE_USER | \
 			 SLAB_RECLAIM_ACCOUNT | SLAB_PANIC | \
-			 SLAB_DESTROY_BY_RCU | SLAB_MEM_SPREAD)
+			 SLAB_DESTROY_BY_RCU | SLAB_MEM_SPREAD | \
+			 SLAB_DEBUG_OBJECTS)
 #else
 # define CREATE_MASK	(SLAB_HWCACHE_ALIGN | \
 			 SLAB_CACHE_DMA | \
 			 SLAB_RECLAIM_ACCOUNT | SLAB_PANIC | \
-			 SLAB_DESTROY_BY_RCU | SLAB_MEM_SPREAD)
+			 SLAB_DESTROY_BY_RCU | SLAB_MEM_SPREAD | \
+			 SLAB_DEBUG_OBJECTS)
 #endif
 
 /*
@@ -3766,6 +3769,8 @@ void kmem_cache_free(struct kmem_cache *
 
 	local_irq_save(flags);
 	debug_check_no_locks_freed(objp, obj_size(cachep));
+	if (!(cachep->flags & SLAB_DEBUG_OBJECTS))
+		debug_check_no_obj_freed(objp, obj_size(cachep));
 	__cache_free(cachep, objp);
 	local_irq_restore(flags);
 }
@@ -3791,6 +3796,7 @@ void kfree(const void *objp)
 	kfree_debugcheck(objp);
 	c = virt_to_cache(objp);
 	debug_check_no_locks_freed(objp, obj_size(c));
+	debug_check_no_obj_freed(objp, obj_size(c));
 	__cache_free(c, (void *)objp);
 	local_irq_restore(flags);
 }
mm/slub.c +3 -0
Index: linux-2.6/mm/slub.c
===================================================================
@@ -21,6 +21,7 @@
 #include <linux/ctype.h>
 #include <linux/kallsyms.h>
 #include <linux/memory.h>
+#include <linux/debugobjects.h>
 
 /*
  * Lock order:
@@ -1688,6 +1689,8 @@ static __always_inline void slab_free(st
 	local_irq_save(flags);
 	c = get_cpu_slab(s, smp_processor_id());
 	debug_check_no_locks_freed(object, c->objsize);
+	if (!(s->flags & SLAB_DEBUG_OBJECTS))
+		debug_check_no_obj_freed(object, s->objsize);
 	if (likely(page == c->page && c->node >= 0)) {
 		object[c->offset] = c->freelist;
 		c->freelist = object;
mm/vmalloc.c +4 -1
Index: linux-2.6/mm/vmalloc.c
===================================================================
@@ -14,6 +14,7 @@
 #include <linux/slab.h>
 #include <linux/spinlock.h>
 #include <linux/interrupt.h>
+#include <linux/debugobjects.h>
 
 #include <linux/vmalloc.h>
 
@@ -382,8 +383,10 @@ static void __vunmap(const void *addr, i
 		return;
 	}
 
-	if (!(area->flags & VM_USERMAP))
+	if (!(area->flags & VM_USERMAP)) {
 		debug_check_no_locks_freed(addr, area->size);
+		debug_check_no_obj_freed(addr, area->size);
+	}
 
 	if (deallocate_pages) {
 		int i;
#9 lore raw

PATCH 4/5 debugobjects: add documentation

In reply to Thomas Gleixner

Add a DocBook for debugobjects.

---
 Documentation/DocBook/Makefile          |    3 
 Documentation/DocBook/debugobjects.tmpl |  354 ++++++++++++++++++++++++++++++++
 2 files changed, 356 insertions(+), 1 deletion(-)
Documentation/DocBook/Makefile +2 -1
Index: linux-2.6/Documentation/DocBook/Makefile
===================================================================
@@ -11,7 +11,8 @@ DOCBOOKS := wanbook.xml z8530book.xml mc
 	    procfs-guide.xml writing_usb_driver.xml networking.xml \
 	    kernel-api.xml filesystems.xml lsm.xml usb.xml \
 	    gadget.xml libata.xml mtdnand.xml librs.xml rapidio.xml \
-	    genericirq.xml s390-drivers.xml uio-howto.xml scsi.xml
+	    genericirq.xml s390-drivers.xml uio-howto.xml scsi.xml \
+	    debugobjects.xml
 
 ###
 # The build process is as follows (targets):
Documentation/DocBook/debugobjects.tmpl new file +354 -0

354 lines not on this page · raw message

#10 lore raw

Re: [patch 4/5] debugobjects: add documentation

Quoted 1 line · On Wed, 05 Mar 2008 16:04:02 -0000 Thomas Gleixner wrote:

Add a DocBook for debugobjects.

Thanks!

Quoted 12 lines
---
 Documentation/DocBook/Makefile          |    3 
 Documentation/DocBook/debugobjects.tmpl |  354 ++++++++++++++++++++++++++++++++
 2 files changed, 356 insertions(+), 1 deletion(-)

Index: linux-2.6/Documentation/DocBook/debugobjects.tmpl
===================================================================

Documentation/DocBook/debugobjects.tmpl line 5

Quoted 5 lines
Documentation/DocBook/debugobjects.tmpl new file +5 -0
@@ -0,0 +1,354 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!DOCTYPE book PUBLIC "-//OASIS//DTD DocBook XML V4.1.2//EN"
+	"http://www.oasis-open.org/docbook/xml/4.1.2/docbookx.dtd" []>
+
+<book id="debog-objects-guide">

debug-

Quoted 14 lines
Documentation/DocBook/debugobjects.tmpl new file +14 -0
+ <bookinfo>
+  <title>Debug objects life time</title>
+
+  <authorgroup>
+   <author>
+    <firstname>Thomas</firstname>
+    <surname>Gleixner</surname>
+    <affiliation>
+     <address>
+      <email>[email protected]</email>
+     </address>
+    </affiliation>
+   </author>
+  </authorgroup>

...

Documentation/DocBook/debugobjects.tmpl line 60

Quoted 6 lines
Documentation/DocBook/debugobjects.tmpl new file +6 -0
+
+  <chapter id="intro">
+    <title>Introduction</title>
+    <para>
+      debugobjects is a generic infrastructure to track the life time
+      of kernel objects and validate the operation on those.

operations

Documentation/DocBook/debugobjects.tmpl line 65

Quoted 5 lines
Documentation/DocBook/debugobjects.tmpl new file +5 -0
+    </para>
+    <para>
+      debugobjects is useful to check for the following error patterns:
+	<itemizedlist>
+	  <listitem><para>Activtation of uninitialized objects</para></listitem>

Activation

Documentation/DocBook/debugobjects.tmpl line 71

Quoted 6 lines
Documentation/DocBook/debugobjects.tmpl new file +6 -0
+	  <listitem><para>Initialization of active objects</para></listitem>
+	  <listitem><para>Usage of freed/destroyed objects</para></listitem>
+	</itemizedlist>
+    </para>
+    <para>
+      debugobjects is not changing the data structure of the real

debugobjects does not change the data ...

Quoted 5 lines
Documentation/DocBook/debugobjects.tmpl new file +5 -0
+      object so it can be compiled in with a minimal runtime impact
+      and enabled on demand with a kernel command line option.
+    </para>
+  </chapter>
+

Documentation/DocBook/debugobjects.tmpl line 86

Quoted 10 lines
Documentation/DocBook/debugobjects.tmpl new file +10 -0
+  <chapter id="howto">
+    <title>Howto use debugobjects</title>
+    <para>
+      A kernel subsystem needs to provide a data structure which
+      describes the object type and add calls into the debug code at
+      appropriate places. The data structure to describe the object
+      type needs at minimum the name of the object type. Optional
+      functions can and should be provided to fixup detected problems
+      so the kernel can continue to work and the debug information can
+      be retrieved from a alive system instead of hard core debugging

live

Quoted 1 line

+ with serial consoles and stack trace transscripts from the

transcripts

Quoted 8 lines
Documentation/DocBook/debugobjects.tmpl new file +8 -0
+      monitor.
+    </para>
+    <para>
+      The debug calls provided by debugobjects are:
+      <itemizedlist>
+	<listitem><para>debug_object_init</para></listitem>
+	<listitem><para>debug_object_activate</para></listitem>
+	<listitem><para>debug_object_deactivate</para></listitem>

Documentation/DocBook/debugobjects.tmpl line 105

Quoted 10 lines
Documentation/DocBook/debugobjects.tmpl new file +10 -0
+	<listitem><para>debug_object_destroy</para></listitem>
+	<listitem><para>debug_object_free</para></listitem>
+      </itemizedlist>
+      Each of these functions takes the address of the real object and
+      a pointer to the object type specific debug description
+      structure.
+    </para>
+    <para>
+      Each detected error is reported in the statistics and a limited
+      number of errors is printk'ed including a full stack trace.

are

Quoted 24 lines
Documentation/DocBook/debugobjects.tmpl new file +24 -0
+    </para>
+    <para>
+      The statistics are available via debugfs/debug_objects/stats.
+      They provide information about the number of warnings and the
+      number of successful fixups along with information about the
+      usage of the internal tracking objects and the state of the
+      internal tracking objects pool.
+    </para>
+  </chapter>
+  <chapter id="debugfunctions">
+    <title>Debug functions</title>
+    <sect1 id="prototypes">
+      <title>Debug object function reference</title>
+!Elib/debugobjects.c
+    </sect1>
+    <sect1 id="debug_object_init">
+      <title>debug_object_init</title>
+      <para>
+	This function is called whenever the initialization function
+	of a real object is called.
+      </para>
+      <para>
+	When the real object is already tracked by debugobjects it is
+	checked, whether the object can be initialized.  Initializing

Documentation/DocBook/debugobjects.tmpl line 139

Quoted 10 lines
Documentation/DocBook/debugobjects.tmpl new file +10 -0
+	is not allowed for active and destroyed objects. When
+	debugobjects detects an error, then it calls the fixup_init
+	function of the object type description structure if provided
+	by the caller. The fixup function can correct the problem
+	before the real initialization of the object happens. E.g. it
+	can deactivate an active object in order to prevent damage to
+	the subsystem.
+      </para>
+      <para>
+	When the real object is not yet tracked by debugobjects

End above line with comma.

Quoted 16 lines
Documentation/DocBook/debugobjects.tmpl new file +16 -0
+	debugobjects allocates a tracker object for the real object
+	and sets the tracker object state to ODEBUG_STATE_INIT.
+      </para>
+    </sect1>
+
+    <sect1 id="debug_object_activate">
+      <title>debug_object_activate</title>
+      <para>
+	This function is called whenever the activation function of a
+	real object is called.
+      </para>
+      <para>
+	When the real object is already tracked by debugobjects it is
+	checked, whether the object can be activated.  Activating is
+	not allowed for active and destroyed objects. When
+	debugobjects detects an error, then it calls the

Documentation/DocBook/debugobjects.tmpl line 165

Quoted 10 lines
Documentation/DocBook/debugobjects.tmpl new file +10 -0
+	fixup_activate function of the object type description
+	structure if provided by the caller. The fixup function can
+	correct the problem before the real activation of the object
+	happens. E.g. it can deactivate an active object in order to
+	prevent damage to the subsystem.
+      </para>
+      <para>
+	When the real object is not yet tracked by debugobjects then
+	the fixup_activate function is called if available.  This is
+	necessary to allow the legit activation of statically

legitimate

Documentation/DocBook/debugobjects.tmpl line 171

Quoted 6 lines
Documentation/DocBook/debugobjects.tmpl new file +6 -0
+	allocated and initialized objects. The fixup function checks
+	whether the object is valid and calls the debug_objects_init()
+	function to initialize the tracking of this object.
+      </para>
+      <para>
+	When the activation is legit, then the state of the associated

legitimate,

Quoted 7 lines
Documentation/DocBook/debugobjects.tmpl new file +7 -0
+	tracker object is set to ODEBUG_STATE_ACTIVE.
+      </para>
+    </sect1>
+
+    <sect1 id="debug_object_deactivate">
+      <title>debug_object_deactivate</title>
+      <para>

Documentation/DocBook/debugobjects.tmpl line 188

Quoted 10 lines
Documentation/DocBook/debugobjects.tmpl new file +10 -0
+	This function is called whenever the deactivation function of
+	a real object is called.
+      </para>
+      <para>
+	When the real object is tracked by debugobjects it is checked,
+	whether the object can be deactivated. Deactivating is not
+	allowed for untracked or destroyed objects.
+      </para>
+      <para>
+	When the deactivation is legit, then the state of the

legitimate,

Quoted 14 lines
Documentation/DocBook/debugobjects.tmpl new file +14 -0
+	associated tracker object is set to ODEBUG_STATE_INACTIVE.
+      </para>
+    </sect1>
+
+    <sect1 id="debug_object_destroy">
+      <title>debug_object_destroy</title>
+      <para>
+	This function is called to mark an object destroyed. This is
+	useful to prevent the usage of invalid objects, which are
+	still available in memory: either statically allocated objects
+	or objects which are freed later.
+      </para>
+      <para>
+	When the real object is tracked by debugobjects it is checked,

Documentation/DocBook/debugobjects.tmpl line 212

Quoted 10 lines
Documentation/DocBook/debugobjects.tmpl new file +10 -0
+	whether the object can be destroyed. Destruction is not
+	allowed for active and destroyed objects. When debugobjects
+	detects an error, then it calls the fixup_destroy function of
+	the object type description structure if provided by the
+	caller. The fixup function can correct the problem before the
+	real destruction of the object happens. E.g. it can deactivate
+	an active object in order to prevent damage to the subsystem.
+      </para>
+      <para>
+	When the destruction is legit, then the state of the

legitimate,

Quoted 40 lines
Documentation/DocBook/debugobjects.tmpl new file +40 -0
+	associated tracker object is set to ODEBUG_STATE_DESTROYED.
+      </para>
+    </sect1>
+
+    <sect1 id="debug_object_free">
+      <title>debug_object_free</title>
+      <para>
+	This function is called before an object is freed.
+      </para>
+      <para>
+	When the real object is tracked by debugobjects it is checked,
+	whether the object can be freed. Free is not allowed for
+	active objects. When debugobjects detects an error, then it
+	calls the fixup_free function of the object type description
+	structure if provided by the caller. The fixup function can
+	correct the problem before the real free of the object
+	happens. E.g. it can deactivate an active object in order to
+	prevent damage to the subsystem.
+      </para>
+      <para>
+	Note that debug_object_free removes the object from the
+	tracker. Later usage of the object is detected by the other
+	debug checks.
+      </para>
+    </sect1>
+  </chapter>
+  <chapter id="fixupfunctions">
+    <title>Fixup functions</title>
+    <sect1 id="debug_obj_descr">
+      <title>Debug object type description structure</title>
+!Iinclude/linux/debugobjects.h
+    </sect1>
+    <sect1 id="fixup_init">
+      <title>fixup_init</title>
+      <para>
+	This function is called from the debug code whenever a problem
+	in debug_object_init is detected. The function takes the
+	address of the object and the state which is currently
+	recorded in the tracker.
+      </para>

Documentation/DocBook/debugobjects.tmpl line 262

Quoted 10 lines
Documentation/DocBook/debugobjects.tmpl new file +10 -0
+      <para>
+	Called from debug_object_init when the object state is:
+	<itemizedlist>
+	  <listitem><para>ODEBUG_STATE_ACTIVE</para></listitem>
+	</itemizedlist>
+      </para>
+      <para>
+	The function returns "1" then the fixup was successful,
+	otherwise "0". The return value is used to update the
+	statistics.

We usually don't quote numbers unless they are strings (instead of numbers).

Documentation/DocBook/debugobjects.tmpl line 265

Quoted 3 lines
Documentation/DocBook/debugobjects.tmpl new file +3 -0
+      </para>
+      <para>
+	Note, that the function needs to call the debug_object_init()

Drop comma.

Documentation/DocBook/debugobjects.tmpl

Quoted 1 line
Documentation/DocBook/debugobjects.tmpl new file +1 -0
+	function again, after the damage has been repaired in order to

Ditto.

Quoted 11 lines
Documentation/DocBook/debugobjects.tmpl new file +11 -0
+	keep the state consistent.
+      </para>
+    </sect1>
+
+    <sect1 id="fixup_activate">
+      <title>fixup_activate</title>
+      <para>
+	This function is called from the debug code whenever a problem
+	in debug_object_activate is detected.
+      </para>
+      <para>

Documentation/DocBook/debugobjects.tmpl line 287

Quoted 10 lines
Documentation/DocBook/debugobjects.tmpl new file +10 -0
+	Called from debug_object_activate when the object state is:
+	<itemizedlist>
+	  <listitem><para>ODEBUG_STATE_NOTAVAILABLE</para></listitem>
+	  <listitem><para>ODEBUG_STATE_ACTIVE</para></listitem>
+	</itemizedlist>
+      </para>
+      <para>
+	The function returns "1" then the fixup was successful,
+	otherwise "0". The return value is used to update the
+	statistics.

1 or 0

Documentation/DocBook/debugobjects.tmpl line 290

Quoted 3 lines
Documentation/DocBook/debugobjects.tmpl new file +3 -0
+      </para>
+      <para>
+	Note, that the function needs to call the debug_object_activate()

Drop comma.

Documentation/DocBook/debugobjects.tmpl

Quoted 1 line
Documentation/DocBook/debugobjects.tmpl new file +1 -0
+	function again, after the damage has been repaired in order to

Ditto.

Documentation/DocBook/debugobjects.tmpl line 299

Quoted 8 lines
Documentation/DocBook/debugobjects.tmpl new file +8 -0
+	keep the state consistent.
+      </para>
+      <para>
+	The activation of statically initialized objects is a special
+	case. When debug_object_activate() has no tracked object for
+	this object address then fixup_activate() is called with
+	object state ODEBUG_STATE_NOTAVAILABLE. The fixup function
+	needs to check whether this is a legit case of a statically

legitimate

Documentation/DocBook/debugobjects.tmpl line 303

Quoted 4 lines
Documentation/DocBook/debugobjects.tmpl new file +4 -0
+	initialized object or not. In case it is it calls
+	debug_object_init() and debug_object_activate() to make the
+	object known to the tracker and marked active. In this case
+	the function should return "0" because this is not a real

0

Quoted 9 lines
Documentation/DocBook/debugobjects.tmpl new file +9 -0
+	fixup.
+      </para>
+    </sect1>
+
+    <sect1 id="fixup_destroy">
+      <title>fixup_destroy</title>
+      <para>
+	This function is called from the debug code whenever a problem
+	in debug_object_destroy is detected.

Documentation/DocBook/debugobjects.tmpl line 322

Quoted 10 lines
Documentation/DocBook/debugobjects.tmpl new file +10 -0
+      </para>
+      <para>
+	Called from debug_object_destroy when the object state is:
+	<itemizedlist>
+	  <listitem><para>ODEBUG_STATE_ACTIVE</para></listitem>
+	</itemizedlist>
+      </para>
+      <para>
+	The function returns "1" then the fixup was successful,
+	otherwise "0". The return value is used to update the

1 or 0

Documentation/DocBook/debugobjects.tmpl line 331

Quoted 9 lines
Documentation/DocBook/debugobjects.tmpl new file +9 -0
+	statistics.
+      </para>
+    </sect1>
+    <sect1 id="fixup_free">
+      <title>fixup_free</title>
+      <para>
+	This function is called from the debug code whenever a problem
+	in debug_object_free is detected. Further it can be called
+	from the debug checks in k/v free, when an active object is

kfree/vfree ?

Documentation/DocBook/debugobjects.tmpl line 343

Quoted 12 lines
Documentation/DocBook/debugobjects.tmpl new file +12 -0
+	detected from the debug_check_no_obj_freed() sanity checks.
+      </para>
+      <para>
+	Called from debug_object_free() or debug_check_no_obj_freed()
+	when the object state is:
+	<itemizedlist>
+	  <listitem><para>ODEBUG_STATE_ACTIVE</para></listitem>
+	</itemizedlist>
+      </para>
+      <para>
+	The function returns "1" then the fixup was successful,
+	otherwise "0". The return value is used to update the

1 or 0

Quoted 13 lines
Documentation/DocBook/debugobjects.tmpl new file +11 -0
+	statistics.
+      </para>
+    </sect1>
+  </chapter>
+  <chapter id="bugs">
+    <title>Known Bugs And Assumptions</title>
+    <para>
+	None (knock on wood).
+    </para>
+  </chapter>
+</book>
Signature · 1 line
-- 

---
~Randy

#11 lore raw

Re: [patch 4/5] debugobjects: add documentation

Quoted 5 lines · Randy Dunlap
Quoted 1 line · On Wed, 05 Mar 2008 16:04:02 -0000 Thomas Gleixner wrote:

Add a DocBook for debugobjects.

Thanks!

Thanks for reviewing ! All comments processed.

Thanks,
tglx

#12 lore raw

PATCH 5/5 debugobjects: add timer specific object debugging code

In reply to Thomas Gleixner

Add calls to the generic object debugging infrastructure and provide fixup functions which allow to keep the system alive when recoverable problems have been detected by the object debugging core code.

---
 include/linux/poison.h |    7 +++
 include/linux/timer.h  |    3 +
 kernel/timer.c         |   91 +++++++++++++++++++++++++++++++++++++++++++++++++
 lib/Kconfig.debug      |    8 ++++
 4 files changed, 108 insertions(+), 1 deletion(-)
include/linux/poison.h +7 -0
Index: linux-2.6/include/linux/poison.h
===================================================================
@@ -10,6 +10,13 @@
 #define LIST_POISON1  ((void *) 0x00100100)
 #define LIST_POISON2  ((void *) 0x00200200)
 
+/********** include/linux/timer.h **********/
+/*
+ * Magic number "tsta" to indicate a static timer initializer
+ * for the object debugging code.
+ */
+#define TIMER_ENTRY_STATIC	((void *) 0x74737461)
+
 /********** mm/slab.c **********/
 /*
  * Magic nums for obj red zoning.
include/linux/timer.h +2 -1
Index: linux-2.6/include/linux/timer.h
===================================================================
@@ -4,6 +4,7 @@
 #include <linux/list.h>
 #include <linux/ktime.h>
 #include <linux/stddef.h>
+#include <linux/debugobjects.h>
 
 struct tvec_base;
 
@@ -25,6 +26,7 @@ struct timer_list {
 extern struct tvec_base boot_tvec_bases;
 
 #define TIMER_INITIALIZER(_function, _expires, _data) {		\
+		.entry = { .prev = TIMER_ENTRY_STATIC },	\
 		.function = (_function),			\
 		.expires = (_expires),				\
 		.data = (_data),				\
@@ -164,5 +166,4 @@ unsigned long __round_jiffies_relative(u
 unsigned long round_jiffies(unsigned long j);
 unsigned long round_jiffies_relative(unsigned long j);
 
-
 #endif
kernel/timer.c +91 -0

121 lines not on this page · raw message

lib/Kconfig.debug +8 -0
Index: linux-2.6/lib/Kconfig.debug
===================================================================
@@ -206,6 +206,14 @@ config DEBUG_OBJECTS_FREE
 	  properly. This can make kmalloc/kfree-intensive workloads
 	  much slower.
 
+config DEBUG_OBJECTS_TIMERS
+	bool "Debug timer objects"
+	depends on DEBUG_OBJECTS
+	help
+	  If you say Y here, additional code will be inserted into the
+	  timer routines to track the life time of timer objects and
+	  validate the timer operations.
+
 config DEBUG_SLAB
 	bool "Debug slab memory allocations"
 	depends on DEBUG_KERNEL && SLAB
author Acked-by Ingo Molnar on v1 5/5
#13 lore raw

Re: [patch 5/5] debugobjects: add timer specific object debugging code

On Wed, 5 Mar 2008, Thomas Gleixner wrote:

Oops. Being exited about the working infrastructure let me forget to make it compile with the timer debug code disabled.

Updated patch below.

Thanks,
tglx

--------------->

Subject: debugobjects: add timer specific object debugging code
From: Thomas Gleixner <[email protected]>
Date: Tue, 26 Feb 2008 09:28:37 +0100

Add calls to the generic object debugging infrastructure and provide fixup functions which allow to keep the system alive when recoverable problems have been detected by the object debugging core code.

---
 include/linux/poison.h |    7 +++
 include/linux/timer.h  |    3 -
 kernel/timer.c         |  110 +++++++++++++++++++++++++++++++++++++++++++++++++
 lib/Kconfig.debug      |    8 +++
 4 files changed, 127 insertions(+), 1 deletion(-)
include/linux/poison.h +7 -0
Index: linux-2.6/include/linux/poison.h
===================================================================
@@ -10,6 +10,13 @@
 #define LIST_POISON1  ((void *) 0x00100100)
 #define LIST_POISON2  ((void *) 0x00200200)
 
+/********** include/linux/timer.h **********/
+/*
+ * Magic number "tsta" to indicate a static timer initializer
+ * for the object debugging code.
+ */
+#define TIMER_ENTRY_STATIC	((void *) 0x74737461)
+
 /********** mm/slab.c **********/
 /*
  * Magic nums for obj red zoning.
include/linux/timer.h +2 -1
Index: linux-2.6/include/linux/timer.h
===================================================================
@@ -4,6 +4,7 @@
 #include <linux/list.h>
 #include <linux/ktime.h>
 #include <linux/stddef.h>
+#include <linux/debugobjects.h>
 
 struct tvec_base;
 
@@ -25,6 +26,7 @@ struct timer_list {
 extern struct tvec_base boot_tvec_bases;
 
 #define TIMER_INITIALIZER(_function, _expires, _data) {		\
+		.entry = { .prev = TIMER_ENTRY_STATIC },	\
 		.function = (_function),			\
 		.expires = (_expires),				\
 		.data = (_data),				\
@@ -164,5 +166,4 @@ unsigned long __round_jiffies_relative(u
 unsigned long round_jiffies(unsigned long j);
 unsigned long round_jiffies_relative(unsigned long j);
 
-
 #endif
kernel/timer.c +110 -0

140 lines not on this page · raw message

lib/Kconfig.debug +8 -0
Index: linux-2.6/lib/Kconfig.debug
===================================================================
@@ -206,6 +206,14 @@ config DEBUG_OBJECTS_FREE
 	  properly. This can make kmalloc/kfree-intensive workloads
 	  much slower.
 
+config DEBUG_OBJECTS_TIMERS
+	bool "Debug timer objects"
+	depends on DEBUG_OBJECTS
+	help
+	  If you say Y here, additional code will be inserted into the
+	  timer routines to track the life time of timer objects and
+	  validate the timer operations.
+
 config DEBUG_SLAB
 	bool "Debug slab memory allocations"
 	depends on DEBUG_KERNEL && SLAB
#14 lore raw
In reply to Thomas Gleixner
Quoted 14 lines · Thomas Gleixner

This is version 2 of the object debugging infrastructure. Changes versus V1:

- review comments processed:
- multiplexing interfaces replaced
- object type is determined by a caller provided pointer to a debug structure which replaces the object type identifier -> No changes to the core code for new users
- new debug state "destroyed" (kobject wishlist)
- various cleanups

- complete detachment of the tracker object from the real object (no change to the real objects data structure necessary anymore)

Quoted 7 lines

- improved robustness (object memory management, oom cleanup)

- full set of self tests

- debugfs statistics

- docbook documentation

This looks very nice, thanks for the documentation as well.

I'll work on adding kobjects to this infrastructure and send you a patch if it works out well.

thanks,

greg k-h

#15 lore raw
In reply to Thomas Gleixner
Quoted 14 lines · Thomas Gleixner

This is version 2 of the object debugging infrastructure. Changes versus V1:

- review comments processed:
- multiplexing interfaces replaced
- object type is determined by a caller provided pointer to a debug structure which replaces the object type identifier -> No changes to the core code for new users
- new debug state "destroyed" (kobject wishlist)
- various cleanups

- complete detachment of the tracker object from the real object (no change to the real objects data structure necessary anymore)

Quoted 7 lines

- improved robustness (object memory management, oom cleanup)

- full set of self tests

- debugfs statistics

- docbook documentation

Here's a patch adding kobjects to this infrastructure (I didn't use a new CONFIG option, which it probably should be.)

Does this look correct? I can't tell if this is going to catch anything, as my trees all seem to have these kinds of bugs worked out now, as we are already catching a number of these types of things already with kobjects.

Should I be doing more checks? Different things based on the state of the object?

thanks,

greg k-h

-------------------

From: Greg Kroah-Hartman <[email protected]>
Subject: kobjects: hook them up to the debugobjects infrastructure

I think this is right, but it's hard to tell...

---
 lib/kobject.c |   53 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 53 insertions(+)
lib/kobject.c +53 -0
@@ -18,8 +18,59 @@
 #include <linux/stat.h>
 #include <linux/slab.h>
 #include <linux/kallsyms.h>
+#include <linux/debugobjects.h>
 #include <asm-generic/sections.h>
 
+#ifdef CONFIG_DEBUG_OBJECTS
+static int kobject_fixup_init(void *addr, enum debug_obj_state state)
+{
+	struct kobject *kobj = addr;
+
+	switch (state) {
+	case ODEBUG_STATE_ACTIVE:
+		printk("kobject: '%s' (%p): is being initialized after "
+			"it was already active\n", kobject_name(kobj), kobj);
+		return 1;
+	default:
+		return 0;
+	}
+}
+
+static int kobject_fixup_destroy(void *addr, enum debug_obj_state state)
+{
+	struct kobject *kobj = addr;
+
+	switch (state) {
+	case ODEBUG_STATE_ACTIVE:
+		printk("kobject: '%s' (%p): is being destroyed yet it is "
+			"still active\n", kobject_name(kobj), kobj);
+		return 1;
+	default:
+		return 0;
+	}
+}
+
+static struct debug_obj_descr kobject_debug_descr = {
+	.name = "kobjects",
+	.fixup_init = kobject_fixup_init,
+	.fixup_destroy = kobject_fixup_destroy,
+};
+
+static void debug_kobject_init(struct kobject *kobj)
+{
+	debug_object_init(kobj, &kobject_debug_descr);
+}
+
+static void debug_kobject_destroy(struct kobject *kobj)
+{
+	debug_object_destroy(kobj, &kobject_debug_descr);
+}
+
+#else
+static inline void debug_kobject_init(struct kobject *kobj) { }
+static inline void debug_kobject_destroy(struct kobject *kobj) { }
+#endif
+
 #ifdef CONFIG_X86_32
 static int ptr_in_range(void *ptr, void *start, void *end)
 {
@@ -202,6 +253,7 @@ static void kobject_init_internal(struct
 {
 	if (!kobj)
 		return;
+	debug_kobject_init(kobj);
 	kref_init(&kobj->kref);
 	INIT_LIST_HEAD(&kobj->entry);
 	kobj->state_in_sysfs = 0;
@@ -599,6 +651,7 @@ static void kobject_cleanup(struct kobje
 	pr_debug("kobject: '%s' (%p): %s\n",
 		 kobject_name(kobj), kobj, __FUNCTION__);
 
+	debug_kobject_destroy(kobj);
 	if (t && !t->release)
 		pr_debug("kobject: '%s' (%p): does not have a release() "
 			 "function, it is broken and must be fixed.\n",
#16 lore raw
Quoted 10 lines · Greg KH

Here's a patch adding kobjects to this infrastructure (I didn't use a new CONFIG option, which it probably should be.)

Does this look correct? I can't tell if this is going to catch anything, as my trees all seem to have these kinds of bugs worked out now, as we are already catching a number of these types of things already with kobjects.

Should I be doing more checks? Different things based on the state of the object?

Depends on the kind of state of objects you have. Right now you only track the initialized and the destroyed state. The question is whether there is some other state like "access". I have a look.

Thanks,

tglx