Skip to contentSkip to conversation

jump-label: allow early jump_label_enable()

RFC PATCH Superseded Jeremy Fitzhardinge 4 participants 15 messages Oct 4, 2011 – Oct 12, 2011

Subsystems (4)

v3 of 4 Superseded Latest is v4 →

lore
Share & tools
lore
#0 lore raw

v3 cover jump-label: allow early jump_label_enable()

From: Jeremy Fitzhardinge <[email protected]>

[ Change from V2: add arch_jump_label_transform_static() for use on
code which can't possibly be currently executing (ie, a freshly loaded module), which can avoid heavyweight things like stop_machine. The default implementation is simply arch_jump_label_transform(), so only architectures which would particularly benefit need implement it (x86 and s390 in this series). ]

While trying to use the jump-label stuff for my PV ticketlock changes, I had some problems using jump labels early in the kernel's lifetime (pre-SMP).

The basic problem is that even if I enable a jump_label_key, the jump_label_init() initializer ends up nopping out all the code sites.

This series enables early use of jump labels by making jump_label_init() respect already-enabled keys.

To do this, I've dropped arch_jump_label_poke_text_early() and replaced it with arch_jump_label_transform(), allowing it to either insert an optimal nop, or leave the jump in place.

Part of this series makes sure that stop_machine() is safe to call in an early pre-SMP environment, by making it just call the function with interrupts disabled.

git://github.com/jsgf/linux-xen upstream/jump-label-noearly

Jeremy Fitzhardinge (8):
jump_label: use proper atomic_t initializer
stop_machine: make stop_machine safe and efficient to call early
jump_label: if a key has already been initialized, don't nop it out x86/jump_label: drop arch_jump_label_text_poke_early() sparc/jump_label: drop arch_jump_label_text_poke_early()
jump_label: add arch_jump_label_transform_static() to optimise
non-live code updates
s390/jump-label: add arch_jump_label_transform_static() x86/jump_label: add arch_jump_label_transform_static()

 arch/s390/kernel/jump_label.c  |   51 ++++++++++++++++++++++++---------------
 arch/sparc/kernel/jump_label.c |    8 ------
 arch/x86/kernel/jump_label.c   |   20 +++++++++++----
 include/linux/jump_label.h     |    9 ++++---
 kernel/jump_label.c            |   32 +++++++++++++++---------
 kernel/stop_machine.c          |   22 +++++++++++++++++
 6 files changed, 92 insertions(+), 50 deletions(-)
Signature · 1 line
1.7.6.4
#1 lore raw

RFC PATCH v3 1/8 jump_label: use proper atomic_t initializer

From: Jeremy Fitzhardinge <[email protected]>

ATOMIC_INIT() is the proper thing to use.

---
 include/linux/jump_label.h |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
include/linux/jump_label.h +2 -2
index 66f23dc..1213e9d 100644
@@ -28,9 +28,9 @@ struct module;
 #ifdef HAVE_JUMP_LABEL
 
 #ifdef CONFIG_MODULES
-#define JUMP_LABEL_INIT {{ 0 }, NULL, NULL}
+#define JUMP_LABEL_INIT {ATOMIC_INIT(0), NULL, NULL}
 #else
-#define JUMP_LABEL_INIT {{ 0 }, NULL}
+#define JUMP_LABEL_INIT {ATOMIC_INIT(0), NULL}
 #endif
 
 static __always_inline bool static_branch(struct jump_label_key *key)
Signature · 1 line
1.7.6.4
#2 lore raw

RFC PATCH v3 2/8 stop_machine: make stop_machine safe and efficient to call early

In reply to Jeremy Fitzhardinge

From: Jeremy Fitzhardinge <[email protected]>

Make stop_machine() safe to call early in boot, before stop_machine() has been set up, by simply calling the callback function directly if there's only one CPU online.

[ Fixes from AKPM:
- add comment
- local_irq_flags, not save_flags
- also call hard_irq_disable() for systems which need it

Tejun suggested using an explicit flag rather than just looking at the online cpu count. ]

Cc Rusty Russell <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Andrew Morton <[email protected]>
Cc H. Peter Anvin <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Steven Rostedt <[email protected]>
---
 kernel/stop_machine.c |   22 ++++++++++++++++++++++
 1 files changed, 22 insertions(+), 0 deletions(-)
kernel/stop_machine.c +22 -0
index ba5070c..d3f960a 100644
@@ -41,6 +41,7 @@ struct cpu_stopper {
 };
 
 static DEFINE_PER_CPU(struct cpu_stopper, cpu_stopper);
+static bool stop_machine_initialized = false;
 
 static void cpu_stop_init_done(struct cpu_stop_done *done, unsigned int nr_todo)
 {
@@ -386,6 +387,8 @@ static int __init cpu_stop_init(void)
 	cpu_stop_cpu_callback(&cpu_stop_cpu_notifier, CPU_ONLINE, bcpu);
 	register_cpu_notifier(&cpu_stop_cpu_notifier);
 
+	stop_machine_initialized = true;
+
 	return 0;
 }
 early_initcall(cpu_stop_init);
@@ -485,6 +488,25 @@ int __stop_machine(int (*fn)(void *), void *data, const struct cpumask *cpus)
 					    .num_threads = num_online_cpus(),
 					    .active_cpus = cpus };
 
