static keys for debugobjects

PATCH New Stephen Boyd 1 participant 4 messages Apr 6, 2012

Subsystems (3)

Library code High-resolution timers, timer wheel, clockevents Debugobjects:

lore

Share & tools

lore

Stephen Boyd

author

2012-04-06 07:03 UTC #0 lore raw

cover static keys for debugobjects

Building in debugobjects support without enabling debugobjects by default is useful in test scenarios where recompiling isn't an option. Moving this interface to static keys/jump labels should allow us to always have this code compiled in without worrying about performance overhead when it's disabled.

RFC because I don't have any numbers to back this up and it's debug code. I took a stab at using perf but I don't think the x86 machine I was using had good enough stuff to see i-cache misses or things like that. Hints on what to do to actually prove this is useful are appreciated.

This is based on the perf/jump-labels branch in the tip tree:

git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git perf/jump-labels

Stephen Boyd (3):
timer: Move debugobjects.h include to timer.c
init: Initialize jump_labels before early parameters
debugobjects: Use static keys for debug_objects_enabled

 include/linux/debugobjects.h |   94 +++++++++++++++++++++++++++++++++++++-----
 include/linux/timer.h        |    1 -
 init/main.c                  |    4 +-
 kernel/timer.c               |    1 +
 lib/debugobjects.c           |   73 ++++++++++++--------------------
 5 files changed, 113 insertions(+), 60 deletions(-)

Signature · 2 lines

Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

Stephen Boyd

author

2012-04-06 07:03 UTC #1 lore raw

PATCH 1/3 timer: Move debugobjects.h include to timer.c

There is no usage of debugobjects structures in timer.h so move the include to timer.c. This avoids an include file circular dependency if timer.h includes debugobjects.h which includes jump_label.h (in a later patch) which includes workqueue.h which includes timer.h.

Signed-off-by Stephen Boyd <[email protected]>

Cc Thomas Gleixner <[email protected]>

---

This should be good to apply regardless of the rest of this series.

 include/linux/timer.h |    1 -
 kernel/timer.c        |    1 +
 2 files changed, 1 insertion(+), 1 deletion(-)

include/linux/timer.h +0 -1

index 6abd913..d7848ab 100644
@@ -4,7 +4,6 @@
 #include <linux/list.h>
 #include <linux/ktime.h>
 #include <linux/stddef.h>
-#include <linux/debugobjects.h>
 #include <linux/stringify.h>
 
 struct tvec_base;

kernel/timer.c +1 -0

index a297ffc..aa0841c 100644
@@ -40,6 +40,7 @@
 #include <linux/irq_work.h>
 #include <linux/sched.h>
 #include <linux/slab.h>
+#include <linux/debugobjects.h>
 
 #include <asm/uaccess.h>
 #include <asm/unistd.h>

Signature · 2 lines

Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

Stephen Boyd

author

2012-04-06 07:04 UTC #2 lore raw

PATCH 2/3 init: Initialize jump_labels before early parameters

In reply to Stephen Boyd

We want to use jump_labels in the debug_objects code so that when debug_objects aren't enabled by default we can skip debug_object code. Move the jump_label initialization before the early parameter parsing so that debug_objects code can enable and disable itself according to kernel command line parameters.

Signed-off-by Stephen Boyd <[email protected]>

Cc Jeremy Fitzhardinge <[email protected]>

Cc Thomas Gleixner <[email protected]>

---

This also opens the window to using static keys for pr_debug(). I tried and wound up in circular dependency hell.

 init/main.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

init/main.c +2 -2

index ff49a6d..39a3092 100644
@@ -502,14 +502,14 @@ asmlinkage void __init start_kernel(void)
 	build_all_zonelists(NULL);
 	page_alloc_init();
 
+	jump_label_init();
+
 	printk(KERN_NOTICE "Kernel command line: %s\n", boot_command_line);
 	parse_early_param();
 	parse_args("Booting kernel", static_command_line, __start___param,
 		   __stop___param - __start___param,
 		   &unknown_bootoption);
 
