thr3ads.net - Xen devel - [Xen-devel] [PATCH RFC V4 00/10] jump-label: allow early jump_label

If this information is useful, please help other people find it:
Share via:

Jeremy Fitzhardinge

2011-Oct-13 00:08 UTC

[Xen-devel] [PATCH RFC V4 00/10] jump-label: allow early jump_label_enable()

From: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>

[ Change from V3: Move jump_label_init() much earlier, so that the
  jump_label mechanism will be reliabily initialized before use. ]

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 (10):
  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()
  x86/jump_label: use GENERIC_NOP5_ATOMIC instead of jmp5 +0
  jump-label: initialize jump-label subsystem much earlier

 arch/s390/kernel/jump_label.c     |   51 ++++++++++++++++++++++--------------
 arch/sparc/kernel/jump_label.c    |    8 ------
 arch/x86/include/asm/jump_label.h |    3 +-
 arch/x86/kernel/jump_label.c      |   20 ++++++++++----
 include/linux/jump_label.h        |   23 ++++++++++------
 init/main.c                       |    1 +
 kernel/jump_label.c               |   37 +++++++++++++++-----------
 kernel/stop_machine.c             |   22 ++++++++++++++++
 8 files changed, 105 insertions(+), 60 deletions(-)

-- 
1.7.6.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Jeremy Fitzhardinge

2011-Oct-13 00:08 UTC

head link

[Xen-devel] [PATCH RFC V4 01/10] jump_label: use proper atomic_t initializer

From: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>

ATOMIC_INIT() is the proper thing to use.

Signed-off-by: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
Acked-by: Jason Baron <jbaron@redhat.com>
---
 include/linux/jump_label.h |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/linux/jump_label.h b/include/linux/jump_label.h
index 66f23dc..1213e9d 100644
--- a/include/linux/jump_label.h
+++ b/include/linux/jump_label.h
@@ -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)
-- 
1.7.6.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Jeremy Fitzhardinge

2011-Oct-13 00:08 UTC

head link

[Xen-devel] [PATCH RFC V4 02/10] stop_machine: make stop_machine safe and efficient to call early

From: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>

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. ]

Signed-off-by: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
Acked-by: Tejun Heo <tj@kernel.org>
Cc: Rusty Russell <rusty@rustcorp.com.au>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: H. Peter Anvin <hpa@linux.intel.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Steven Rostedt <rostedt@goodmis.org>
---
 kernel/stop_machine.c |   22 ++++++++++++++++++++++
 1 files changed, 22 insertions(+), 0 deletions(-)

diff --git a/kernel/stop_machine.c b/kernel/stop_machine.c
index ba5070c..d3f960a 100644
--- a/kernel/stop_machine.c
+++ b/kernel/stop_machine.c
@@ -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);
-- 
1.7.6.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Jeremy Fitzhardinge

2011-Oct-13 00:08 UTC

head link

[Xen-devel] [PATCH RFC V4 03/10] jump_label: if a key has already been initialized, don''t nop it out

From: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>

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.

Signed-off-by: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
Acked-by: Jason Baron <jbaron@redhat.com>
---
 include/linux/jump_label.h |    3 +--
 kernel/jump_label.c        |   20 ++++++++------------
 2 files changed, 9 insertions(+), 14 deletions(-)

diff --git a/include/linux/jump_label.h b/include/linux/jump_label.h
index 1213e9d..12e804e 100644
--- a/include/linux/jump_label.h
+++ b/include/linux/jump_label.h
@@ -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);
diff --git a/kernel/jump_label.c b/kernel/jump_label.c
index a8ce450..059202d5 100644
--- a/kernel/jump_label.c
+++ b/kernel/jump_label.c
@@ -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)
-- 
1.7.6.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Jeremy Fitzhardinge

2011-Oct-13 00:08 UTC

head link

[Xen-devel] [PATCH RFC V4 04/10] x86/jump_label: drop arch_jump_label_text_poke_early()

From: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>

It is no longer used.

Signed-off-by: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
Acked-by: Jason Baron <jbaron@redhat.com>
---
 arch/x86/kernel/jump_label.c |    6 ------
 1 files changed, 0 insertions(+), 6 deletions(-)

diff --git a/arch/x86/kernel/jump_label.c b/arch/x86/kernel/jump_label.c
index 3fee346..2ad0298 100644
--- a/arch/x86/kernel/jump_label.c
+++ b/arch/x86/kernel/jump_label.c
@@ -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
-- 
1.7.6.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Jeremy Fitzhardinge

2011-Oct-13 00:08 UTC

head link

[Xen-devel] [PATCH RFC V4 05/10] sparc/jump_label: drop arch_jump_label_text_poke_early()

From: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>

It is no longer used.

Signed-off-by: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
Acked-by: Jason Baron <jbaron@redhat.com>
Cc: David Miller <davem@davemloft.net>
---
 arch/sparc/kernel/jump_label.c |    8 --------
 1 files changed, 0 insertions(+), 8 deletions(-)

diff --git a/arch/sparc/kernel/jump_label.c b/arch/sparc/kernel/jump_label.c
index ea2dafc..971fd43 100644
--- a/arch/sparc/kernel/jump_label.c
+++ b/arch/sparc/kernel/jump_label.c
@@ -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
-- 
1.7.6.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Jeremy Fitzhardinge

2011-Oct-13 00:08 UTC

head link

[Xen-devel] [PATCH RFC V4 06/10] jump_label: add arch_jump_label_transform_static() to optimise non-live code updates

From: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>

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().

Signed-off-by: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
Acked-by: Jason Baron <jbaron@redhat.com>
---
 include/linux/jump_label.h |    2 ++
 kernel/jump_label.c        |   18 +++++++++++++++---
 2 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/include/linux/jump_label.h b/include/linux/jump_label.h
index 12e804e..56594e4 100644
--- a/include/linux/jump_label.h
+++ b/include/linux/jump_label.h
@@ -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);
diff --git a/kernel/jump_label.c b/kernel/jump_label.c
index 059202d5..ff2028f 100644
--- a/kernel/jump_label.c
+++ b/kernel/jump_label.c
@@ -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)
@@ -135,8 +147,8 @@ static __init int jump_label_init(void)
 		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);
+		arch_jump_label_transform_static(iter, jump_label_enabled(iterk) ?
+						 JUMP_LABEL_ENABLE : JUMP_LABEL_DISABLE);
 		if (iterk == key)
 			continue;
 
@@ -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)
-- 
1.7.6.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Jeremy Fitzhardinge

2011-Oct-13 00:08 UTC

head link

[Xen-devel] [PATCH RFC V4 07/10] s390/jump-label: add arch_jump_label_transform_static()

From: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>

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

Signed-off-by: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
Acked-by: Jason Baron <jbaron@redhat.com>
Cc: Jan Glauber <jang@linux.vnet.ibm.com>
---
 arch/s390/kernel/jump_label.c |   51 +++++++++++++++++++++++++----------------
 1 files changed, 31 insertions(+), 20 deletions(-)

diff --git a/arch/s390/kernel/jump_label.c b/arch/s390/kernel/jump_label.c
index 44cc06b..b987ab2 100644
--- a/arch/s390/kernel/jump_label.c
+++ b/arch/s390/kernel/jump_label.c
@@ -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
-- 
1.7.6.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Jeremy Fitzhardinge

2011-Oct-13 00:08 UTC

head link

[Xen-devel] [PATCH RFC V4 08/10] x86/jump_label: add arch_jump_label_transform_static()

From: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>

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

Signed-off-by: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
Acked-by: Jason Baron <jbaron@redhat.com>
---
 arch/x86/kernel/jump_label.c |   20 +++++++++++++++++---
 1 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kernel/jump_label.c b/arch/x86/kernel/jump_label.c
index 2ad0298..ea9d5f2f 100644
--- a/arch/x86/kernel/jump_label.c
+++ b/arch/x86/kernel/jump_label.c
@@ -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
-- 
1.7.6.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Jeremy Fitzhardinge

2011-Oct-13 00:08 UTC

head link

[Xen-devel] [PATCH RFC V4 09/10] x86/jump_label: use GENERIC_NOP5_ATOMIC instead of jmp5 +0

From: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>