+	if (!stop_machine_initialized) {
+		/*
+		 * Handle the case where stop_machine() is called
+		 * early in boot before stop_machine() has been
+		 * initialized.
+		 */
+ 		unsigned long flags;
+		int ret;
+
+		WARN_ON_ONCE(smdata.num_threads != 1);
+
+		local_irq_save(flags);
+		hard_irq_disable();
+		ret = (*fn)(data);
+		local_irq_restore(flags);
+
+		return ret;
+	}
+
 	/* Set the initial state and stop all online cpus. */
 	set_state(&smdata, STOPMACHINE_PREPARE);
 	return stop_cpus(cpu_online_mask, stop_machine_cpu_stop, &smdata);
Signature · 1 line
1.7.6.4
Acked-by on v3 2/8
#3 lore raw

Re: [PATCH RFC V3 2/8] stop_machine: make stop_machine safe and efficient to call early

Quoted 5 lines · Jeremy Fitzhardinge

From: Jeremy Fitzhardinge <[email protected]>

Make stop_machine() safe to call early in boot, before stop_machine() has been set up, by simply calling the callback function directly if there's only one CPU online.

Not that you need this, since it's >90% Tejun's code now, but:

Cheers,
Rusty.

#4 lore raw

Re: [PATCH RFC V3 2/8] stop_machine: make stop_machine safe and efficient to call early

Quoted 10 lines · Rusty Russell
Quoted 5 lines · Jeremy Fitzhardinge

From: Jeremy Fitzhardinge <[email protected]>

Make stop_machine() safe to call early in boot, before stop_machine() has been set up, by simply calling the callback function directly if there's only one CPU online.

Not that you need this, since it's >90% Tejun's code now, but:

hey, extra review is always useful.

Jeremy, Tejun, what's the expected merge path of these changes?

Thanks,

Ingo

#5 lore raw

Re: [PATCH RFC V3 2/8] stop_machine: make stop_machine safe and efficient to call early

Quoted 8 lines · Ingo Molnar
Quoted 6 lines · Rusty Russell
Quoted 5 lines · Jeremy Fitzhardinge

From: Jeremy Fitzhardinge <[email protected]>

Make stop_machine() safe to call early in boot, before stop_machine() has been set up, by simply calling the callback function directly if there's only one CPU online.

Quoted 6 lines
Quoted 3 lines

Not that you need this, since it's >90% Tejun's code now, but:

hey, extra review is always useful.

Jeremy, Tejun, what's the expected merge path of these changes?

Andrew has already picked up the stop_machine patch, I think. But it's pretty settled now, so it shouldn't cause any problem if it gets via multiple paths.

What about the jump-label stuff? Has that been going via tip.git in the past, or by some other path?

J

#6 lore raw

Re: [PATCH RFC V3 2/8] stop_machine: make stop_machine safe and efficient to call early

Quoted 14 lines · Jeremy Fitzhardinge
Quoted 13 lines · Ingo Molnar
Quoted 9 lines · Rusty Russell
Quoted 5 lines · Jeremy Fitzhardinge

From: Jeremy Fitzhardinge <[email protected]>

Make stop_machine() safe to call early in boot, before stop_machine() has been set up, by simply calling the callback function directly if there's only one CPU online.

Not that you need this, since it's >90% Tejun's code now, but:

hey, extra review is always useful.

Quoted 8 lines
Quoted 1 line

Jeremy, Tejun, what's the expected merge path of these changes?

Andrew has already picked up the stop_machine patch, I think. But it's pretty settled now, so it shouldn't cause any problem if it gets via multiple paths.

What about the jump-label stuff? Has that been going via tip.git in the past, or by some other path?

If it has Jason's and PeterZ's ack then it can go via any other tree in this cycle - we are not carrying jump label patches right now.

But those acks are very much desired.