-	jump_label_init();
-
 	/*
 	 * These use large bootmem allocations and must precede
 	 * kmem_cache_init()

Signature · 2 lines

Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

Stephen Boyd

author

2012-04-06 07:03 UTC #3 lore raw

PATCH 3/3 debugobjects: Use static keys for debug_objects_enabled

In reply to Stephen Boyd

Instead of using a __read_mostly variable for returning early from debug_objects operations use static keys. This should allow us to dynamically patch out debug_objects code from a running kernel and reduce the overhead of having debug objects compiled in and not enabled.

Signed-off-by Stephen Boyd <[email protected]>

Cc Thomas Gleixner <[email protected]>

---

The inlining is sort of ugly.

 include/linux/debugobjects.h |   94 +++++++++++++++++++++++++++++++++++++-----
 lib/debugobjects.c           |   73 ++++++++++++--------------------
 2 files changed, 110 insertions(+), 57 deletions(-)

include/linux/debugobjects.h +84 ~~-10~~

index 0e5f578..902e430 100644
@@ -3,6 +3,7 @@
 
 #include <linux/list.h>
 #include <linux/spinlock.h>
+#include <linux/jump_label.h>
 
 enum debug_obj_state {
 	ODEBUG_STATE_NONE,

@@ -60,26 +61,94 @@ struct debug_obj_descr {
 };
 
 #ifdef CONFIG_DEBUG_OBJECTS
-extern void debug_object_init      (void *addr, struct debug_obj_descr *descr);
+extern struct static_key debug_objects_enabled;
+#if CONFIG_DEBUG_OBJECTS_ENABLE_DEFAULT == 1
+	#define debug_object_branch static_key_true
+#else
+	#define debug_object_branch static_key_false
+#endif
+
+extern void _debug_object_init(void *addr, struct debug_obj_descr *descr);
+static __always_inline void
+debug_object_init(void *addr, struct debug_obj_descr *descr)
+{
+	if (debug_object_branch(&debug_objects_enabled))
+		_debug_object_init(addr, descr);
+}
+
 extern void
-debug_object_init_on_stack(void *addr, struct debug_obj_descr *descr);
-extern void debug_object_activate  (void *addr, struct debug_obj_descr *descr);
-extern void debug_object_deactivate(void *addr, struct debug_obj_descr *descr);
-extern void debug_object_destroy   (void *addr, struct debug_obj_descr *descr);
-extern void debug_object_free      (void *addr, struct debug_obj_descr *descr);
-extern void debug_object_assert_init(void *addr, struct debug_obj_descr *descr);
+_debug_object_init_on_stack(void *addr, struct debug_obj_descr *descr);
+static __always_inline void
+debug_object_init_on_stack(void *addr, struct debug_obj_descr *descr)
+{
+	if (debug_object_branch(&debug_objects_enabled))
+		_debug_object_init_on_stack(addr, descr);
+}
+
+extern void _debug_object_activate(void *addr, struct debug_obj_descr *descr);
+static __always_inline void
+debug_object_activate(void *addr, struct debug_obj_descr *descr)
+{
+	if (debug_object_branch(&debug_objects_enabled))
+		_debug_object_activate(addr, descr);
+}
+
+extern void _debug_object_deactivate(void *addr, struct debug_obj_descr *descr);
+static __always_inline void
+debug_object_deactivate(void *addr, struct debug_obj_descr *descr)
+{
+	if (debug_object_branch(&debug_objects_enabled))
+		_debug_object_deactivate(addr, descr);
+}
 
+extern void _debug_object_destroy(void *addr, struct debug_obj_descr *descr);
+static __always_inline void
+debug_object_destroy(void *addr, struct debug_obj_descr *descr)
+{
+	if (debug_object_branch(&debug_objects_enabled))
+		_debug_object_destroy(addr, descr);
+}
+
+extern void _debug_object_free(void *addr, struct debug_obj_descr *descr);
+static __always_inline void
+debug_object_free(void *addr, struct debug_obj_descr *descr)
+{
+	if (debug_object_branch(&debug_objects_enabled))
+		_debug_object_free(addr, descr);
+}
+
+extern void
+_debug_object_assert_init(void *addr, struct debug_obj_descr *descr);
+static __always_inline void
+debug_object_assert_init(void *addr, struct debug_obj_descr *descr)
+{
+	if (debug_object_branch(&debug_objects_enabled))
+		_debug_object_assert_init(addr, descr);
+}
 /*
  * Active state:
  * - Set at 0 upon initialization.
  * - Must return to 0 before deactivation.
  */
 extern void
-debug_object_active_state(void *addr, struct debug_obj_descr *descr,
+_debug_object_active_state(void *addr, struct debug_obj_descr *descr,
 			  unsigned int expect, unsigned int next);
+static __always_inline void
+debug_object_active_state(void *addr, struct debug_obj_descr *descr,
+			  unsigned int expect, unsigned int next)
+{
+	if (debug_object_branch(&debug_objects_enabled))
+		_debug_object_active_state(addr, descr, expect, next);
+}
 
 extern void debug_objects_early_init(void);
-extern void debug_objects_mem_init(void);
+
+extern void _debug_objects_mem_init(void);
+static __always_inline void debug_objects_mem_init(void)
+{
+	if (debug_object_branch(&debug_objects_enabled))
+		_debug_objects_mem_init();
+}
 #else
 static inline void
 debug_object_init      (void *addr, struct debug_obj_descr *descr) { }

@@ -101,7 +170,12 @@ static inline void debug_objects_mem_init(void) { }
 #endif
 
 #ifdef CONFIG_DEBUG_OBJECTS_FREE
-extern void debug_check_no_obj_freed(const void *address, unsigned long size);
+extern void _debug_check_no_obj_freed(const void *address, unsigned long size);
+#define debug_check_no_obj_freed(address, size) \
+	({ \
+		if (debug_object_branch(&debug_objects_enabled)) \
+			_debug_check_no_obj_freed(address, size); \
+	})
 #else
 static inline void
 debug_check_no_obj_freed(const void *address, unsigned long size) { }

lib/debugobjects.c +26 ~~-47~~

217 lines not on this page · raw message

Signature · 2 lines

Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.