GENERIC_NOP5_ATOMIC should make a better nop.

Signed-off-by: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
---
 arch/x86/include/asm/jump_label.h |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/arch/x86/include/asm/jump_label.h
b/arch/x86/include/asm/jump_label.h
index a32b18c..189100e 100644
--- a/arch/x86/include/asm/jump_label.h
+++ b/arch/x86/include/asm/jump_label.h
@@ -4,12 +4,13 @@
 #ifdef __KERNEL__
 
 #include <linux/types.h>
+#include <linux/stringify.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_INITIAL_NOP ".byte "
__stringify(GENERIC_NOP5_ATOMIC) "\n\t"
 
 static __always_inline bool arch_static_branch(struct jump_label_key *key)
 {
-- 
1.7.6.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Jeremy Fitzhardinge

2011-Oct-13 00:08 UTC

head link

[Xen-devel] [PATCH RFC V4 10/10] jump-label: initialize jump-label subsystem much earlier

From: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>

Initialize jump_labels much earlier, we can use them.

Signed-off-by: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
---
 include/linux/jump_label.h |   14 +++++++++-----
 init/main.c                |    1 +
 kernel/jump_label.c        |    5 +----
 3 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/include/linux/jump_label.h b/include/linux/jump_label.h
index 56594e4..388b0d4 100644
--- a/include/linux/jump_label.h
+++ b/include/linux/jump_label.h
@@ -16,7 +16,7 @@ struct jump_label_key {
 
 # include <asm/jump_label.h>
 # define HAVE_JUMP_LABEL
-#endif
+#endif	/* CC_HAVE_ASM_GOTO && CONFIG_JUMP_LABEL */
 
 enum jump_label_type {
 	JUMP_LABEL_DISABLE = 0,
@@ -41,6 +41,7 @@ static __always_inline bool static_branch(struct
jump_label_key *key)
 extern struct jump_entry __start___jump_table[];
 extern struct jump_entry __stop___jump_table[];
 
+extern void jump_label_init(void);
 extern void jump_label_lock(void);
 extern void jump_label_unlock(void);
 extern void arch_jump_label_transform(struct jump_entry *entry,
@@ -53,7 +54,7 @@ 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);
 
-#else
+#else  /* !HAVE_JUMP_LABEL */
 
 #include <linux/atomic.h>
 
@@ -63,6 +64,10 @@ struct jump_label_key {
 	atomic_t enabled;
 };
 
+static __always_inline void jump_label_init(void)
+{
+}
+
 static __always_inline bool static_branch(struct jump_label_key *key)
 {
 	if (unlikely(atomic_read(&key->enabled)))
@@ -97,7 +102,6 @@ static inline int jump_label_apply_nops(struct module *mod)
 {
 	return 0;
 }
+#endif	/* HAVE_JUMP_LABEL */
 
-#endif
-
-#endif
+#endif	/* _LINUX_JUMP_LABEL_H */
diff --git a/init/main.c b/init/main.c
index 2a9b88a..f4094ed 100644
--- a/init/main.c
+++ b/init/main.c
@@ -637,6 +637,7 @@ asmlinkage void __init start_kernel(void)
 	acpi_early_init(); /* before LAPIC and SMP init */
 	sfi_init_late();
 
+	jump_label_init();
 	ftrace_init();
 
 	/* Do the rest non-__init''ed, we''re now alive */
diff --git a/kernel/jump_label.c b/kernel/jump_label.c
index ff2028f..bbdfe2a 100644
--- a/kernel/jump_label.c
+++ b/kernel/jump_label.c
@@ -133,7 +133,7 @@ static void __jump_label_update(struct jump_label_key *key,
 	}
 }
 
-static __init int jump_label_init(void)
+void __init jump_label_init(void)
 {
 	struct jump_entry *iter_start = __start___jump_table;
 	struct jump_entry *iter_stop = __stop___jump_table;
@@ -159,10 +159,7 @@ static __init int jump_label_init(void)
 #endif
 	}
 	jump_label_unlock();
-
-	return 0;
 }
-early_initcall(jump_label_init);
 
 #ifdef CONFIG_MODULES
 
-- 
1.7.6.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Peter Zijlstra

2011-Oct-13 10:15 UTC

head link

[Xen-devel] Re: [PATCH RFC V4 04/10] x86/jump_label: drop arch_jump_label_text_poke_early()

On Wed, 2011-10-12 at 17:08 -0700, Jeremy Fitzhardinge
wrote:> From: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
> 
> It is no longer used.
> 
> Signed-off-by: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
> Acked-by: Jason Baron <jbaron@redhat.com>
> ---
>  arch/x86/kernel/jump_label.c |    6 ------
>  1 files changed, 0 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/x86/kernel/jump_label.c b/arch/x86/kernel/jump_label.c
> index 3fee346..2ad0298 100644
> --- a/arch/x86/kernel/jump_label.c
> +++ b/arch/x86/kernel/jump_label.c
> @@ -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
This was the (first and) last usage site outside of alternative.c, so at
the very least we should clean up arch/x86/include/asm/alternative.h.

However, since stop_machine now works early, is there any reason to keep
text_poke_early() itself? Shouldn''t we also convert
alternative.c''s use
of text_poke_early and simply kill the thing off?

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Peter Zijlstra

2011-Oct-13 10:32 UTC

head link

[Xen-devel] Re: [PATCH RFC V4 06/10] jump_label: add arch_jump_label_transform_static() to optimise non-live code updates

On Wed, 2011-10-12 at 17:08 -0700, Jeremy Fitzhardinge
wrote:> From: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
> 
> 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().
> 
> Signed-off-by: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
> Acked-by: Jason Baron <jbaron@redhat.com>
> ---
>  include/linux/jump_label.h |    2 ++
>  kernel/jump_label.c        |   18 +++++++++++++++---
>  2 files changed, 17 insertions(+), 3 deletions(-)
> 
> diff --git a/include/linux/jump_label.h b/include/linux/jump_label.h
> index 12e804e..56594e4 100644
> --- a/include/linux/jump_label.h
> +++ b/include/linux/jump_label.h
> @@ -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);
> diff --git a/kernel/jump_label.c b/kernel/jump_label.c
> index 059202d5..ff2028f 100644
> --- a/kernel/jump_label.c
> +++ b/kernel/jump_label.c
> @@ -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)
> @@ -135,8 +147,8 @@ static __init int jump_label_init(void)
>  		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);
> +		arch_jump_label_transform_static(iter, jump_label_enabled(iterk) ?
> +						 JUMP_LABEL_ENABLE : JUMP_LABEL_DISABLE);
>  		if (iterk == key)
>  			continue;
>  
> @@ -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);
>  }
>  
So I got myself a little confused wrt the early jump_label_apply_nops()
call and the MODULE_COMING notifiers.

It looks to me like jump_label_apply_nops() is called way early and is
in fact called before _any_ of the module code has had a chance of
running. However it simply NOPs out all jump_labels.

The jump_label_add_module() thing, which is ran on the MODULE_COMING
callback will then set up stuff and do the proper patch-up.

Now the only bit of the module text that can be ran between those two
calls appears to be the module argument parsing stuff, but since
jump_labels are non-functional it can''t rely on them, so why do we do
the early patch up again?



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Peter Zijlstra

2011-Oct-13 10:36 UTC

head link

[Xen-devel] Re: [PATCH RFC V4 08/10] x86/jump_label: add arch_jump_label_transform_static()

On Wed, 2011-10-12 at 17:08 -0700, Jeremy Fitzhardinge
wrote:> +       __jump_label_transform(entry, type, text_poke_early);
Ah, a new text_poke_early() user, ignore my earlier comment on it.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Peter Zijlstra

2011-Oct-13 10:43 UTC

head link

[Xen-devel] Re: [PATCH RFC V4 10/10] jump-label: initialize jump-label subsystem much earlier