Thanks,

Ingo

#7 lore raw

Re: [PATCH RFC V3 2/8] stop_machine: make stop_machine safe and efficient to call early

Quoted 3 lines · Ingo Molnar

If it has Jason's and PeterZ's ack then it can go via any other tree in this cycle - we are not carrying jump label patches right now.

I don't seem to have them in my inbox, just one or two patches of a larger series.

#8 lore raw

Re: [PATCH RFC V3 2/8] stop_machine: make stop_machine safe and efficient to call early

In reply to Ingo Molnar
Quoted 21 lines · Ingo Molnar
Quoted 19 lines · Jeremy Fitzhardinge
Quoted 8 lines · Ingo Molnar
Quoted 6 lines · Rusty Russell
Quoted 5 lines · Jeremy Fitzhardinge

From: Jeremy Fitzhardinge <[email protected]>

Make stop_machine() safe to call early in boot, before stop_machine() has been set up, by simply calling the callback function directly if there's only one CPU online.

Quoted 6 lines
Quoted 3 lines

Not that you need this, since it's >90% Tejun's code now, but:

hey, extra review is always useful.

Jeremy, Tejun, what's the expected merge path of these changes?

Andrew has already picked up the stop_machine patch, I think. But it's pretty settled now, so it shouldn't cause any problem if it gets via multiple paths.

Quoted 6 lines
Quoted 2 lines

What about the jump-label stuff? Has that been going via tip.git in the past, or by some other path?

If it has Jason's and PeterZ's ack then it can go via any other tree in this cycle - we are not carrying jump label patches right now.

But those acks are very much desired.

Jason has already Acked it, but then pointed out a couple of problems to address.

I'll add PeterZ to the next repost.

J

#9 lore raw

RFC PATCH v3 3/8 jump_label: if a key has already been initialized, don't nop it out

In reply to Jeremy Fitzhardinge

From: Jeremy Fitzhardinge <[email protected]>

If a key has been enabled before jump_label_init() is called, don't nop it out.

This removes arch_jump_label_text_poke_early() (which can only nop out a site) and uses arch_jump_label_transform() instead.

---
 include/linux/jump_label.h |    3 +--
 kernel/jump_label.c        |   20 ++++++++------------
 2 files changed, 9 insertions(+), 14 deletions(-)
include/linux/jump_label.h +1 -2
index 1213e9d..12e804e 100644
@@ -44,8 +44,7 @@ extern struct jump_entry __stop___jump_table[];
 extern void jump_label_lock(void);
 extern void jump_label_unlock(void);
 extern void arch_jump_label_transform(struct jump_entry *entry,
-				 enum jump_label_type type);
-extern void arch_jump_label_text_poke_early(jump_label_t addr);
+				      enum jump_label_type type);
 extern int jump_label_text_reserved(void *start, void *end);
 extern void jump_label_inc(struct jump_label_key *key);
 extern void jump_label_dec(struct jump_label_key *key);
kernel/jump_label.c +8 -12
index a8ce450..059202d5 100644
@@ -121,13 +121,6 @@ static void __jump_label_update(struct jump_label_key *key,
 	}
 }
 
