Re: [PATCH RFC] remove jump_label optimization for perf sched events

[Gleb Natapov]

On Thu, Nov 17, 2011 at 01:49:19PM +0100, Peter Zijlstra wrote:
> On Thu, 2011-11-17 at 14:30 +0200, Gleb Natapov wrote:
> > jump_lable patching is very expensive operation that involves pausing all
> > cpus. The patching of perf_sched_events jump_label is easily controllable
> > from userspace by unprivileged user. When user runs loop like this
> > "while true; do perf stat -e cycles true; done" the performance of my
> > test application that just increments a counter for one second drops by
> > 4%. This is on a 16 cpu box with my test application using only one of
> > them. An impact on a real server doing real work will be much worse.
> > Performance of KVM PMU drops nearly 50% due to jump_lable for "perf
> > record" since KVM PMU implementation creates and destroys perf event
> > frequently.
> 
> Ideally we'd fix text_poke to not use stop_machine() we know how to, but
> we haven't had the green light from Intel/AMD yet.
> 
> Rostedt was going to implement it anyway and see if anything breaks.
> 
> Also, virt might be able to pull something smart on text_poke() dunno.
> 
> That said, I'd much rather throttle this particular jump label than
> remove it altogether, some people really don't like all this scheduler
> hot path crap.
> 
So how about throttling it like the patch below does until stop_machine()
no longer needed for patching (and it is possible that new way of patching
will still have significant overhead).

Signed-off-by: Gleb Natapov <gleb@xxxxxxxxxx>
diff --git a/include/linux/jump_label.h b/include/linux/jump_label.h
index 66f23dc..a4687f6 100644
--- a/include/linux/jump_label.h
+++ b/include/linux/jump_label.h
@@ -3,12 +3,15 @@
 
 #include <linux/types.h>
 #include <linux/compiler.h>
+#include <linux/workqueue.h>
 
 #if defined(CC_HAVE_ASM_GOTO) && defined(CONFIG_JUMP_LABEL)
 
 struct jump_label_key {
 	atomic_t enabled;
 	struct jump_entry *entries;
+	unsigned long rl_timeout;
+	struct delayed_work rl_work;
 #ifdef CONFIG_MODULES
 	struct jump_label_mod *next;
 #endif
@@ -28,9 +31,18 @@ struct module;
 #ifdef HAVE_JUMP_LABEL
 
 #ifdef CONFIG_MODULES
-#define JUMP_LABEL_INIT {{ 0 }, NULL, NULL}
+#define JUMP_LABEL_INIT {	\
+	.enabled = { 0 },	\
+	.entries = NULL,	\
+	.rl_timeout = 0,	\
+	.next = NULL,		\
+	}
 #else
-#define JUMP_LABEL_INIT {{ 0 }, NULL}
+#define JUMP_LABEL_INIT {	\
+	.enabled = { 0 },	\
+	.entries = NULL,	\
+	.rl_timeout = 0,	\
+	}
 #endif
 
 static __always_inline bool static_branch(struct jump_label_key *key)
@@ -51,6 +63,7 @@ extern void jump_label_inc(struct jump_label_key *key);
 extern void jump_label_dec(struct jump_label_key *key);
 extern bool jump_label_enabled(struct jump_label_key *key);
 extern void jump_label_apply_nops(struct module *mod);
+extern void jump_label_rate_limit(struct jump_label_key *key, unsigned long rl);
 
 #else
 
@@ -97,6 +110,10 @@ static inline int jump_label_apply_nops(struct module *mod)
 	return 0;
 }
 
+static inline void jump_label_rate_limit(struct jump_label_key *key,
+		unsigned long rl)
+{
+}
 #endif
 
 #endif
diff --git a/kernel/events/core.c b/kernel/events/core.c
index bdcd413..50bb631 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -7071,6 +7091,9 @@ void __init perf_event_init(void)
 
 	ret = init_hw_breakpoint();
 	WARN(ret, "hw_breakpoint initialization failed with: %d", ret);
+
+	/* do not patch jump label more than once per second */
+	jump_label_rate_limit(&perf_sched_events, HZ);
 }
 
 static int __init perf_event_sysfs_init(void)
diff --git a/kernel/jump_label.c b/kernel/jump_label.c
index e6f1f24..88a82d2 100644
--- a/kernel/jump_label.c
+++ b/kernel/jump_label.c
@@ -72,15 +72,39 @@ void jump_label_inc(struct jump_label_key *key)
 	jump_label_unlock();
 }
 
-void jump_label_dec(struct jump_label_key *key)
+static void __jump_label_dec(struct jump_label_key *key,
+		unsigned long rate_limit)
 {
 	if (!atomic_dec_and_mutex_lock(&key->enabled, &jump_label_mutex))
 		return;
 
-	jump_label_update(key, JUMP_LABEL_DISABLE);
+	if (rate_limit) {
+		atomic_inc(&key->enabled);
+		schedule_delayed_work(&key->rl_work, rate_limit);
+	} else
+		jump_label_update(key, JUMP_LABEL_DISABLE);
+
 	jump_label_unlock();
 }
 
+static void jump_label_update_timeout(struct work_struct *work)
+{
+	struct jump_label_key *key =
+		container_of(work, struct jump_label_key, rl_work.work);
+	__jump_label_dec(key, 0);
+}
+
+void jump_label_dec(struct jump_label_key *key)
+{
+	__jump_label_dec(key, key->rl_timeout);
+}
+
+void jump_label_rate_limit(struct jump_label_key *key, unsigned long rl)
+{
+	key->rl_timeout = rl;
+	INIT_DELAYED_WORK(&key->rl_work, jump_label_update_timeout);
+}
+
 static int addr_conflict(struct jump_entry *entry, void *start, void *end)
 {
 	if (entry->code <= (unsigned long)end &&
--
			Gleb.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/