On Wed, 2011-10-12 at 17:08 -0700, Jeremy Fitzhardinge
wrote:> From: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
> 
> Initialize jump_labels much earlier, we can use them.
We can use them, where? how? what?, that sentence just begs for more.
> diff --git a/init/main.c b/init/main.c
> index 2a9b88a..f4094ed 100644
> --- a/init/main.c
> +++ b/init/main.c
> @@ -637,6 +637,7 @@ asmlinkage void __init start_kernel(void)
>  	acpi_early_init(); /* before LAPIC and SMP init */
>  	sfi_init_late();
>  
> +	jump_label_init();
>  	ftrace_init();
>  
>  	/* Do the rest non-__init''ed, we''re now alive */
Can we push them much earlier still? If possible I''d like them to be
before sched_init() so that I might use them there, if not possible, at
the very least before enabling interrupts would be nice.



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Jason Baron

2011-Oct-13 13:54 UTC

head link

[Xen-devel] Re: [PATCH RFC V4 06/10] jump_label: add arch_jump_label_transform_static() to optimise non-live code updates

On Thu, Oct 13, 2011 at 12:32:34PM +0200, Peter Zijlstra
wrote:> On Wed, 2011-10-12 at 17:08 -0700, Jeremy Fitzhardinge wrote:
> > From: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
> > 
> > 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().
> > 
> > Signed-off-by: Jeremy Fitzhardinge
<jeremy.fitzhardinge@citrix.com>
> > Acked-by: Jason Baron <jbaron@redhat.com>
> > ---
> >  include/linux/jump_label.h |    2 ++
> >  kernel/jump_label.c        |   18 +++++++++++++++---
> >  2 files changed, 17 insertions(+), 3 deletions(-)
> > 
> > diff --git a/include/linux/jump_label.h b/include/linux/jump_label.h
> > index 12e804e..56594e4 100644
> > --- a/include/linux/jump_label.h
> > +++ b/include/linux/jump_label.h
> > @@ -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);
> > diff --git a/kernel/jump_label.c b/kernel/jump_label.c
> > index 059202d5..ff2028f 100644
> > --- a/kernel/jump_label.c
> > +++ b/kernel/jump_label.c
> > @@ -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)
> > @@ -135,8 +147,8 @@ static __init int jump_label_init(void)
> >  		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);
> > +		arch_jump_label_transform_static(iter, jump_label_enabled(iterk) ?
> > +						 JUMP_LABEL_ENABLE : JUMP_LABEL_DISABLE);
> >  		if (iterk == key)
> >  			continue;
> >  
> > @@ -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);
> >  }
> >  
> 
> So I got myself a little confused wrt the early jump_label_apply_nops()
> call and the MODULE_COMING notifiers.
> 
> It looks to me like jump_label_apply_nops() is called way early and is
> in fact called before _any_ of the module code has had a chance of
> running. However it simply NOPs out all jump_labels.
> 
yes.
> The jump_label_add_module() thing, which is ran on the MODULE_COMING
> callback will then set up stuff and do the proper patch-up.
> 
yes, only for the enabled ones.
> Now the only bit of the module text that can be ran between those two
> calls appears to be the module argument parsing stuff, but since
> jump_labels are non-functional it can''t rely on them, so why do we
do
> the early patch up again?
> 
> 
The ''early patch'' is for putting in the
''ideal'' no-ops into the module
code. These ''ideal'' no-ops are discovered at run-time, not
boot-time.

The code is optimized (hopefully) for the most common case. The
jump labels are by nature expected to be off, and by patching them early
like this, at least for x86, we can avoid the stop machine calls. So its
the combination of most are expected to be off and no sense to call extra
stop machines that lead the code to its present state.

Thanks,

-Jason

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Jason Baron

2011-Oct-13 13:59 UTC

head link

[Xen-devel] Re: [PATCH RFC V4 10/10] jump-label: initialize jump-label subsystem much earlier

On Thu, Oct 13, 2011 at 12:43:48PM +0200, Peter Zijlstra
wrote:> On Wed, 2011-10-12 at 17:08 -0700, Jeremy Fitzhardinge wrote:
> > From: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
> > 
> > Initialize jump_labels much earlier, we can use them.
> 
> We can use them, where? how? what?, that sentence just begs for more.
> 
> > diff --git a/init/main.c b/init/main.c
> > index 2a9b88a..f4094ed 100644
> > --- a/init/main.c
> > +++ b/init/main.c
> > @@ -637,6 +637,7 @@ asmlinkage void __init start_kernel(void)
> >  	acpi_early_init(); /* before LAPIC and SMP init */
> >  	sfi_init_late();
> >  
> > +	jump_label_init();
> >  	ftrace_init();
> >  
> >  	/* Do the rest non-__init''ed, we''re now alive */
> 
> Can we push them much earlier still? If possible I''d like them to
be
> before sched_init() so that I might use them there, if not possible, at
> the very least before enabling interrupts would be nice.
> 
> 
Yes, earlier still would be good. Also, it still bothers me a bit that
jump_label_inc()/dec() could be called and the branch not updated until
the jump label initialization. I feel like there should be a WARN() for
this case...ie:

jump_label_inc()/dec() {
	if (!jump_label_initialized)
		WARN("calling branch update before subsystem initialization");
}






_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Peter Zijlstra

2011-Oct-13 15:29 UTC

head link

[Xen-devel] Re: [PATCH RFC V4 06/10] jump_label: add arch_jump_label_transform_static() to optimise non-live code updates

On Thu, 2011-10-13 at 09:54 -0400, Jason Baron wrote:> 
> > So I got myself a little confused wrt the early
jump_label_apply_nops()
> > call and the MODULE_COMING notifiers.
> > 
> > It looks to me like jump_label_apply_nops() is called way early and is
> > in fact called before _any_ of the module code has had a chance of
> > running. However it simply NOPs out all jump_labels.
> > 
> 
> yes.
> 
> > The jump_label_add_module() thing, which is ran on the MODULE_COMING
> > callback will then set up stuff and do the proper patch-up.
> > 
> 
> yes, only for the enabled ones.
But we could make it nop out the disabled ones quite trivially.
> > Now the only bit of the module text that can be ran between those two
> > calls appears to be the module argument parsing stuff, but since
> > jump_labels are non-functional it can''t rely on them, so why
do we do
> > the early patch up again?
> > 
> > 
> 
> The ''early patch'' is for putting in the
''ideal'' no-ops into the module
> code. These ''ideal'' no-ops are discovered at run-time,
not boot-time.
Sure, but since we can''t use static_branch() and friends from very
early
module code (arg parsing) anyway, it doesn''t matter what NOP they
encounter, so we might as well do something like the below, right?
> The code is optimized (hopefully) for the most common case. The
> jump labels are by nature expected to be off, 
I actually need them to be either way.. no preference between on or off
just a means of very _very_ infrequent runtime change in behaviour.

If we can push jump_label init to before sched_init() all I need is a
static_branch() without the unlikely() in to avoid GCC out-of-lining the
branch.
> and by patching them early
> like this, at least for x86, we can avoid the stop machine calls. So its
> the combination of most are expected to be off and no sense to call extra
> stop machines that lead the code to its present state.
But we could use arch_jump_label_transform_static because its before we
actually execute any module text (sans the arg crap) which is
stomp-machine free, removing that obstacle.

Or am I confused more?

---
 arch/mips/kernel/module.c  |    3 ---
 arch/sparc/kernel/module.c |    3 ---
 arch/x86/kernel/module.c   |    3 ---
 include/linux/jump_label.h |    6 ------
 kernel/jump_label.c        |   22 +++++++++++++---------
 5 files changed, 13 insertions(+), 24 deletions(-)