-/*
- * Not all archs need this.
- */
-void __weak arch_jump_label_text_poke_early(jump_label_t addr)
-{
-}
-
 static __init int jump_label_init(void)
 {
 	struct jump_entry *iter_start = __start___jump_table;
@@ -139,12 +132,15 @@ static __init int jump_label_init(void)
 	jump_label_sort_entries(iter_start, iter_stop);
 
 	for (iter = iter_start; iter < iter_stop; iter++) {
-		arch_jump_label_text_poke_early(iter->code);
-		if (iter->key == (jump_label_t)(unsigned long)key)
+		struct jump_label_key *iterk;
+
+		iterk = (struct jump_label_key *)(unsigned long)iter->key;
+		arch_jump_label_transform(iter, jump_label_enabled(iterk) ?
+					  JUMP_LABEL_ENABLE : JUMP_LABEL_DISABLE);
+		if (iterk == key)
 			continue;
 
-		key = (struct jump_label_key *)(unsigned long)iter->key;
-		atomic_set(&key->enabled, 0);
+		key = iterk;
 		key->entries = iter;
 #ifdef CONFIG_MODULES
 		key->next = NULL;
@@ -212,7 +208,7 @@ void jump_label_apply_nops(struct module *mod)
 		return;
 
 	for (iter = iter_start; iter < iter_stop; iter++)
-		arch_jump_label_text_poke_early(iter->code);
+		arch_jump_label_transform(iter, JUMP_LABEL_DISABLE);
 }
 
 static int jump_label_add_module(struct module *mod)
Signature · 1 line
1.7.6.4
#10 lore raw

RFC PATCH v3 4/8 x86/jump_label: drop arch_jump_label_text_poke_early()

In reply to Jeremy Fitzhardinge

From: Jeremy Fitzhardinge <[email protected]>

It is no longer used.

---
 arch/x86/kernel/jump_label.c |    6 ------
 1 files changed, 0 insertions(+), 6 deletions(-)
arch/x86/kernel/jump_label.c +0 -6
index 3fee346..2ad0298 100644
@@ -42,10 +42,4 @@ void arch_jump_label_transform(struct jump_entry *entry,
 	put_online_cpus();
 }
 
-void arch_jump_label_text_poke_early(jump_label_t addr)
-{
-	text_poke_early((void *)addr, ideal_nops[NOP_ATOMIC5],
-			JUMP_LABEL_NOP_SIZE);
-}
-
 #endif
Signature · 1 line
1.7.6.4
#11 lore raw

RFC PATCH v3 5/8 sparc/jump_label: drop arch_jump_label_text_poke_early()

In reply to Jeremy Fitzhardinge

From: Jeremy Fitzhardinge <[email protected]>

It is no longer used.

---
 arch/sparc/kernel/jump_label.c |    8 --------
 1 files changed, 0 insertions(+), 8 deletions(-)
arch/sparc/kernel/jump_label.c +0 -8
index ea2dafc..971fd43 100644
@@ -36,12 +36,4 @@ void arch_jump_label_transform(struct jump_entry *entry,
 	put_online_cpus();
 }
 
-void arch_jump_label_text_poke_early(jump_label_t addr)
-{
-	u32 *insn_p = (u32 *) (unsigned long) addr;
-
-	*insn_p = 0x01000000;
-	flushi(insn_p);
-}
-
 #endif
Signature · 1 line
1.7.6.4
#12 lore raw

RFC PATCH v3 6/8 jump_label: add arch_jump_label_transform_static() to optimise non-live code updates

In reply to Jeremy Fitzhardinge

From: Jeremy Fitzhardinge <[email protected]>

When updating a newly loaded module, the code is definitely not yet executing on any processor, so it can be updated with no need for any heavyweight synchronization.

This patch adds arch_jump_label_static() which is implemented as arch_jump_label_transform() by default, but architectures can override it if it avoids, say, a call to stop_machine().

---
 include/linux/jump_label.h |    2 ++
 kernel/jump_label.c        |   14 +++++++++++++-
 2 files changed, 15 insertions(+), 1 deletions(-)
include/linux/jump_label.h +2 -0
index 12e804e..56594e4 100644
@@ -45,6 +45,8 @@ extern void jump_label_lock(void);
 extern void jump_label_unlock(void);
 extern void arch_jump_label_transform(struct jump_entry *entry,
 				      enum jump_label_type type);
+extern void arch_jump_label_transform_static(struct jump_entry *entry,
+					     enum jump_label_type type);
 extern int jump_label_text_reserved(void *start, void *end);
 extern void jump_label_inc(struct jump_label_key *key);
 extern void jump_label_dec(struct jump_label_key *key);
kernel/jump_label.c +13 -1
index 059202d5..f0a9bef 100644
@@ -104,6 +104,18 @@ static int __jump_label_text_reserved(struct jump_entry *iter_start,
 	return 0;
 }
 
+/* 
+ * Update code which is definitely not currently executing.
+ * Architectures which need heavyweight synchronization to modify
+ * running code can override this to make the non-live update case
+ * cheaper.
+ */
+void __weak arch_jump_label_transform_static(struct jump_entry *entry,
+					    enum jump_label_type type)
+{
+	arch_jump_label_transform(entry, type);	
+}
+
 static void __jump_label_update(struct jump_label_key *key,
 				struct jump_entry *entry,
 				struct jump_entry *stop, int enable)
@@ -208,7 +220,7 @@ void jump_label_apply_nops(struct module *mod)
 		return;
 
 	for (iter = iter_start; iter < iter_stop; iter++)
-		arch_jump_label_transform(iter, JUMP_LABEL_DISABLE);
+		arch_jump_label_transform_static(iter, JUMP_LABEL_DISABLE);
 }
 
 static int jump_label_add_module(struct module *mod)
Signature · 1 line
1.7.6.4
#13 lore raw

RFC PATCH v3 7/8 s390/jump-label: add arch_jump_label_transform_static()

In reply to Jeremy Fitzhardinge

