[PATCH 2/2] jump label: introduce static_branch()

[Jason Baron <jbaron@redhat.com>]

Introduce:

static __always_inline bool static_branch(struct jump_label_key *key)

to replace the old JUMP_LABEL(key, label) macro.

The new static_branch(), simplifies the usage of jump labels. Since,
static_branch() returns a boolean, it can be used as part of an if()
construct. It also, allows us to drop the 'label' argument from the
prototype. Its probably best understood with an example, here is the part
of the patch that converts the tracepoints to use unlikely_switch():

--- a/include/linux/tracepoint.h
+++ b/include/linux/tracepoint.h
@@ -146,9 +146,7 @@ static inline void tracepoint_update_probe_range(struct tracepoint *begin,
 	extern struct tracepoint __tracepoint_##name;			\
 	static inline void trace_##name(proto)				\
 	{								\
-		JUMP_LABEL(&__tracepoint_##name.key, do_trace);		\
-		return;							\
-do_trace:								\
+		if (static_branch(&__tracepoint_##name.key))		\
 			__DO_TRACE(&__tracepoint_##name,		\
 				TP_PROTO(data_proto),			\
 				TP_ARGS(data_args));			\


I analyzed the code produced by static_branch(), and it seems to be
at least as good as the code generated by the JUMP_LABEL(). As a reminder,
we get a single nop in the fastpath for -02. But will often times get
a 'double jmp' in the -Os case. That is, 'jmp 0', followed by a jmp around
the disabled code. We believe that future gcc tweaks to allow block
re-ordering in the -Os, will solve the -Os case in the future.

I also saw a 1-2% tbench throughput improvement when compiling with
jump labels.

This patch also addresses a build issue that Tetsuo Handa reported where
gcc v3.3 currently chokes on compiling 'dynamic debug':

include/net/inet_connection_sock.h: In function `inet_csk_reset_xmit_timer':
include/net/inet_connection_sock.h:236: error: duplicate label declaration `do_printk'
include/net/inet_connection_sock.h:219: error: this is a previous declaration
include/net/inet_connection_sock.h:236: error: duplicate label declaration `out'
include/net/inet_connection_sock.h:219: error: this is a previous declaration
include/net/inet_connection_sock.h:236: error: duplicate label `do_printk'
include/net/inet_connection_sock.h:236: error: duplicate label `out'


Thanks to H. Peter Anvin for suggesting this improved syntax.

Suggested-by: H. Peter Anvin <hpa@linux.intel.com>
Signed-off-by: Jason Baron <jbaron@redhat.com>
Tested-by: Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>
---
 arch/sparc/include/asm/jump_label.h |   25 ++++++++++++++-----------
 arch/x86/include/asm/jump_label.h   |   22 +++++++++++++---------
 arch/x86/kernel/jump_label.c        |    2 +-
 include/linux/dynamic_debug.h       |   18 ++++--------------
 include/linux/jump_label.h          |   26 +++++++++++---------------
 include/linux/jump_label_ref.h      |   18 +++++++++++-------
 include/linux/perf_event.h          |   26 +++++++++++++-------------
 include/linux/tracepoint.h          |    4 +---
 kernel/jump_label.c                 |    2 +-
 9 files changed, 69 insertions(+), 74 deletions(-)

diff --git a/arch/sparc/include/asm/jump_label.h b/arch/sparc/include/asm/jump_label.h
index 427d468..882651c 100644
--- a/arch/sparc/include/asm/jump_label.h
+++ b/arch/sparc/include/asm/jump_label.h
@@ -7,17 +7,20 @@
 
 #define JUMP_LABEL_NOP_SIZE 4
 
-#define JUMP_LABEL(key, label)					\
-	do {							\
-		asm goto("1:\n\t"				\
-			 "nop\n\t"				\
-			 "nop\n\t"				\
-			 ".pushsection __jump_table,  \"a\"\n\t"\
-			 ".align 4\n\t"				\
-			 ".word 1b, %l[" #label "], %c0\n\t"	\
-			 ".popsection \n\t"			\
-			 : :  "i" (key) :  : label);\
-	} while (0)
+static __always_inline bool __static_branch(struct jump_label_key *key)
+{
+		asm goto("1:\n\t"				
+			 "nop\n\t"				
+			 "nop\n\t"				
+			 ".pushsection __jump_table,  \"a\"\n\t"
+			 ".align 4\n\t"				
+			 ".word 1b, %l[l_yes], %c0\n\t"	
+			 ".popsection \n\t"			
+			 : :  "i" (key) :  : l_yes);
+	return false;
+l_yes:
+	return true;
+}
 
 #endif /* __KERNEL__ */
 
diff --git a/arch/x86/include/asm/jump_label.h b/arch/x86/include/asm/jump_label.h
index f52d42e..3d44a7c 100644
--- a/arch/x86/include/asm/jump_label.h
+++ b/arch/x86/include/asm/jump_label.h
@@ -5,20 +5,24 @@
 
 #include <linux/types.h>
 #include <asm/nops.h>
+#include <asm/asm.h>
 
 #define JUMP_LABEL_NOP_SIZE 5
 
 # define JUMP_LABEL_INITIAL_NOP ".byte 0xe9 \n\t .long 0\n\t"
 
-# define JUMP_LABEL(key, label)					\
-	do {							\
-		asm goto("1:"					\
-			JUMP_LABEL_INITIAL_NOP			\
-			".pushsection __jump_table,  \"a\" \n\t"\
-			_ASM_PTR "1b, %l[" #label "], %c0 \n\t" \
-			".popsection \n\t"			\
-			: :  "i" (key) :  : label);		\
-	} while (0)
+static __always_inline bool __static_branch(struct jump_label_key *key)
+{
+	asm goto("1:"
+		JUMP_LABEL_INITIAL_NOP
+		".pushsection __jump_table,  \"a\" \n\t"
+		_ASM_PTR "1b, %l[l_yes], %c0 \n\t"
+		".popsection \n\t"
+		: :  "i" (key) : : l_yes );
+	return false;
+l_yes:
+	return true;
+}
 
 #endif /* __KERNEL__ */
 
diff --git a/arch/x86/kernel/jump_label.c b/arch/x86/kernel/jump_label.c
index 961b6b3..dfa4c3c 100644
--- a/arch/x86/kernel/jump_label.c
+++ b/arch/x86/kernel/jump_label.c
@@ -4,13 +4,13 @@
  * Copyright (C) 2009 Jason Baron <jbaron@redhat.com>
  *
  */
-#include <linux/jump_label.h>
 #include <linux/memory.h>
 #include <linux/uaccess.h>
 #include <linux/module.h>
 #include <linux/list.h>
 #include <linux/jhash.h>
 #include <linux/cpu.h>
+#include <linux/jump_label.h>
 #include <asm/kprobes.h>
 #include <asm/alternative.h>
 
diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index ddf7bae..2ade291 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -44,34 +44,24 @@ int ddebug_add_module(struct _ddebug *tab, unsigned int n,
 extern int ddebug_remove_module(const char *mod_name);
 
 #define dynamic_pr_debug(fmt, ...) do {					\
-	__label__ do_printk;						\
-	__label__ out;							\
 	static struct _ddebug descriptor				\
 	__used								\
 	__attribute__((section("__verbose"), aligned(8))) =		\
 	{ KBUILD_MODNAME, __func__, __FILE__, fmt, __LINE__,		\
 		_DPRINTK_FLAGS_DEFAULT, JUMP_LABEL_INIT };		\
-	JUMP_LABEL(&descriptor.enabled, do_printk);			\
-	goto out;							\
-do_printk:								\
-	printk(KERN_DEBUG pr_fmt(fmt),	##__VA_ARGS__);			\
-out:	;								\
+	if (static_branch(&descriptor.enabled))				\
+		printk(KERN_DEBUG pr_fmt(fmt),	##__VA_ARGS__);		\
 	} while (0)
 
 
 #define dynamic_dev_dbg(dev, fmt, ...) do {				\
-	__label__ do_printk;						\
-	__label__ out;							\
 	static struct _ddebug descriptor				\
 	__used								\
 	__attribute__((section("__verbose"), aligned(8))) =		\
 	{ KBUILD_MODNAME, __func__, __FILE__, fmt, __LINE__,		\
 		_DPRINTK_FLAGS_DEFAULT, JUMP_LABEL_INIT };		\
-	JUMP_LABEL(&descriptor.enabled, do_printk);			\
-	goto out;							\
-do_printk:								\
-	dev_printk(KERN_DEBUG, dev, fmt, ##__VA_ARGS__);		\
-out:	;								\
+	if (static_branch(&descriptor.enabled))				\
+		dev_printk(KERN_DEBUG, dev, fmt, ##__VA_ARGS__);	\
 	} while (0)
 
 #else
diff --git a/include/linux/jump_label.h b/include/linux/jump_label.h
index 152f7de..0ad9c2e 100644
--- a/include/linux/jump_label.h
+++ b/include/linux/jump_label.h
@@ -22,6 +22,11 @@ struct module;
 
 #ifdef HAVE_JUMP_LABEL
 
+static __always_inline bool static_branch(struct jump_label_key *key)
+{
+	return __static_branch(key);
+}
+
 extern struct jump_entry __start___jump_table[];
 extern struct jump_entry __stop___jump_table[];
 
@@ -42,11 +47,12 @@ struct jump_label_key {
 	int state;
 };
 
-#define JUMP_LABEL(key, label)			\
-do {						\
-	if (unlikely(((struct jump_label_key *)key)->state))		\
-		goto label;			\
-} while (0)
+static __always_inline bool static_branch(struct jump_label_key *key)
+{
+	if (unlikely(key->state))
+		return true;
+	return false;
+}
 
 static inline int jump_label_enabled(struct jump_label_key *key)
 {
@@ -78,14 +84,4 @@ static inline void jump_label_unlock(void) {}
 
 #endif
 
-#define COND_STMT(key, stmt)					\
-do {								\
-	__label__ jl_enabled;					\
-	JUMP_LABEL_ELSE_ATOMIC_READ(key, jl_enabled);		\
-	if (0) {						\
-jl_enabled:							\
-		stmt;						\
-	}							\
-} while (0)
-
 #endif
diff --git a/include/linux/jump_label_ref.h b/include/linux/jump_label_ref.h
index 8a76e89..5178696 100644
--- a/include/linux/jump_label_ref.h
+++ b/include/linux/jump_label_ref.h
@@ -7,19 +7,23 @@
 struct jump_label_key_counter {
 	atomic_t ref;
 	struct jump_label_key key;
-}
+};
 
 #ifdef HAVE_JUMP_LABEL
 
-#define JUMP_LABEL_ELSE_ATOMIC_READ(key, label, counter) JUMP_LABEL(key, label)
+static __always_inline bool static_branch_else_atomic_read(struct jump_label_key *key, atomic_t *count)
+{
+	return __static_branch(key);
+}
 
 #else /* !HAVE_JUMP_LABEL */
 
-#define JUMP_LABEL_ELSE_ATOMIC_READ(key, label, counter)		\
-do {									\
-	if (unlikely(atomic_read((atomic_t *)counter)))			\
-		goto label;						\
-} while (0)
+static __always_inline bool static_branch_else_atomic_read(struct jump_label_key *key, atomic_t *count)
+{
+	if (unlikely(atomic_read(count)))
+		return true;
+	return false;
+}
 
 #endif /* HAVE_JUMP_LABEL */
 
diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
index 94834ce..26fe115 100644
--- a/include/linux/perf_event.h
+++ b/include/linux/perf_event.h
@@ -1029,32 +1029,32 @@ perf_sw_event(u32 event_id, u64 nr, int nmi, struct pt_regs *regs, u64 addr)
 {
 	struct pt_regs hot_regs;
 
-	JUMP_LABEL_ELSE_ATOMIC_READ(&perf_swevent_enabled[event_id].key,
-				    have_event,
-				    &perf_swevent_enabled[event_id].ref);
-	return;
-
-have_event:
-	if (!regs) {
-		perf_fetch_caller_regs(&hot_regs);
-		regs = &hot_regs;
+	if (static_branch_else_atomic_read(&perf_swevent_enabled[event_id].key,
+					 &perf_swevent_enabled[event_id].ref)) {
+		if (!regs) {
+			perf_fetch_caller_regs(&hot_regs);
+			regs = &hot_regs;
+		}
+		__perf_sw_event(event_id, nr, nmi, regs, addr);
 	}
-	__perf_sw_event(event_id, nr, nmi, regs, addr);
 }
 
 extern struct jump_label_key_counter perf_task_events;
 
 static inline void perf_event_task_sched_in(struct task_struct *task)
 {
-	COND_STMT(&perf_task_events, __perf_event_task_sched_in(task));
+	if (static_branch_else_atomic_read(&perf_task_events.key,
+					   &perf_task_events.ref))
+		__perf_event_task_sched_in(task);
 }
 
 static inline
 void perf_event_task_sched_out(struct task_struct *task, struct task_struct *next)
 {
 	perf_sw_event(PERF_COUNT_SW_CONTEXT_SWITCHES, 1, 1, NULL, 0);
-
-	COND_STMT(&perf_task_events, __perf_event_task_sched_out(task, next));
+	if (static_branch_else_atomic_read(&perf_task_events.key,
+					   &perf_task_events.ref))
+		__perf_event_task_sched_out(task, next);
 }
 
 extern void perf_event_mmap(struct vm_area_struct *vma);
diff --git a/include/linux/tracepoint.h b/include/linux/tracepoint.h
index 2ff00e5..b95e99a 100644
--- a/include/linux/tracepoint.h
+++ b/include/linux/tracepoint.h
@@ -149,9 +149,7 @@ static inline void tracepoint_update_probe_range(struct tracepoint *begin,
 	extern struct tracepoint __tracepoint_##name;			\
 	static inline void trace_##name(proto)				\
 	{								\
-		JUMP_LABEL(&__tracepoint_##name.key, do_trace);		\
-		return;							\
-do_trace:								\
+		if (static_branch(&__tracepoint_##name.key))		\
 			__DO_TRACE(&__tracepoint_##name,		\
 				TP_PROTO(data_proto),			\
 				TP_ARGS(data_args),			\
diff --git a/kernel/jump_label.c b/kernel/jump_label.c
index b6d461c..b72d3cd 100644
--- a/kernel/jump_label.c
+++ b/kernel/jump_label.c
@@ -4,7 +4,6 @@
  * Copyright (C) 2009 Jason Baron <jbaron@redhat.com>
  *
  */
-#include <linux/jump_label.h>
 #include <linux/memory.h>
 #include <linux/uaccess.h>
 #include <linux/module.h>
@@ -13,6 +12,7 @@
 #include <linux/slab.h>
 #include <linux/sort.h>
 #include <linux/err.h>
+#include <linux/jump_label.h>
 
 #ifdef HAVE_JUMP_LABEL
 
-- 
1.7.1