Index: linux-2.6/arch/mips/kernel/module.c
==================================================================---
linux-2.6.orig/arch/mips/kernel/module.c
+++ linux-2.6/arch/mips/kernel/module.c
@@ -368,9 +368,6 @@ int module_finalize(const Elf_Ehdr *hdr,
 	const Elf_Shdr *s;
 	char *secstrings = (void *)hdr + sechdrs[hdr->e_shstrndx].sh_offset;
 
-	/* Make jump label nops. */
-	jump_label_apply_nops(me);
-
 	INIT_LIST_HEAD(&me->arch.dbe_list);
 	for (s = sechdrs; s < sechdrs + hdr->e_shnum; s++) {
 		if (strcmp("__dbe_table", secstrings + s->sh_name) != 0)
Index: linux-2.6/arch/sparc/kernel/module.c
==================================================================---
linux-2.6.orig/arch/sparc/kernel/module.c
+++ linux-2.6/arch/sparc/kernel/module.c
@@ -207,9 +207,6 @@ int module_finalize(const Elf_Ehdr *hdr,
 		    const Elf_Shdr *sechdrs,
 		    struct module *me)
 {
-	/* make jump label nops */
-	jump_label_apply_nops(me);
-
 	/* Cheetah''s I-cache is fully coherent.  */
 	if (tlb_type == spitfire) {
 		unsigned long va;
Index: linux-2.6/arch/x86/kernel/module.c
==================================================================---
linux-2.6.orig/arch/x86/kernel/module.c
+++ linux-2.6/arch/x86/kernel/module.c
@@ -194,9 +194,6 @@ int module_finalize(const Elf_Ehdr *hdr,
 		apply_paravirt(pseg, pseg + para->sh_size);
 	}
 
-	/* make jump label nops */
-	jump_label_apply_nops(me);
-
 	return 0;
 }
 
Index: linux-2.6/include/linux/jump_label.h
==================================================================---
linux-2.6.orig/include/linux/jump_label.h
+++ linux-2.6/include/linux/jump_label.h
@@ -52,7 +52,6 @@ extern int jump_label_text_reserved(void
 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);
 
 #else  /* !HAVE_JUMP_LABEL */
 
@@ -97,11 +96,6 @@ static inline bool jump_label_enabled(st
 {
 	return !!atomic_read(&key->enabled);
 }
-
-static inline int jump_label_apply_nops(struct module *mod)
-{
-	return 0;
-}
 #endif	/* HAVE_JUMP_LABEL */
 
 #endif	/* _LINUX_JUMP_LABEL_H */
Index: linux-2.6/kernel/jump_label.c
==================================================================---
linux-2.6.orig/kernel/jump_label.c
+++ linux-2.6/kernel/jump_label.c
@@ -116,9 +116,15 @@ void __weak arch_jump_label_transform_st
 	arch_jump_label_transform(entry, type);	
 }
 
+static inline enum jump_label_type jump_label_dyn_type(struct jump_label_key
*key)
+{
+	return jump_label_enabled(key) ? JUMP_LABEL_ENABLE : JUMP_LABEL_DISABLE;
+}
+
 static void __jump_label_update(struct jump_label_key *key,
 				struct jump_entry *entry,
-				struct jump_entry *stop, int enable)
+				struct jump_entry *stop, int enable,
+				void (*transform)(struct jump_entry *, enum jump_label_type))
 {
 	for (; (entry < stop) &&
 	      (entry->key == (jump_label_t)(unsigned long)key);
@@ -129,7 +135,7 @@ static void __jump_label_update(struct j
 		 * init code, see jump_label_invalidate_module_init().
 		 */
 		if (entry->code && kernel_text_address(entry->code))
-			arch_jump_label_transform(entry, enable);
+			transform(entry, enable);
 	}
 }
 
@@ -147,8 +153,7 @@ void __init jump_label_init(void)
 		struct jump_label_key *iterk;
 
 		iterk = (struct jump_label_key *)(unsigned long)iter->key;
-		arch_jump_label_transform_static(iter, jump_label_enabled(iterk) ?
-						 JUMP_LABEL_ENABLE : JUMP_LABEL_DISABLE);
+		arch_jump_label_transform_static(iter, jump_label_dyn_type(iterk));
 		if (iterk == key)
 			continue;
 
@@ -193,7 +198,7 @@ static void __jump_label_mod_update(stru
 
 		__jump_label_update(key, mod->entries,
 				    m->jump_entries + m->num_jump_entries,
-				    enable);
+				    enable, arch_jump_label_transform);
 		mod = mod->next;
 	}
 }
@@ -256,9 +261,8 @@ static int jump_label_add_module(struct
 		jlm->next = key->next;
 		key->next = jlm;
 
-		if (jump_label_enabled(key))
-			__jump_label_update(key, iter, iter_stop,
-					    JUMP_LABEL_ENABLE);
+		__jump_label_update(key, iter, iter_stop, jump_label_dyn_type(key),
+				arch_jump_label_transform_static);
 	}
 
 	return 0;
@@ -392,7 +396,7 @@ static void jump_label_update(struct jum
 #endif
 	/* if there are no users, entry can be NULL */
 	if (entry)
-		__jump_label_update(key, entry, stop, enable);
+		__jump_label_update(key, entry, stop, enable, arch_jump_label_transform);
 }
 
 #endif


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

H. Peter Anvin

2011-Oct-13 15:40 UTC

head link

[Xen-devel] Re: [PATCH RFC V4 09/10] x86/jump_label: use GENERIC_NOP5_ATOMIC instead of jmp5 +0

On 10/12/2011 05:08 PM, Jeremy Fitzhardinge wrote:> From: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
> 
> GENERIC_NOP5_ATOMIC should make a better nop.
> 
On 32 bits, yes.  On 64 bits you should use P6_NOP5_ATOMIC.

	-hpa

-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don''t speak on their behalf.


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Jason Baron

2011-Oct-13 15:55 UTC

head link

[Xen-devel] Re: [PATCH RFC V4 06/10] jump_label: add arch_jump_label_transform_static() to optimise non-live code updates

On Thu, Oct 13, 2011 at 05:29:17PM +0200, Peter Zijlstra
wrote:> On Thu, 2011-10-13 at 09:54 -0400, Jason Baron wrote:
> > 
> > > So I got myself a little confused wrt the early
jump_label_apply_nops()
> > > call and the MODULE_COMING notifiers.
> > > 
> > > It looks to me like jump_label_apply_nops() is called way early
and is
> > > in fact called before _any_ of the module code has had a chance
of
> > > running. However it simply NOPs out all jump_labels.
> > > 
> > 
> > yes.
> > 
> > > The jump_label_add_module() thing, which is ran on the
MODULE_COMING
> > > callback will then set up stuff and do the proper patch-up.
> > > 
> > 
> > yes, only for the enabled ones.
> 
> But we could make it nop out the disabled ones quite trivially.
> 
> > > Now the only bit of the module text that can be ran between those
two
> > > calls appears to be the module argument parsing stuff, but since
> > > jump_labels are non-functional it can''t rely on them, so
why do we do
> > > the early patch up again?
> > > 
> > > 
> > 
> > The ''early patch'' is for putting in the
''ideal'' no-ops into the module
> > code. These ''ideal'' no-ops are discovered at
run-time, not boot-time.
> 
> Sure, but since we can''t use static_branch() and friends from very
early
> module code (arg parsing) anyway, it doesn''t matter what NOP they
> encounter, so we might as well do something like the below, right?
> 
> > The code is optimized (hopefully) for the most common case. The
> > jump labels are by nature expected to be off, 
> 
> I actually need them to be either way.. no preference between on or off
> just a means of very _very_ infrequent runtime change in behaviour.
> 
ok, this is a new use case, all the current users are biased with gcc
out-of-lining the infrequent case.
> If we can push jump_label init to before sched_init() all I need is a
> static_branch() without the unlikely() in to avoid GCC out-of-lining the
> branch.
> 
hmmm....the current code (I believe) is biased  b/c gcc sees the
branch as always false, see: arch_static_branch() - its not b/c we have
an unlikely there. Without open coding the label, like we had before
everybody hated, I''ll have to play around and see what will create an
unbiased branch...perhaps, somebody has an idea? 
> > and by patching them early
> > like this, at least for x86, we can avoid the stop machine calls. So
its
> > the combination of most are expected to be off and no sense to call
extra
> > stop machines that lead the code to its present state.
> 
> But we could use arch_jump_label_transform_static because its before we
> actually execute any module text (sans the arg crap) which is
> stomp-machine free, removing that obstacle.
> 
> Or am I confused more?
> 
The MODULE_COMING callback happens *after* the call to flush_module_icache(mod),
so I''m not sure that is safe...

thanks,

-Jason

> ---
>  arch/mips/kernel/module.c  |    3 ---
>  arch/sparc/kernel/module.c |    3 ---
>  arch/x86/kernel/module.c   |    3 ---
>  include/linux/jump_label.h |    6 ------
>  kernel/jump_label.c        |   22 +++++++++++++---------
>  5 files changed, 13 insertions(+), 24 deletions(-)
> 
> Index: linux-2.6/arch/mips/kernel/module.c
> ==================================================================> ---
linux-2.6.orig/arch/mips/kernel/module.c
> +++ linux-2.6/arch/mips/kernel/module.c
> @@ -368,9 +368,6 @@ int module_finalize(const Elf_Ehdr *hdr,
>  	const Elf_Shdr *s;
>  	char *secstrings = (void *)hdr + sechdrs[hdr->e_shstrndx].sh_offset;
>  
> -	/* Make jump label nops. */
> -	jump_label_apply_nops(me);
> -
>  	INIT_LIST_HEAD(&me->arch.dbe_list);
>  	for (s = sechdrs; s < sechdrs + hdr->e_shnum; s++) {
>  		if (strcmp("__dbe_table", secstrings + s->sh_name) != 0)
> Index: linux-2.6/arch/sparc/kernel/module.c
> ==================================================================> ---
linux-2.6.orig/arch/sparc/kernel/module.c
> +++ linux-2.6/arch/sparc/kernel/module.c
> @@ -207,9 +207,6 @@ int module_finalize(const Elf_Ehdr *hdr,
>  		    const Elf_Shdr *sechdrs,
>  		    struct module *me)
>  {
> -	/* make jump label nops */
> -	jump_label_apply_nops(me);
> -
>  	/* Cheetah''s I-cache is fully coherent.  */
>  	if (tlb_type == spitfire) {
>  		unsigned long va;
> Index: linux-2.6/arch/x86/kernel/module.c
> ==================================================================> ---
linux-2.6.orig/arch/x86/kernel/module.c
> +++ linux-2.6/arch/x86/kernel/module.c
> @@ -194,9 +194,6 @@ int module_finalize(const Elf_Ehdr *hdr,
>  		apply_paravirt(pseg, pseg + para->sh_size);
>  	}
>  
> -	/* make jump label nops */
> -	jump_label_apply_nops(me);
> -
>  	return 0;
>  }
>  
> Index: linux-2.6/include/linux/jump_label.h
> ==================================================================> ---
linux-2.6.orig/include/linux/jump_label.h
> +++ linux-2.6/include/linux/jump_label.h
> @@ -52,7 +52,6 @@ extern int jump_label_text_reserved(void
>  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);
>  
>  #else  /* !HAVE_JUMP_LABEL */
>  
> @@ -97,11 +96,6 @@ static inline bool jump_label_enabled(st
>  {
>  	return !!atomic_read(&key->enabled);
>  }
> -
> -static inline int jump_label_apply_nops(struct module *mod)
> -{
> -	return 0;
> -}
>  #endif	/* HAVE_JUMP_LABEL */
>  
>  #endif	/* _LINUX_JUMP_LABEL_H */
> Index: linux-2.6/kernel/jump_label.c
> ==================================================================> ---
linux-2.6.orig/kernel/jump_label.c
> +++ linux-2.6/kernel/jump_label.c
> @@ -116,9 +116,15 @@ void __weak arch_jump_label_transform_st
>  	arch_jump_label_transform(entry, type);	
>  }
>  
> +static inline enum jump_label_type jump_label_dyn_type(struct
jump_label_key *key)
> +{
> +	return jump_label_enabled(key) ? JUMP_LABEL_ENABLE : JUMP_LABEL_DISABLE;
> +}
> +
>  static void __jump_label_update(struct jump_label_key *key,
>  				struct jump_entry *entry,
> -				struct jump_entry *stop, int enable)
> +				struct jump_entry *stop, int enable,
> +				void (*transform)(struct jump_entry *, enum jump_label_type))
>  {
>  	for (; (entry < stop) &&
>  	      (entry->key == (jump_label_t)(unsigned long)key);
> @@ -129,7 +135,7 @@ static void __jump_label_update(struct j
>  		 * init code, see jump_label_invalidate_module_init().
>  		 */
>  		if (entry->code && kernel_text_address(entry->code))
> -			arch_jump_label_transform(entry, enable);
> +			transform(entry, enable);
>  	}
>  }
>  
> @@ -147,8 +153,7 @@ void __init jump_label_init(void)
>  		struct jump_label_key *iterk;
>  
>  		iterk = (struct jump_label_key *)(unsigned long)iter->key;
> -		arch_jump_label_transform_static(iter, jump_label_enabled(iterk) ?
> -						 JUMP_LABEL_ENABLE : JUMP_LABEL_DISABLE);
> +		arch_jump_label_transform_static(iter, jump_label_dyn_type(iterk));
>  		if (iterk == key)
>  			continue;
>  
> @@ -193,7 +198,7 @@ static void __jump_label_mod_update(stru
>  
>  		__jump_label_update(key, mod->entries,
>  				    m->jump_entries + m->num_jump_entries,
> -				    enable);
> +				    enable, arch_jump_label_transform);
>  		mod = mod->next;
>  	}
>  }
> @@ -256,9 +261,8 @@ static int jump_label_add_module(struct
>  		jlm->next = key->next;
>  		key->next = jlm;
>  
> -		if (jump_label_enabled(key))
> -			__jump_label_update(key, iter, iter_stop,
> -					    JUMP_LABEL_ENABLE);
> +		__jump_label_update(key, iter, iter_stop, jump_label_dyn_type(key),
> +				arch_jump_label_transform_static);
>  	}
>  
>  	return 0;
> @@ -392,7 +396,7 @@ static void jump_label_update(struct jum
>  #endif
>  	/* if there are no users, entry can be NULL */
>  	if (entry)
> -		__jump_label_update(key, entry, stop, enable);
> +		__jump_label_update(key, entry, stop, enable,
arch_jump_label_transform);
>  }
>  
>  #endif
> 
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Peter Zijlstra

2011-Oct-13 16:32 UTC

head link

[Xen-devel] Re: [PATCH RFC V4 06/10] jump_label: add arch_jump_label_transform_static() to optimise non-live code updates

On Thu, 2011-10-13 at 11:55 -0400, Jason Baron wrote:> > I actually need them to be either way.. no preference between on or
off
> > just a means of very _very_ infrequent runtime change in behaviour.
> > 
> 
> ok, this is a new use case, all the current users are biased with gcc
> out-of-lining the infrequent case.
Right, 
> > If we can push jump_label init to before sched_init() all I need is a
> > static_branch() without the unlikely() in to avoid GCC out-of-lining
the
> > branch.
> > 
> 
> hmmm....the current code (I believe) is biased  b/c gcc sees the
> branch as always false, see: arch_static_branch() - its not b/c we have
> an unlikely there. Without open coding the label, like we had before
> everybody hated, I''ll have to play around and see what will create
an
> unbiased branch...perhaps, somebody has an idea? 
Fix gcc and stick an unlikely in static_branch() ? :-)
> > > and by patching them early
> > > like this, at least for x86, we can avoid the stop machine calls.
So its
> > > the combination of most are expected to be off and no sense to
call extra
> > > stop machines that lead the code to its present state.
> > 
> > But we could use arch_jump_label_transform_static because its before
we
> > actually execute any module text (sans the arg crap) which is
> > stomp-machine free, removing that obstacle.
> > 
> > Or am I confused more?
> > 
> 
> The MODULE_COMING callback happens *after* the call to
flush_module_icache(mod),
> so I''m not sure that is safe... 
We can issue another one of those?

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Jeremy Fitzhardinge

2011-Oct-13 16:50 UTC

head link

[Xen-devel] Re: [PATCH RFC V4 09/10] x86/jump_label: use GENERIC_NOP5_ATOMIC instead of jmp5 +0

On 10/13/2011 08:40 AM, H. Peter Anvin wrote:> On 10/12/2011 05:08 PM, Jeremy Fitzhardinge wrote:
>> From: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
>>
>> GENERIC_NOP5_ATOMIC should make a better nop.
>>
> On 32 bits, yes.  On 64 bits you should use P6_NOP5_ATOMIC.
So the cleanest way of fixing that is to make the GENERIC_NOP*  be
defined to P6_NOP* on 64-bit then?  Assuming I''m correct in guessing
that the intent of GENERIC_NOP* is "a basically good-enough NOP that
will work on any x86".

    J

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Jeremy Fitzhardinge

2011-Oct-13 16:56 UTC

head link

[Xen-devel] Re: [PATCH RFC V4 10/10] jump-label: initialize jump-label subsystem much earlier

On 10/13/2011 03:43 AM, Peter Zijlstra wrote:> On Wed, 2011-10-12 at 17:08 -0700, Jeremy Fitzhardinge wrote:
>> From: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
>>
>> Initialize jump_labels much earlier, we can use them.
> We can use them, where? how? what?, that sentence just begs for more.
Er, yeah, looks like I committed it without actually finishing the sentence.

    J
>> diff --git a/init/main.c b/init/main.c
>> index 2a9b88a..f4094ed 100644
>> --- a/init/main.c
>> +++ b/init/main.c
>> @@ -637,6 +637,7 @@ asmlinkage void __init start_kernel(void)
>>  	acpi_early_init(); /* before LAPIC and SMP init */
>>  	sfi_init_late();
>>  
>> +	jump_label_init();
>>  	ftrace_init();
>>  
>>  	/* Do the rest non-__init''ed, we''re now alive */
> Can we push them much earlier still? If possible I''d like them to
be
> before sched_init() so that I might use them there, if not possible, at
> the very least before enabling interrupts would be nice.
>
>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Jeremy Fitzhardinge

2011-Oct-13 16:57 UTC

head link

[Xen-devel] Re: [PATCH RFC V4 09/10] x86/jump_label: use GENERIC_NOP5_ATOMIC instead of jmp5 +0

On 10/13/2011 08:40 AM, H. Peter Anvin wrote:> On 10/12/2011 05:08 PM, Jeremy Fitzhardinge wrote:
>> From: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
>>
>> GENERIC_NOP5_ATOMIC should make a better nop.
>>
> On 32 bits, yes.  On 64 bits you should use P6_NOP5_ATOMIC.
Something like this?

From: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
Date: Thu, 13 Oct 2011 09:55:25 -0700
Subject: [PATCH] x86/nop: we can use P6 as generic NOP on 64-bit

Signed-off-by: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>

diff --git a/arch/x86/include/asm/nops.h b/arch/x86/include/asm/nops.h
index 405b403..f9fead9 100644
--- a/arch/x86/include/asm/nops.h
+++ b/arch/x86/include/asm/nops.h
@@ -19,15 +19,15 @@
    6: leal 0x00000000(%esi),%esi
    7: leal 0x00000000(,%esi,1),%esi
 */
-#define GENERIC_NOP1 0x90
-#define GENERIC_NOP2 0x89,0xf6
-#define GENERIC_NOP3 0x8d,0x76,0x00
-#define GENERIC_NOP4 0x8d,0x74,0x26,0x00
-#define GENERIC_NOP5 GENERIC_NOP1,GENERIC_NOP4
-#define GENERIC_NOP6 0x8d,0xb6,0x00,0x00,0x00,0x00
-#define GENERIC_NOP7 0x8d,0xb4,0x26,0x00,0x00,0x00,0x00
-#define GENERIC_NOP8 GENERIC_NOP1,GENERIC_NOP7
-#define GENERIC_NOP5_ATOMIC NOP_DS_PREFIX,GENERIC_NOP4
+#define GAS_NOP1 0x90
+#define GAS_NOP2 0x89,0xf6
+#define GAS_NOP3 0x8d,0x76,0x00
+#define GAS_NOP4 0x8d,0x74,0x26,0x00
+#define GAS_NOP5 GENERIC_NOP1,GENERIC_NOP4
+#define GAS_NOP6 0x8d,0xb6,0x00,0x00,0x00,0x00
+#define GAS_NOP7 0x8d,0xb4,0x26,0x00,0x00,0x00,0x00
+#define GAS_NOP8 GENERIC_NOP1,GENERIC_NOP7
+#define GAS_NOP5_ATOMIC NOP_DS_PREFIX,GENERIC_NOP4
 
 /* Opteron 64bit nops
    1: nop
@@ -87,6 +87,28 @@
 #define P6_NOP8	0x0f,0x1f,0x84,0x00,0,0,0,0
 #define P6_NOP5_ATOMIC P6_NOP5
 
+#ifdef CONFIG_X86_32
+#define GENERIC_NOP1		GAS_NOP1
+#define GENERIC_NOP2		GAS_NOP2
+#define GENERIC_NOP3		GAS_NOP3
+#define GENERIC_NOP4		GAS_NOP4
+#define GENERIC_NOP5		GAS_NOP5
+#define GENERIC_NOP6		GAS_NOP6
+#define GENERIC_NOP7		GAS_NOP7
+#define GENERIC_NOP8		GAS_NOP8
+#define GENERIC_NOP5_ATOMIC	GAS_NOP5_ATOMIC
+#else
+#define GENERIC_NOP1		P6_NOP1
+#define GENERIC_NOP2		P6_NOP2
+#define GENERIC_NOP3		P6_NOP3
+#define GENERIC_NOP4		P6_NOP4
+#define GENERIC_NOP5		P6_NOP5
+#define GENERIC_NOP6		P6_NOP6
+#define GENERIC_NOP7		P6_NOP7
+#define GENERIC_NOP8		P6_NOP8
+#define GENERIC_NOP5_ATOMIC	P6_NOP5_ATOMIC
+#endif
+
 #define _ASM_MK_NOP(x) ".byte " __stringify(x) "\n"
 
 #if defined(CONFIG_MK7)



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Steven Rostedt

2011-Oct-13 18:37 UTC

head link

[Xen-devel] Re: [PATCH RFC V4 09/10] x86/jump_label: use GENERIC_NOP5_ATOMIC instead of jmp5 +0

On Thu, 2011-10-13 at 09:57 -0700, Jeremy Fitzhardinge
wrote:> On 10/13/2011 08:40 AM, H. Peter Anvin wrote:
> > On 10/12/2011 05:08 PM, Jeremy Fitzhardinge wrote:
> >> From: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
> >>
> >> GENERIC_NOP5_ATOMIC should make a better nop.
> >>
> > On 32 bits, yes.  On 64 bits you should use P6_NOP5_ATOMIC.
> 
> Something like this?
> 
No, we only care about 5byte nops here. Look at ideal_nop in
alterative.c

-- Steve



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Jeremy Fitzhardinge

2011-Oct-14 21:51 UTC

head link

[Xen-devel] Re: [PATCH RFC V4 10/10] jump-label: initialize jump-label subsystem much earlier

On 10/13/2011 03:43 AM, Peter Zijlstra wrote:> On Wed, 2011-10-12 at 17:08 -0700, Jeremy Fitzhardinge wrote:
>> From: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
>>
>> Initialize jump_labels much earlier, we can use them.
> We can use them, where? how? what?, that sentence just begs for more.
How about this?  Proper comment and much earlier init.

    J
>From 62720522d512ffce8f4be9140f73fefbdfd2872e Mon Sep 17 00:00:00 2001From: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
Date: Wed, 12 Oct 2011 16:17:54 -0700
Subject: [PATCH] jump-label: initialize jump-label subsystem much earlier

Initialize jump_labels much, much earlier, so they''re available for use
during system setup.

Signed-off-by: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>

diff --git a/include/linux/jump_label.h b/include/linux/jump_label.h
index 56594e4..388b0d4 100644
--- a/include/linux/jump_label.h
+++ b/include/linux/jump_label.h
@@ -16,7 +16,7 @@ struct jump_label_key {
 
 # include <asm/jump_label.h>
 # define HAVE_JUMP_LABEL
-#endif
+#endif	/* CC_HAVE_ASM_GOTO && CONFIG_JUMP_LABEL */
 
 enum jump_label_type {
 	JUMP_LABEL_DISABLE = 0,
@@ -41,6 +41,7 @@ static __always_inline bool static_branch(struct
jump_label_key *key)
 extern struct jump_entry __start___jump_table[];
 extern struct jump_entry __stop___jump_table[];
 
+extern void jump_label_init(void);
 extern void jump_label_lock(void);
 extern void jump_label_unlock(void);
 extern void arch_jump_label_transform(struct jump_entry *entry,
@@ -53,7 +54,7 @@ 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);
 
-#else
+#else  /* !HAVE_JUMP_LABEL */
 
 #include <linux/atomic.h>
 
@@ -63,6 +64,10 @@ struct jump_label_key {
 	atomic_t enabled;
 };
 
+static __always_inline void jump_label_init(void)
+{
+}
+
 static __always_inline bool static_branch(struct jump_label_key *key)
 {
 	if (unlikely(atomic_read(&key->enabled)))
@@ -97,7 +102,6 @@ static inline int jump_label_apply_nops(struct module *mod)
 {
 	return 0;
 }
+#endif	/* HAVE_JUMP_LABEL */
 
-#endif
-
-#endif
+#endif	/* _LINUX_JUMP_LABEL_H */
diff --git a/init/main.c b/init/main.c
index 2a9b88a..29d8d84 100644
--- a/init/main.c
+++ b/init/main.c
@@ -515,6 +515,9 @@ asmlinkage void __init start_kernel(void)
 	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()
diff --git a/kernel/jump_label.c b/kernel/jump_label.c
index ff2028f..bbdfe2a 100644
--- a/kernel/jump_label.c
+++ b/kernel/jump_label.c
@@ -133,7 +133,7 @@ static void __jump_label_update(struct jump_label_key *key,
 	}
 }
 
-static __init int jump_label_init(void)
+void __init jump_label_init(void)
 {
 	struct jump_entry *iter_start = __start___jump_table;
 	struct jump_entry *iter_stop = __stop___jump_table;
@@ -159,10 +159,7 @@ static __init int jump_label_init(void)
 #endif
 	}
 	jump_label_unlock();
-
-	return 0;
 }
-early_initcall(jump_label_init);
 
 #ifdef CONFIG_MODULES
 



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

H. Peter Anvin

2011-Oct-14 21:52 UTC

head link

[Xen-devel] Re: [PATCH RFC V4 09/10] x86/jump_label: use GENERIC_NOP5_ATOMIC instead of jmp5 +0

On 10/13/2011 09:50 AM, Jeremy Fitzhardinge wrote:> On 10/13/2011 08:40 AM, H. Peter Anvin wrote:
>> On 10/12/2011 05:08 PM, Jeremy Fitzhardinge wrote:
>>> From: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
>>>
>>> GENERIC_NOP5_ATOMIC should make a better nop.
>>>
>> On 32 bits, yes.  On 64 bits you should use P6_NOP5_ATOMIC.
> 
> So the cleanest way of fixing that is to make the GENERIC_NOP*  be
> defined to P6_NOP* on 64-bit then?  Assuming I''m correct in
guessing
> that the intent of GENERIC_NOP* is "a basically good-enough NOP that
> will work on any x86".
> 
No, GENERIC_NOPs are not valid on 64 bits at all.

	-hpa


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

H. Peter Anvin

2011-Oct-14 21:53 UTC

head link

[Xen-devel] Re: [PATCH RFC V4 09/10] x86/jump_label: use GENERIC_NOP5_ATOMIC instead of jmp5 +0

On 10/13/2011 09:57 AM, Jeremy Fitzhardinge wrote:> 
> Something like this?
> 
> From: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
> Date: Thu, 13 Oct 2011 09:55:25 -0700
> Subject: [PATCH] x86/nop: we can use P6 as generic NOP on 64-bit
> 
> Signed-off-by: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
> 
> diff --git a/arch/x86/include/asm/nops.h b/arch/x86/include/asm/nops.h
> index 405b403..f9fead9 100644
> --- a/arch/x86/include/asm/nops.h
> +++ b/arch/x86/include/asm/nops.h
> @@ -19,15 +19,15 @@
>     6: leal 0x00000000(%esi),%esi
>     7: leal 0x00000000(,%esi,1),%esi
>  */
> -#define GENERIC_NOP1 0x90
> -#define GENERIC_NOP2 0x89,0xf6
> -#define GENERIC_NOP3 0x8d,0x76,0x00
> -#define GENERIC_NOP4 0x8d,0x74,0x26,0x00
> -#define GENERIC_NOP5 GENERIC_NOP1,GENERIC_NOP4
> -#define GENERIC_NOP6 0x8d,0xb6,0x00,0x00,0x00,0x00
> -#define GENERIC_NOP7 0x8d,0xb4,0x26,0x00,0x00,0x00,0x00
> -#define GENERIC_NOP8 GENERIC_NOP1,GENERIC_NOP7
> -#define GENERIC_NOP5_ATOMIC NOP_DS_PREFIX,GENERIC_NOP4
> +#define GAS_NOP1 0x90
> +#define GAS_NOP2 0x89,0xf6
> +#define GAS_NOP3 0x8d,0x76,0x00
> +#define GAS_NOP4 0x8d,0x74,0x26,0x00
> +#define GAS_NOP5 GENERIC_NOP1,GENERIC_NOP4
> +#define GAS_NOP6 0x8d,0xb6,0x00,0x00,0x00,0x00
> +#define GAS_NOP7 0x8d,0xb4,0x26,0x00,0x00,0x00,0x00
> +#define GAS_NOP8 GENERIC_NOP1,GENERIC_NOP7
> +#define GAS_NOP5_ATOMIC NOP_DS_PREFIX,GENERIC_NOP4
>  
They''re not really the gas nops, either.  If you want to rename them I
would suggest GENERIC32_*
>  /* Opteron 64bit nops
>     1: nop
> @@ -87,6 +87,28 @@
>  #define P6_NOP8	0x0f,0x1f,0x84,0x00,0,0,0,0
>  #define P6_NOP5_ATOMIC P6_NOP5
>  
> +#ifdef CONFIG_X86_32
> +#define GENERIC_NOP1		GAS_NOP1
> +#define GENERIC_NOP2		GAS_NOP2
> +#define GENERIC_NOP3		GAS_NOP3
> +#define GENERIC_NOP4		GAS_NOP4
> +#define GENERIC_NOP5		GAS_NOP5
> +#define GENERIC_NOP6		GAS_NOP6
> +#define GENERIC_NOP7		GAS_NOP7
> +#define GENERIC_NOP8		GAS_NOP8
> +#define GENERIC_NOP5_ATOMIC	GAS_NOP5_ATOMIC
> +#else
> +#define GENERIC_NOP1		P6_NOP1
> +#define GENERIC_NOP2		P6_NOP2
> +#define GENERIC_NOP3		P6_NOP3
> +#define GENERIC_NOP4		P6_NOP4
> +#define GENERIC_NOP5		P6_NOP5
> +#define GENERIC_NOP6		P6_NOP6
> +#define GENERIC_NOP7		P6_NOP7
> +#define GENERIC_NOP8		P6_NOP8
> +#define GENERIC_NOP5_ATOMIC	P6_NOP5_ATOMIC
> +#endif
> +
Make sure you change all the downstream users too.

	-hpa


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

H. Peter Anvin

2011-Oct-14 21:53 UTC

head link

[Xen-devel] Re: [PATCH RFC V4 09/10] x86/jump_label: use GENERIC_NOP5_ATOMIC instead of jmp5 +0

On 10/13/2011 11:37 AM, Steven Rostedt wrote:> On Thu, 2011-10-13 at 09:57 -0700, Jeremy Fitzhardinge wrote:
>> On 10/13/2011 08:40 AM, H. Peter Anvin wrote:
>>> On 10/12/2011 05:08 PM, Jeremy Fitzhardinge wrote:
>>>> From: Jeremy Fitzhardinge
<jeremy.fitzhardinge@citrix.com>
>>>>
>>>> GENERIC_NOP5_ATOMIC should make a better nop.
>>>>
>>> On 32 bits, yes.  On 64 bits you should use P6_NOP5_ATOMIC.
>>
>> Something like this?
>>
> 
> No, we only care about 5byte nops here. Look at ideal_nop in
> alterative.c
> 
He needs a compile-time alternative.

	-hpa


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Steven Rostedt

2011-Oct-15 00:22 UTC

head link

[Xen-devel] Re: [PATCH RFC V4 09/10] x86/jump_label: use GENERIC_NOP5_ATOMIC instead of jmp5 +0

On Fri, 2011-10-14 at 14:53 -0700, H. Peter Anvin wrote:> On 10/13/2011 11:37 AM, Steven Rostedt wrote:
> > On Thu, 2011-10-13 at 09:57 -0700, Jeremy Fitzhardinge wrote:
> >> On 10/13/2011 08:40 AM, H. Peter Anvin wrote:
> >>> On 10/12/2011 05:08 PM, Jeremy Fitzhardinge wrote:
> >>>> From: Jeremy Fitzhardinge
<jeremy.fitzhardinge@citrix.com>
> >>>>
> >>>> GENERIC_NOP5_ATOMIC should make a better nop.
> >>>>
> >>> On 32 bits, yes.  On 64 bits you should use P6_NOP5_ATOMIC.
> >>
> >> Something like this?
> >>
> > 
> > No, we only care about 5byte nops here. Look at ideal_nop in
> > alterative.c
> > 
> 
> He needs a compile-time alternative.
I didn''t explain this well. I actually just meant something like:

#ifdef CONFIG_X86_64
# define IDEAL_NOP5	P6_NOP5_ATOMIC
#else
# define IDEAL_NOP5	GENERIC_NOP5_ATOMIC
#endif

That is, we know we are using this for a 5byte jump. Just keep it simple
for that. Do you envision needing other nops too? Well, we could do 2
byte nops, but I think we could use the same for both. Can''t we?

-- Steve



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Peter Zijlstra

2011-Oct-15 08:42 UTC

head link

[Xen-devel] Re: [PATCH RFC V4 10/10] jump-label: initialize jump-label subsystem much earlier

On Fri, 2011-10-14 at 14:51 -0700, Jeremy Fitzhardinge
wrote:> How about this?  Proper comment and much earlier init.
Looks good to me, thanks!

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Jeremy Fitzhardinge

2011-Oct-16 01:52 UTC

head link

[Xen-devel] Re: [PATCH RFC V4 10/10] jump-label: initialize jump-label subsystem much earlier

On 10/15/2011 01:42 AM, Peter Zijlstra wrote:> On Fri, 2011-10-14 at 14:51 -0700, Jeremy Fitzhardinge wrote:
>> How about this?  Proper comment and much earlier init.
> Looks good to me, thanks!
Should I take that as an ACK on the series?

    J

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Peter Zijlstra

2011-Oct-18 11:02 UTC

head link

[Xen-devel] Re: [PATCH RFC V4 10/10] jump-label: initialize jump-label subsystem much earlier

On Sat, 2011-10-15 at 18:52 -0700, Jeremy Fitzhardinge
wrote:> On 10/15/2011 01:42 AM, Peter Zijlstra wrote:
> > On Fri, 2011-10-14 at 14:51 -0700, Jeremy Fitzhardinge wrote:
> >> How about this?  Proper comment and much earlier init.
> > Looks good to me, thanks!
> 
> Should I take that as an ACK on the series?
Yes, I don''t see any problems with it,

Acked-by: Peter Zijlstra <a.p.zijlstra@chello.nl>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Jeremy Fitzhardinge

2011-Oct-25 17:56 UTC

head link

[Xen-devel] Re: [PATCH RFC V4 10/10] jump-label: initialize jump-label subsystem much earlier

On 10/18/2011 04:02 AM, Peter Zijlstra wrote:> On Sat, 2011-10-15 at 18:52 -0700, Jeremy Fitzhardinge wrote:
>> On 10/15/2011 01:42 AM, Peter Zijlstra wrote:
>>> On Fri, 2011-10-14 at 14:51 -0700, Jeremy Fitzhardinge wrote:
>>>> How about this?  Proper comment and much earlier init.
>>> Looks good to me, thanks!
>> Should I take that as an ACK on the series?
> Yes, I don''t see any problems with it,
>
> Acked-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
How should this go upstream?   Should I submit it, or does someone else
want to do it?

Thanks,
    J


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Xen devel - Oct 2011 - [PATCH RFC V4 00/10] jump-label: allow early jump_label_enable()

[Xen-devel] [PATCH RFC V4 00/10] jump-label: allow early jump_label_enable()

[Xen-devel] [PATCH RFC V4 01/10] jump_label: use proper atomic_t initializer

[Xen-devel] [PATCH RFC V4 02/10] stop_machine: make stop_machine safe and efficient to call early

[Xen-devel] [PATCH RFC V4 03/10] jump_label: if a key has already been initialized, don''t nop it out

[Xen-devel] [PATCH RFC V4 04/10] x86/jump_label: drop arch_jump_label_text_poke_early()

[Xen-devel] [PATCH RFC V4 05/10] sparc/jump_label: drop arch_jump_label_text_poke_early()

[Xen-devel] [PATCH RFC V4 06/10] jump_label: add arch_jump_label_transform_static() to optimise non-live code updates

[Xen-devel] [PATCH RFC V4 07/10] s390/jump-label: add arch_jump_label_transform_static()

[Xen-devel] [PATCH RFC V4 08/10] x86/jump_label: add arch_jump_label_transform_static()

[Xen-devel] [PATCH RFC V4 09/10] x86/jump_label: use GENERIC_NOP5_ATOMIC instead of jmp5 +0

[Xen-devel] [PATCH RFC V4 10/10] jump-label: initialize jump-label subsystem much earlier

[Xen-devel] Re: [PATCH RFC V4 04/10] x86/jump_label: drop arch_jump_label_text_poke_early()

[Xen-devel] Re: [PATCH RFC V4 06/10] jump_label: add arch_jump_label_transform_static() to optimise non-live code updates

[Xen-devel] Re: [PATCH RFC V4 08/10] x86/jump_label: add arch_jump_label_transform_static()

[Xen-devel] Re: [PATCH RFC V4 10/10] jump-label: initialize jump-label subsystem much earlier

[Xen-devel] Re: [PATCH RFC V4 06/10] jump_label: add arch_jump_label_transform_static() to optimise non-live code updates

[Xen-devel] Re: [PATCH RFC V4 10/10] jump-label: initialize jump-label subsystem much earlier

[Xen-devel] Re: [PATCH RFC V4 06/10] jump_label: add arch_jump_label_transform_static() to optimise non-live code updates

[Xen-devel] Re: [PATCH RFC V4 09/10] x86/jump_label: use GENERIC_NOP5_ATOMIC instead of jmp5 +0

[Xen-devel] Re: [PATCH RFC V4 06/10] jump_label: add arch_jump_label_transform_static() to optimise non-live code updates

[Xen-devel] Re: [PATCH RFC V4 06/10] jump_label: add arch_jump_label_transform_static() to optimise non-live code updates

[Xen-devel] Re: [PATCH RFC V4 09/10] x86/jump_label: use GENERIC_NOP5_ATOMIC instead of jmp5 +0

[Xen-devel] Re: [PATCH RFC V4 10/10] jump-label: initialize jump-label subsystem much earlier

[Xen-devel] Re: [PATCH RFC V4 09/10] x86/jump_label: use GENERIC_NOP5_ATOMIC instead of jmp5 +0

[Xen-devel] Re: [PATCH RFC V4 09/10] x86/jump_label: use GENERIC_NOP5_ATOMIC instead of jmp5 +0

[Xen-devel] Re: [PATCH RFC V4 10/10] jump-label: initialize jump-label subsystem much earlier

[Xen-devel] Re: [PATCH RFC V4 09/10] x86/jump_label: use GENERIC_NOP5_ATOMIC instead of jmp5 +0

[Xen-devel] Re: [PATCH RFC V4 09/10] x86/jump_label: use GENERIC_NOP5_ATOMIC instead of jmp5 +0

[Xen-devel] Re: [PATCH RFC V4 09/10] x86/jump_label: use GENERIC_NOP5_ATOMIC instead of jmp5 +0

[Xen-devel] Re: [PATCH RFC V4 09/10] x86/jump_label: use GENERIC_NOP5_ATOMIC instead of jmp5 +0

[Xen-devel] Re: [PATCH RFC V4 10/10] jump-label: initialize jump-label subsystem much earlier

[Xen-devel] Re: [PATCH RFC V4 10/10] jump-label: initialize jump-label subsystem much earlier

[Xen-devel] Re: [PATCH RFC V4 10/10] jump-label: initialize jump-label subsystem much earlier

[Xen-devel] Re: [PATCH RFC V4 10/10] jump-label: initialize jump-label subsystem much earlier