From: Jeremy Fitzhardinge <[email protected]>

This allows jump-label entries to be cheaply updated on code which is not yet live.

---
 arch/s390/kernel/jump_label.c |   51 +++++++++++++++++++++++++----------------
 1 files changed, 31 insertions(+), 20 deletions(-)
arch/s390/kernel/jump_label.c +31 -20
index 44cc06b..b987ab2 100644
@@ -18,26 +18,15 @@ struct insn {
 } __packed;
 
 struct insn_args {
-	unsigned long *target;
-	struct insn *insn;
-	ssize_t size;
+	struct jump_entry *entry;
+	enum jump_label_type type;
 };
 
-static int __arch_jump_label_transform(void *data)
+static void __jump_label_transform(struct jump_entry *entry,
+				   enum jump_label_type type)
 {
-	struct insn_args *args = data;
-	int rc;
-
-	rc = probe_kernel_write(args->target, args->insn, args->size);
-	WARN_ON_ONCE(rc < 0);
-	return 0;
-}
-
-void arch_jump_label_transform(struct jump_entry *entry,
-			       enum jump_label_type type)
-{
-	struct insn_args args;
 	struct insn insn;
+	int rc;
 
 	if (type == JUMP_LABEL_ENABLE) {
 		/* brcl 15,offset */
@@ -49,11 +38,33 @@ void arch_jump_label_transform(struct jump_entry *entry,
 		insn.offset = 0;
 	}
 
-	args.target = (void *) entry->code;
-	args.insn = &insn;
-	args.size = JUMP_LABEL_NOP_SIZE;
+	rc = probe_kernel_write((void *)entry->code, &insn, JUMP_LABEL_NOP_SIZE);
+	WARN_ON_ONCE(rc < 0);
+}
 
-	stop_machine(__arch_jump_label_transform, &args, NULL);
+static int __sm_arch_jump_label_transform(void *data)
+{
+	struct insn_args *args = data;
+
+	__jump_label_transform(args->entry, args->type);
+	return 0;
+}
+
+void arch_jump_label_transform(struct jump_entry *entry,
+			       enum jump_label_type type)
+{
+	struct insn_args args;
+
+	args.entry = entry;
+	args.type = type;
+
+	stop_machine(__sm_arch_jump_label_transform, &args, NULL);
+}
+
+void arch_jump_label_transform_static(struct jump_entry *entry,
+				      enum jump_label_type type)
+{
+	__jump_label_transform(entry, type);
 }
 
 #endif
Signature · 1 line
1.7.6.4
#14 lore raw

RFC PATCH v3 8/8 x86/jump_label: add arch_jump_label_transform_static()

In reply to Jeremy Fitzhardinge

From: Jeremy Fitzhardinge <[email protected]>

This allows jump-label entries to be cheaply updated on code which is not yet live.

---
 arch/x86/kernel/jump_label.c |   20 +++++++++++++++++---
 1 files changed, 17 insertions(+), 3 deletions(-)
arch/x86/kernel/jump_label.c +17 -3
index 2ad0298..ea9d5f2f 100644
@@ -24,8 +24,9 @@ union jump_code_union {
 	} __attribute__((packed));
 };
 
-void arch_jump_label_transform(struct jump_entry *entry,
-			       enum jump_label_type type)
+static void __jump_label_transform(struct jump_entry *entry,
+				   enum jump_label_type type,
+				   void *(*poker)(void *, const void *, size_t))
 {
 	union jump_code_union code;
 
@@ -35,11 +36,24 @@ void arch_jump_label_transform(struct jump_entry *entry,
 				(entry->code + JUMP_LABEL_NOP_SIZE);
 	} else
 		memcpy(&code, ideal_nops[NOP_ATOMIC5], JUMP_LABEL_NOP_SIZE);
+
+	(*poker)((void *)entry->code, &code, JUMP_LABEL_NOP_SIZE);
+}
+
+void arch_jump_label_transform(struct jump_entry *entry,
+			       enum jump_label_type type)
+{
 	get_online_cpus();
 	mutex_lock(&text_mutex);
-	text_poke_smp((void *)entry->code, &code, JUMP_LABEL_NOP_SIZE);
+	__jump_label_transform(entry, type, text_poke_smp);
 	mutex_unlock(&text_mutex);
 	put_online_cpus();
 }
 
+void arch_jump_label_transform_static(struct jump_entry *entry,
+				      enum jump_label_type type)
+{
+	__jump_label_transform(entry, type, text_poke_early);
+}
+
 #endif
Signature · 1 line
1.7.6.4