Markers use immediate values, for 2.6.24-rc5-mm1
Subsystems (10)
cover Markers use immediate values, for 2.6.24-rc5-mm1
Hi Andrew,
Here are the patches that would be interesting to queue for 2.6.25. As you asked, the patchset applies to 2.6.24-rc5-mm1.
It includes those logical changes and applies in the following order.
Thanks,
Mathieu
#Text Edit Lock
kprobes-use-mutex-for-insn-pages.patch kprobes-dont-use-kprobes-mutex-in-arch-code.patch kprobes-declare-kprobes-mutex-static.patch declare-array.patch text-edit-lock-architecture-independent-code.patch text-edit-lock-alternative-i386-and-x86_64.patch text-edit-lock-kprobes-architecture-independent.patch text-edit-lock-kprobes-i386.patch text-edit-lock-kprobes-x86_64.patch text-edit-lock-i386-standardize-debug-rodata.patch text-edit-lock-x86_64-standardize-debug-rodata.patch
#
#Immediate Values immediate-values-architecture-independent-code.patch immediate-values-kconfig-embedded.patch immediate-values-x86-optimization.patch add-text-poke-to-powerpc.patch immediate-values-powerpc-optimization.patch immediate-values-documentation.patch
#
profiling-use-immediate-values.patch
#
#Markers use immediate values immediate-values-move-kprobes-x86-restore-interrupt-to-kdebug-h.patch add-discard-section-to-x86.patch immediate-values-x86-optimization-nmi-mce-support.patch immediate-values-powerpc-optimization-nmi-mce-support.patch immediate-values-use-arch-nmi-mce-support.patch linux-kernel-markers-immediate-values.patch
Signature · 3 lines
Mathieu Desnoyers Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
PATCH 1/24 Kprobes - use a mutex to protect the instruction pages list.
Protect the instruction pages list by a specific insn pages mutex, called in get_insn_slot() and free_insn_slot(). It makes sure that architectures that does not need to call arch_remove_kprobe() does not take an unneeded kprobes mutex.
--- kernel/kprobes.c | 27 +++++++++++++++++++++------ 1 file changed, 21 insertions(+), 6 deletions(-)
kernel/kprobes.c +21 -6
@@ -95,6 +95,10 @@ enum kprobe_slot_state { SLOT_USED = 2, }; +/* +* Protects the kprobe_insn_pages list. Can nest into kprobe_mutex. +*/ +static DEFINE_MUTEX (kprobe_insn_mutex);static struct hlist_head kprobe_insn_pages;static int kprobe_garbage_slots;static int collect_garbage_slots (void );
@@ -131,7 +135,9 @@ kprobe_opcode_t __kprobes *get_insn_slot {struct kprobe_insn_page *kip;struct hlist_node *pos; +kprobe_opcode_t *ret; +mutex_lock (&kprobe_insn_mutex); retry:hlist_for_each_entry (kip, pos, &kprobe_insn_pages, hlist) {if (kip->nused <INSNS_PER_PAGE ) {
@@ -140,7 +146,8 @@ kprobe_opcode_t __kprobes *get_insn_slotif (kip->slot_used[i] ==SLOT_CLEAN ) { kip->slot_used[i] =SLOT_USED ; kip->nused++; -return kip->insns + (i *MAX_INSN_SIZE ); + ret = kip->insns + (i *MAX_INSN_SIZE ); + goto end; } }/* Surprise! No unused slots. Fix kip->nused. */
@@ -154,8 +161,10 @@ kprobe_opcode_t __kprobes *get_insn_slot }/* All out of space. Need to allocate a new page. Use slot 0. */ kip =kmalloc (sizeof (struct kprobe_insn_page ),GFP_KERNEL ); -if (!kip) -return NULL ; +if (!kip) { + ret =NULL ; + goto end; + }/* * Use module_alloc so this page is within +/- 2GB of where the
@@ -165,7 +174,8 @@ kprobe_opcode_t __kprobes *get_insn_slot kip->insns =module_alloc (PAGE_SIZE );if (!kip->insns) {kfree (kip); -return NULL ; + ret =NULL ; + goto end; }INIT_HLIST_NODE (&kip->hlist);hlist_add_head (&kip->hlist, &kprobe_insn_pages);
@@ -173,7 +183,10 @@ kprobe_opcode_t __kprobes *get_insn_slot kip->slot_used[0 ] =SLOT_USED ; kip->nused =1 ; kip->ngarbage =0 ; -return kip->insns; + ret = kip->insns; +end: +mutex_unlock (&kprobe_insn_mutex); +return ret; }/* Return 1 if all garbages are collected, otherwise 0. */
@@ -207,7 +220,7 @@ static int __kprobes collect_garbage_slostruct kprobe_insn_page *kip;struct hlist_node *pos, *next; -/* Ensure no-one is preepmted on the garbages */ +/* Ensure no-one is preempted on the garbages */ if (check_safety () !=0 )return -EAGAIN ;
@@ -231,6 +244,7 @@ void __kprobes free_insn_slot(kprobe_opcstruct kprobe_insn_page *kip;struct hlist_node *pos; +mutex_lock (&kprobe_insn_mutex);hlist_for_each_entry (kip, pos, &kprobe_insn_pages, hlist) {if (kip->insns <= slot && slot < kip->insns + (INSNS_PER_PAGE *MAX_INSN_SIZE )) {
@@ -247,6 +261,7 @@ void __kprobes free_insn_slot(kprobe_opcif (dirty && ++kprobe_garbage_slots >INSNS_PER_PAGE )collect_garbage_slots (); + mutex_unlock(&kprobe_insn_mutex); }#endif
Signature · 3 lines
Mathieu Desnoyers Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
PATCH 2/24 Kprobes - do not use kprobes mutex in arch code
In reply to Mathieu DesnoyersRemove the kprobes mutex from kprobes.h, since it does not belong there. Also remove all use of this mutex in the architecture specific code, replacing it by a proper mutex lock/unlock in the architecture agnostic code.
--- arch/ia64/kernel/kprobes.c | 2 -- arch/powerpc/kernel/kprobes.c | 2 -- arch/s390/kernel/kprobes.c | 2 -- arch/x86/kernel/kprobes_32.c | 2 -- arch/x86/kernel/kprobes_64.c | 2 -- include/linux/kprobes.h | 2 -- kernel/kprobes.c | 2 ++ 7 files changed, 2 insertions(+), 12 deletions(-)
include/linux/kprobes.h +0 -2
@@ -35,7 +35,6 @@#include <linux/percpu.h> #include <linux/spinlock.h> #include <linux/rcupdate.h> -#include <linux/mutex.h> #ifdef CONFIG_KPROBES #include <asm/kprobes.h>
@@ -183,7 +182,6 @@ static inline void kretprobe_assert(stru }extern spinlock_t kretprobe_lock; -extern struct mutex kprobe_mutex;extern int arch_prepare_kprobe (struct kprobe *p);extern void arch_arm_kprobe (struct kprobe *p);extern void arch_disarm_kprobe (struct kprobe *p);
arch/x86/kernel/kprobes_32.c +0 -2
@@ -186,9 +186,7 @@ void __kprobes arch_disarm_kprobe(structvoid __kprobes arch_remove_kprobe (struct kprobe *p) { -mutex_lock (&kprobe_mutex);free_insn_slot (p->ainsn.insn, (p->ainsn.boostable ==1 )); -mutex_unlock (&kprobe_mutex); }static void __kprobes save_previous_kprobe (struct kprobe_ctlblk *kcb)
kernel/kprobes.c +2 -0
@@ -644,7 +644,9 @@ valid_p:list_del_rcu (&p->list);kfree (old_p); } +mutex_lock (&kprobe_mutex);arch_remove_kprobe (p); +mutex_unlock (&kprobe_mutex); }else {mutex_lock (&kprobe_mutex);if (p->break_handler)
arch/ia64/kernel/kprobes.c +0 -2
@@ -582,9 +582,7 @@ void __kprobes arch_disarm_kprobe(structvoid __kprobes arch_remove_kprobe (struct kprobe *p) { -mutex_lock (&kprobe_mutex);free_insn_slot (p->ainsn.insn,0 ); -mutex_unlock (&kprobe_mutex); }/* * We are resuming execution after a single step fault, so the pt_regs
arch/powerpc/kernel/kprobes.c +0 -2
@@ -88,9 +88,7 @@ void __kprobes arch_disarm_kprobe(structvoid __kprobes arch_remove_kprobe (struct kprobe *p) { -mutex_lock (&kprobe_mutex);free_insn_slot (p->ainsn.insn,0 ); -mutex_unlock (&kprobe_mutex); }static void __kprobes prepare_singlestep (struct kprobe *p,struct pt_regs *regs)
arch/s390/kernel/kprobes.c +0 -2
@@ -220,9 +220,7 @@ void __kprobes arch_disarm_kprobe(structvoid __kprobes arch_remove_kprobe (struct kprobe *p) { -mutex_lock (&kprobe_mutex);free_insn_slot (p->ainsn.insn,0 ); -mutex_unlock (&kprobe_mutex); }static void __kprobes prepare_singlestep (struct kprobe *p,struct pt_regs *regs)
arch/x86/kernel/kprobes_64.c +0 -2
@@ -225,9 +225,7 @@ void __kprobes arch_disarm_kprobe(structvoid __kprobes arch_remove_kprobe (struct kprobe *p) { -mutex_lock (&kprobe_mutex);free_insn_slot (p->ainsn.insn,0 ); -mutex_unlock (&kprobe_mutex); }static void __kprobes save_previous_kprobe (struct kprobe_ctlblk *kcb)
Signature · 3 lines
Mathieu Desnoyers Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
PATCH 3/24 Kprobes - declare kprobe_mutex static
In reply to Mathieu DesnoyersSince it will not be used by other kernel objects, it makes sense to declare it static.
--- kernel/kprobes.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
kernel/kprobes.c +1 -1
@@ -68,7 +68,7 @@ static struct hlist_head kretprobe_inst_/* NOTE: change this value only with kprobe_mutex held */ static bool kprobe_enabled; -DEFINE_MUTEX (kprobe_mutex);/* Protects kprobe_table */ +static DEFINE_MUTEX (kprobe_mutex);/* Protects kprobe_table */ DEFINE_SPINLOCK (kretprobe_lock);/* Protects kretprobe_inst_table */ static DEFINE_PER_CPU(struct kprobe *, kprobe_instance) = NULL;
Signature · 3 lines
Mathieu Desnoyers Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
PATCH 4/24 Add INIT_ARRAY() to kernel.h
In reply to Mathieu DesnoyersAdd initialization of an array, which needs brackets that would pollute kernel code, to kernel.h. It is used to declare arguments passed as function parameters such as:text_poke(addr, INIT_ARRAY(unsigned char, 0xf0, len), len);
--- include/linux/kernel.h | 2 ++ 1 file changed, 2 insertions(+)
include/linux/kernel.h +2 -0
@@ -421,4 +421,6 @@ struct sysinfo {#define NUMA_BUILD 0#endif +#define INIT_ARRAY (type, val, len) ((type [len]) { [0 ... (len)-1] = (val) }) +#endif
Signature · 3 lines
Mathieu Desnoyers Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
PATCH 5/24 Text Edit Lock - Architecture Independent Code
In reply to Mathieu DesnoyersThis is an architecture independant synchronization around kernel text modifications through use of a global mutex.
A mutex has been chosen so that kprobes, the main user of this, can sleep during memory allocation between the memory read of the instructions it must replace and the memory write of the breakpoint.
Other user of this interface: immediate values.
Paravirt and alternatives are always done when SMP is inactive, so there is no need to use locks.
--- include/linux/memory.h | 7 +++++++ mm/memory.c | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+)
include/linux/memory.h +7 -0
@@ -93,4 +93,11 @@ extern int memory_notify(unsigned long v#define hotplug_memory_notifier (fn, pri) do { } while (0)#endif +/* +* Take and release the kernel text modification lock, used for code patching. +* Users of this lock can sleep. +*/ +extern void kernel_text_lock (void ); +extern void kernel_text_unlock (void ); +#endif /* _LINUX_MEMORY_H_ */
mm/memory.c +34 -0
@@ -50,6 +50,8 @@#include <linux/delayacct.h> #include <linux/init.h> #include <linux/writeback.h> +#include <linux/kprobes.h> +#include <linux/mutex.h> #include <asm/pgalloc.h> #include <asm/uaccess.h>
@@ -84,6 +86,12 @@ EXPORT_SYMBOL(high_memory);int randomize_va_space__read_mostly =1 ; +/* +* mutex protecting text section modification (dynamic code patching). +* some users need to sleep (allocating memory...) while they hold this lock. +*/ +static DEFINE_MUTEX (text_mutex); +static int __init disable_randmaps (char *s) { randomize_va_space =0 ;
@@ -2748,3 +2756,29 @@ int access_process_vm(struct task_structreturn buf - old_buf; } + +/** +* kernel_text_lock - Take the kernel text modification lock +* +* Insures mutual write exclusion of kernel and modules text live text +* modification. Should be used for code patching. +* Users of this lock can sleep. +*/ +void __kprobes kernel_text_lock (void ) +{ +mutex_lock (&text_mutex); +} +EXPORT_SYMBOL_GPL (kernel_text_lock); + +/** +* kernel_text_unlock - Release the kernel text modification lock +* +* Insures mutual write exclusion of kernel and modules text live text +* modification. Should be used for code patching. +* Users of this lock can sleep. +*/ +void __kprobes kernel_text_unlock (void ) +{ +mutex_unlock (&text_mutex); +} +EXPORT_SYMBOL_GPL (kernel_text_unlock);
Signature · 3 lines
Mathieu Desnoyers Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
RE: [patch 05/24] Text Edit Lock - Architecture Independent Code
hello,
I have some questions for your patches.
Quoted 14 lines
Paravirt and alternatives are always done when SMP is inactive, so there is no
need to use locks.(quoted diff) +0
-9-#ifndef CONFIG_KPROBES -#ifdef CONFIG_HOTPLUG_CPU - /* It must still be possible to apply SMP alternatives. */ - if (num_possible_cpus() <= 1) -#endif - { - change_page_attr(virt_to_page(start), - size >> PAGE_SHIFT, PAGE_KERNEL_RX); - printk("Write protecting the kernel text:%luk\n", size >> 10);
Review
Quoted 7 lines
(quoted diff) +5
-2- } -#endif + change_page_attr(virt_to_page(start), + size >> PAGE_SHIFT, PAGE_KERNEL_RX); + printk(KERN_INFO "Write protecting the kernel text: %luk\n", + size >> 10); +
Why "mark_rodata_ro" doesn't consider smp instance? Maybe it will be appied in future.
Quoted 6 lines
===================================================================
---linux-2.6-lttng.orig/kernel/kprobes.c2007-12-12
18:10:32.000000000 -0500
+++linux-2.6-lttng/kernel/kprobes.c2007-12-12
18:10:34.000000000 -0500
Review valid_p: · line 652
Quoted 9 lines
(quoted diff) +2
-0@@ -644,7 +644,9 @@ valid_p: list_del_rcu(&p->list); kfree(old_p); } + mutex_lock(&kprobe_mutex); arch_remove_kprobe(p); + mutex_unlock(&kprobe_mutex); } else { mutex_lock(&kprobe_mutex); if (p->break_handler)
I think "mutex_lock" and "mutex_unlock" shoud be in architecture code. In "__register_kprobe" funtion, its implement "arch_prepare_kprobe" and "arch_arm_kprobe" is also depended on arch. So the remove implement is not the same on the different architecture code.
Maybe it doesn't need the mutex_lock in "arch_remove_kprobe" on some embeded system chips if linux can support the other embeded system chips in future.
Quoted 127 lines
-----Original Message-----
From: [email protected] [mailto:[email protected]] On Behalf Of Mathieu Desnoyers
Sent: Friday, December 21, 2007 9:55 AM
To: [email protected]; Ingo Molnar; [email protected]
Cc: Mathieu Desnoyers; Andi Kleen
Subject: [patch 05/24] Text Edit Lock - Architecture Independent CodeThis is an architecture independant synchronization around kernel text modifications through use of a global mutex.
A mutex has been chosen so that kprobes, the main user of this, can sleep during
memory allocation between the memory read of the instructions it must replace
and the memory write of the breakpoint.Other user of this interface: immediate values.
Paravirt and alternatives are always done when SMP is inactive, so there is no
need to use locks.Signed-off-by Mathieu Desnoyers <[email protected]>CC Andi Kleen <[email protected]>--- include/linux/memory.h | 7 +++++++ mm/memory.c | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+)Index:
linux-2.6-lttng/include/linux/memory.h
===================================================================
---linux-2.6-lttng.orig/include/linux/memory.h>2007-11-07 11:11:26.000000000 -0500+++
linux-2.6-lttng/include/linux/memory.h2007-11-07
11:13:48.000000000 -0500include/linux/memory.h +2
-0@@ -93,4 +93,11 @@ extern int memory_notify(unsigned long v#define hotplug_memory_notifier (fn, pri) do { } while (0)#endif +/* +* Take and release the kernel text modification lock, used for code patching.
+ * Users of this lock can sleep.
+ */include/linux/memory.h +3
-0+extern void kernel_text_lock (void ); +extern void kernel_text_unlock (void ); +#endif /* _LINUX_MEMORY_H_ */ Index:
linux-2.6-lttng/mm/memory.c
===================================================================
---linux-2.6-lttng.orig/mm/memory.c2007-11-07
11:12:33.000000000 -0500
+++linux-2.6-lttng/mm/memory.c2007-11-07
11:14:25.000000000 -0500mm/memory.c +4
-0@@ -50,6 +50,8 @@#include <linux/delayacct.h> #include <linux/init.h> #include <linux/writeback.h> +#include <linux/kprobes.h> +#include <linux/mutex.h> #include <asm/pgalloc.h> #include <asm/uaccess.h> @@ -84,6 +86,12 @@ EXPORT_SYMBOL(high_memory);int randomize_va_space__read_mostly =1 ; +/* +* mutex protecting text section modification (dynamic code patching).
+ * some users need to sleep (allocating memory...) while they hold this lock.
+ */mm/memory.c +7
-0+static DEFINE_MUTEX (text_mutex); +static int __init disable_randmaps (char *s) { randomize_va_space =0 ;@@ -2748,3 +2756,29 @@ int access_process_vm(struct task_structreturn buf - old_buf; } + +/** +* kernel_text_lock - Take the kernel text modification lock +* +* Insures mutual write exclusion of kernel and modules text live text
+ * modification. Should be used for code patching.
+ * Users of this lock can sleep.
+ */mm/memory.c +10
-0+void __kprobes kernel_text_lock (void ) +{ +mutex_lock (&text_mutex); +} +EXPORT_SYMBOL_GPL (kernel_text_lock); + +/** +* kernel_text_unlock - Release the kernel text modification lock +* +* Insures mutual write exclusion of kernel and modules text live text
+ * modification. Should be used for code patching.
+ * Users of this lock can sleep.
+ */mm/memory.c +5
-0+void __kprobes kernel_text_unlock (void ) +{ +mutex_unlock (&text_mutex); +} +EXPORT_SYMBOL_GPL (kernel_text_unlock);Signature · 11 lines
-- Mathieu Desnoyers Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [patch 05/24] Text Edit Lock - Architecture Independent Code
Quoted 24 lines
Quoted 7 lines
===================================================================
---linux-2.6-lttng.orig/kernel/kprobes.c2007-12-12
18:10:32.000000000 -0500
+++linux-2.6-lttng/kernel/kprobes.c2007-12-12
18:10:34.000000000 -0500Review valid_p: · line 652
Quoted 9 lines
(quoted diff) +2
-0@@ -644,7 +644,9 @@ valid_p: list_del_rcu(&p->list); kfree(old_p); } + mutex_lock(&kprobe_mutex); arch_remove_kprobe(p); + mutex_unlock(&kprobe_mutex); } else { mutex_lock(&kprobe_mutex); if (p->break_handler)I think "
mutex_lock" and "mutex_unlock" shoud be in architecture code. In "__register_kprobe" funtion, its implement "arch_prepare_kprobe" and
"arch_arm_kprobe" is also depended on arch. So the remove implement is not
the same on the different architecture code.
Quoted 4 lines
Maybe it doesn't need the
mutex_lockin "arch_remove_kprobe" on some embeded
system chips if linux can support the other embeded system chips in future.
Could we insert the "mutex_lock" and "mutex_unlock" into "free_insn_slot" instead of architecture code?
modify as follows:
void __kprobes free_insn_slot(kprobe_opcode_t * slot, int dirty)
{
struct kprobe_insn_page *kip;
struct hlist_node *pos;+ mutex_lock(&kprobe_mutex);
hlist_for_each_entry(kip, pos, &kprobe_insn_pages, hlist) {
if (kip->insns <= slot &&
slot < kip->insns + (INSNS_PER_PAGE * MAX_INSN_SIZE)) {
int i = (slot - kip->insns) / MAX_INSN_SIZE;
if (dirty) {
kip->slot_used[i] = SLOT_DIRTY;
kip->ngarbage++;
} else {
collect_one_slot(kip, i);
}
break;
}
}
if (dirty && ++kprobe_garbage_slots > INSNS_PER_PAGE)
collect_garbage_slots();+ mutex_unlock(&kprobe_mutex);
}
Quoted 210 lines
-----Original Message-----
From: [email protected] [mailto:[email protected]] On Behalf Of zhangxiliang
Sent: Friday, December 21, 2007 1:19 PM
To: 'Mathieu Desnoyers'; [email protected]; 'Ingo Molnar'; [email protected]
Cc: 'Andi Kleen'
Subject: RE: [patch 05/24] Text Edit Lock - Architecture Independent Codehello,
I have some questions for your patches.Quoted 14 lines
Paravirt and alternatives are always done when SMP is inactive, so there is no
need to use locks.(quoted diff) +0
-9-#ifndef CONFIG_KPROBES -#ifdef CONFIG_HOTPLUG_CPU - /* It must still be possible to apply SMP alternatives. */ - if (num_possible_cpus() <= 1) -#endif - { - change_page_attr(virt_to_page(start), - size >> PAGE_SHIFT, PAGE_KERNEL_RX); - printk("Write protecting the kernel text:%luk\n", size >> 10);
Review
Quoted 7 lines
(quoted diff) +5
-2- } -#endif + change_page_attr(virt_to_page(start), + size >> PAGE_SHIFT, PAGE_KERNEL_RX); + printk(KERN_INFO "Write protecting the kernel text: %luk\n", + size >> 10); +Why "
mark_rodata_ro" doesn't consider smp instance? Maybe it will be appied in
future.Quoted 6 lines
===================================================================
---linux-2.6-lttng.orig/kernel/kprobes.c2007-12-12
18:10:32.000000000 -0500
+++linux-2.6-lttng/kernel/kprobes.c2007-12-12
18:10:34.000000000 -0500Review valid_p: · line 652
Quoted 9 lines
(quoted diff) +2
-0@@ -644,7 +644,9 @@ valid_p: list_del_rcu(&p->list); kfree(old_p); } + mutex_lock(&kprobe_mutex); arch_remove_kprobe(p); + mutex_unlock(&kprobe_mutex); } else { mutex_lock(&kprobe_mutex); if (p->break_handler)I think "
mutex_lock" and "mutex_unlock" shoud be in architecture code. In "__register_kprobe" funtion, its implement "arch_prepare_kprobe" and
"arch_arm_kprobe" is also depended on arch. So the remove implement is not
the same on the different architecture code.Maybe it doesn't need the
mutex_lockin "arch_remove_kprobe" on some embeded
system chips if linux can support the other embeded system chips in future.Quoted 9 lines
-----Original Message-----
From: [email protected] [mailto:[email protected]] On Behalf Of Mathieu Desnoyers
Sent: Friday, December 21, 2007 9:55 AM
To: [email protected]; Ingo Molnar; [email protected]
Cc: Mathieu Desnoyers; Andi Kleen
Subject: [patch 05/24] Text Edit Lock - ArchitectureIndependent Code
Quoted 2 lines
This is an architecture independant synchronization around
kernel text
Quoted 83 lines
modifications through use of a global mutex.
A mutex has been chosen so that kprobes, the main user of this, can sleep during
memory allocation between the memory read of the instructions it must replace
and the memory write of the breakpoint.Other user of this interface: immediate values.
Paravirt and alternatives are always done when SMP is inactive, so there is no
need to use locks.Signed-off-by Mathieu Desnoyers <[email protected]>CC Andi Kleen <[email protected]>--- include/linux/memory.h | 7 +++++++ mm/memory.c | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+)Index:
linux-2.6-lttng/include/linux/memory.h
===================================================================
---linux-2.6-lttng.orig/include/linux/memory.h>2007-11-07 11:11:26.000000000 -0500+++
linux-2.6-lttng/include/linux/memory.h2007-11-07
11:13:48.000000000 -0500include/linux/memory.h +2
-0@@ -93,4 +93,11 @@ extern int memory_notify(unsigned long v#define hotplug_memory_notifier (fn, pri) do { } while (0)#endif +/* +* Take and release the kernel text modification lock, used for code patching.
+ * Users of this lock can sleep.
+ */include/linux/memory.h +3
-0+extern void kernel_text_lock (void ); +extern void kernel_text_unlock (void ); +#endif /* _LINUX_MEMORY_H_ */ Index:
linux-2.6-lttng/mm/memory.c
===================================================================
---linux-2.6-lttng.orig/mm/memory.c2007-11-07
11:12:33.000000000 -0500
+++linux-2.6-lttng/mm/memory.c2007-11-07
11:14:25.000000000 -0500mm/memory.c +4
-0@@ -50,6 +50,8 @@#include <linux/delayacct.h> #include <linux/init.h> #include <linux/writeback.h> +#include <linux/kprobes.h> +#include <linux/mutex.h> #include <asm/pgalloc.h> #include <asm/uaccess.h> @@ -84,6 +86,12 @@ EXPORT_SYMBOL(high_memory);int randomize_va_space__read_mostly =1 ; +/* +* mutex protecting text section modification (dynamic code patching).
+ * some users need to sleep (allocating memory...) while they hold this lock.
+ */mm/memory.c +7
-0+static DEFINE_MUTEX (text_mutex); +static int __init disable_randmaps (char *s) { randomize_va_space =0 ;@@ -2748,3 +2756,29 @@ int access_process_vm(struct task_structreturn buf - old_buf; } + +/** +* kernel_text_lock - Take the kernel text modification lock +* +* Insures mutual write exclusion of kernel and modules text live text
+ * modification. Should be used for code patching.
+ * Users of this lock can sleep.
+ */mm/memory.c int access_process_vm(struct task_struct
Quoted 8 lines
mm/memory.c +8
-0+void __kprobes kernel_text_lock (void ) +{ +mutex_lock (&text_mutex); +} +EXPORT_SYMBOL_GPL (kernel_text_lock); + +/** +* kernel_text_unlock - Release the kernel text modification lock
Quoted 15 lines
+ *
+ * Insures mutual write exclusion of kernel and modules text live text
+ * modification. Should be used for code patching.
+ * Users of this lock can sleep.
+ */mm/memory.c +5
-1+void __kprobes kernel_text_unlock (void ) +{ +mutex_unlock (&text_mutex); +} +EXPORT_SYMBOL_GPL (kernel_text_unlock); --Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de MontrealQuoted 10 lines
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 05/24] Text Edit Lock - Architecture Independent Code
In reply to zhangxiliangQuoted 3 lines · * zhangxiliang ([email protected]) wrote:
hello,
I have some questions for your patches.
Hi,
Quoted 16 lines
Quoted 16 lines
Paravirt and alternatives are always done when SMP is inactive, so there is no
need to use locks.(quoted diff) +0
-9-#ifndef CONFIG_KPROBES -#ifdef CONFIG_HOTPLUG_CPU - /* It must still be possible to apply SMP alternatives. */ - if (num_possible_cpus() <= 1) -#endif - { - change_page_attr(virt_to_page(start), - size >> PAGE_SHIFT, PAGE_KERNEL_RX); - printk("Write protecting the kernel text:%luk\n", size >> 10);
(quoted diff) +0
-2- } -#endif
Quoted 9 lines
Review
Quoted 5 lines
(quoted diff) +5
-0+ change_page_attr(virt_to_page(start), + size >> PAGE_SHIFT, PAGE_KERNEL_RX); + printk(KERN_INFO "Write protecting the kernel text: %luk\n", + size >> 10); +Why "
mark_rodata_ro" doesn't consider smp instance? Maybe it will be appied in future.
In its previous state, mark_rodata_ro was disabled in these situations :
- System supports CPU_HOTPLUG (alternatives will need to be applied when we pass from 1->2 and 2->1 cpu.
- System supports KPROBES, it need to put breakpoints in the code.
The main effect of the change I introduce is that I allow the kernel code to be marked RO even in these situations. Alternatives and kprobes uses the text_poke to modify kernel code, which temporarily disabled the Write Protection bit on the local CPU so the memory write to RO pages can be done.
So I guess the answer to your question is : previously, mark_rodata_ro did not support CPU HOTPLUG nor KPROBES, and now it does, which is much cleaner.
Quoted 22 lines
Quoted 7 lines
===================================================================
---linux-2.6-lttng.orig/kernel/kprobes.c2007-12-12
18:10:32.000000000 -0500
+++linux-2.6-lttng/kernel/kprobes.c2007-12-12
18:10:34.000000000 -0500Review valid_p: · line 652
Quoted 9 lines
(quoted diff) +2
-0@@ -644,7 +644,9 @@ valid_p: list_del_rcu(&p->list); kfree(old_p); } + mutex_lock(&kprobe_mutex); arch_remove_kprobe(p); + mutex_unlock(&kprobe_mutex); } else { mutex_lock(&kprobe_mutex); if (p->break_handler)I think "
mutex_lock" and "mutex_unlock" shoud be in architecture code. In "__register_kprobe" funtion, its implement "arch_prepare_kprobe" and "arch_arm_kprobe" is also depended on arch. So the remove implement is not the same on the different architecture code.
Quoted 3 lines
Maybe it doesn't need the
mutex_lockin "arch_remove_kprobe" on some embeded system chips if linux can support the other embeded system chips in future.
Which patch is this coming from ?
My Text Edit Lock - kprobes architecture independent support patch _removes_ the kprobe_mutex taken around arch_remove_kprobes because it is useless, I don't see how this patch snippet applies to my patchset at all.
If you suggest to change the way locking is currently done in kprobes, please do this in a separate thread, as a RFC ?
Mathieu
Quoted 131 lines
Quoted 128 lines
-----Original Message-----
From: [email protected] [mailto:[email protected]] On Behalf Of Mathieu Desnoyers
Sent: Friday, December 21, 2007 9:55 AM
To: [email protected]; Ingo Molnar; [email protected]
Cc: Mathieu Desnoyers; Andi Kleen
Subject: [patch 05/24] Text Edit Lock - Architecture Independent CodeThis is an architecture independant synchronization around kernel text modifications through use of a global mutex.
A mutex has been chosen so that kprobes, the main user of this, can sleep during
memory allocation between the memory read of the instructions it must replace
and the memory write of the breakpoint.Other user of this interface: immediate values.
Paravirt and alternatives are always done when SMP is inactive, so there is no
need to use locks.Signed-off-by Mathieu Desnoyers <[email protected]>CC Andi Kleen <[email protected]>--- include/linux/memory.h | 7 +++++++ mm/memory.c | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+)Index:
linux-2.6-lttng/include/linux/memory.h
===================================================================
---linux-2.6-lttng.orig/include/linux/memory.h>2007-11-07 11:11:26.000000000 -0500+++
linux-2.6-lttng/include/linux/memory.h2007-11-07
11:13:48.000000000 -0500include/linux/memory.h +2
-0@@ -93,4 +93,11 @@ extern int memory_notify(unsigned long v#define hotplug_memory_notifier (fn, pri) do { } while (0)#endif +/* +* Take and release the kernel text modification lock, used for code patching.
+ * Users of this lock can sleep.
+ */include/linux/memory.h +3
-0+extern void kernel_text_lock (void ); +extern void kernel_text_unlock (void ); +#endif /* _LINUX_MEMORY_H_ */ Index:
linux-2.6-lttng/mm/memory.c
===================================================================
---linux-2.6-lttng.orig/mm/memory.c2007-11-07
11:12:33.000000000 -0500
+++linux-2.6-lttng/mm/memory.c2007-11-07
11:14:25.000000000 -0500mm/memory.c +4
-0@@ -50,6 +50,8 @@#include <linux/delayacct.h> #include <linux/init.h> #include <linux/writeback.h> +#include <linux/kprobes.h> +#include <linux/mutex.h> #include <asm/pgalloc.h> #include <asm/uaccess.h> @@ -84,6 +86,12 @@ EXPORT_SYMBOL(high_memory);int randomize_va_space__read_mostly =1 ; +/* +* mutex protecting text section modification (dynamic code patching).
+ * some users need to sleep (allocating memory...) while they hold this lock.
+ */mm/memory.c +7
-0+static DEFINE_MUTEX (text_mutex); +static int __init disable_randmaps (char *s) { randomize_va_space =0 ;@@ -2748,3 +2756,29 @@ int access_process_vm(struct task_structreturn buf - old_buf; } + +/** +* kernel_text_lock - Take the kernel text modification lock +* +* Insures mutual write exclusion of kernel and modules text live text
+ * modification. Should be used for code patching.
+ * Users of this lock can sleep.
+ */mm/memory.c +10
-0+void __kprobes kernel_text_lock (void ) +{ +mutex_lock (&text_mutex); +} +EXPORT_SYMBOL_GPL (kernel_text_lock); + +/** +* kernel_text_unlock - Release the kernel text modification lock +* +* Insures mutual write exclusion of kernel and modules text live text
+ * modification. Should be used for code patching.
+ * Users of this lock can sleep.
+ */mm/memory.c +5
-0+void __kprobes kernel_text_unlock (void ) +{ +mutex_unlock (&text_mutex); +} +EXPORT_SYMBOL_GPL (kernel_text_unlock);Signature · 11 lines
-- Mathieu Desnoyers Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Signature · 3 lines
Mathieu Desnoyers Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
PATCH 6/24 Text Edit Lock - Alternative code for x86
In reply to Mathieu DesnoyersFix a memcpy that should be a text_poke (in apply_alternatives).
Use kernel_wp_save/kernel_wp_restore in text_poke to support DEBUG_RODATA correctly and so the CPU HOTPLUG special case can be removed.
Add text_poke_early, for alternatives and paravirt boot-time and module load time patching.
Notes:
- A macro is used instead of an inline function to deal with circular header include otherwise necessary for read_cr0 and preempt_disable/enable.
Changelog:
- Fix text_set and text_poke alignment check (mixed up bitwise and and or)
- Remove text_set
- Use the new macro INIT_ARRAY() to stop polluting the C files with ({ }) brackets (which breaks some c parsers in editors).
- Export add_nops, so it can be used by others.
- Remove x86 test for "wp_works_ok", it will just be ignored by the architecture if not supported.
- Document text_poke_early.
- Remove clflush, since it breaks some VIA architectures and is not strictly necessary.
- Add kerneldoc to text_poke and text_poke_early.
- Remove arg cr0 from kernel_wp_save/restore. Change the macro name for kernel_wp_disable/enable.
--- arch/x86/kernel/alternative.c | 56 ++++++++++++++++++++++++++++++++------- include/asm-x86/alternative_32.h | 36 ++++++++++++++++++++++++- include/asm-x86/alternative_64.h | 36 ++++++++++++++++++++++++- 3 files changed, 116 insertions(+), 12 deletions(-)
arch/x86/kernel/alternative.c +46 -10
@@ -173,7 +173,7 @@ static const unsigned char*const * find_#endif /* CONFIG_X86_64 */ /* Use this to add nops to a buffer, then text_poke the whole buffer. */ -static void add_nops (void *insns,unsigned int len) +void add_nops (void *insns,unsigned int len) {const unsigned char *const *noptable =find_nop_table ();
@@ -186,6 +186,7 @@ static void add_nops(void *insns, unsign len -= noplen; } } +EXPORT_SYMBOL_GPL (add_nops);extern struct alt_instr __alt_instructions[], __alt_instructions_end[];extern u8 *__smp_locks[], *__smp_locks_end[];
@@ -219,7 +220,7 @@ void apply_alternatives(struct alt_instrmemcpy (insnbuf, a->replacement, a->replacementlen);add_nops (insnbuf + a->replacementlen, a->instrlen - a->replacementlen); -text_poke (instr, insnbuf, a->instrlen); +text_poke_early (instr, insnbuf, a->instrlen); } }
@@ -234,7 +235,8 @@ static void alternatives_smp_lock(u8 **scontinue ;if (*ptr > text_end)continue ; -text_poke (*ptr, ((unsigned char []){0xf0 }),1 );/* add lock prefix */ +/* add lock prefix */ + text_poke(*ptr, INIT_ARRAY(unsigned char, 0xf0, 1), 1); }; }
@@ -397,7 +399,7 @@ void apply_paravirt(struct paravirt_patc/* Pad the rest with nops */ add_nops (insnbuf + used, p->len - used); -text_poke (p->instr, insnbuf, p->len); +text_poke_early (p->instr, insnbuf, p->len); } } extern struct paravirt_patch_site __start_parainstructions[],
@@ -457,18 +459,52 @@ void __init alternative_instructions(voi#endif } -/* -* Warning: +/** +* text_poke_early - Update instructions on a live kernel at boot time +* @addr: address to modify +* @opcode: source of the copy +* @len: length to copy +* * When you use this code to patch more than one byte of an instruction * you need to make sure that other CPUs cannot execute this code in parallel. -* Also no thread must be currently preempted in the middle of these instructions. -* And on the local CPU you need to be protected again NMI or MCE handlers -* seeing an inconsistent instruction while you patch. +* Also no thread must be currently preempted in the middle of these +* instructions. And on the local CPU you need to be protected again NMI or MCE +* handlers seeing an inconsistent instruction while you patch. +* Warning: read_cr0 is modified by paravirt, this is why we have _early +* versions. They are not in the __init section because they can be used at +* module load time. */ -void __kprobes text_poke (void *addr,unsigned char *opcode,int len) +void *text_poke_early (void *addr,const void *opcode,size_t len) {memcpy (addr, opcode, len);sync_core ();/* Could also do a CLFLUSH here to speed up CPU recovery; but that causes hangs on some VIA CPUs. */ +return addr; } + +/** +* text_poke - Update instructions on a live kernel +* @addr: address to modify +* @opcode: source of the copy +* @len: length to copy +* +* Only atomic text poke/set should be allowed when not doing early patching. +* It means the size must be writable atomically and the address must be aligned +* in a way that permits an atomic write. +*/ +void *__kprobes text_poke (void *addr,const void *opcode,size_t len) +{ +BUG_ON (len >sizeof (long )); +BUG_ON ((((long )addr + len -1 ) & ~(sizeof (long ) -1 )) + - ((long )addr & ~(sizeof (long ) -1 ))); +kernel_wp_disable (); +memcpy (addr, opcode, len); +kernel_wp_enable (); +sync_core (); +/* Could also do a CLFLUSH here to speed up CPU recovery; but +that causes hangs on some VIA CPUs. */ +return addr; +} + +
include/asm-x86/alternative_32.h +35 -1
@@ -4,6 +4,7 @@#include <asm/types.h> #include <linux/stddef.h> #include <linux/types.h> +#include <asm/processor-flags.h> struct alt_instr {u8 *instr;/* original instruction */
@@ -149,6 +150,39 @@ apply_paravirt(struct paravirt_patch_sit#define __parainstructions_end NULL#endif -extern void text_poke (void *addr,unsigned char *opcode,int len); +extern void add_nops (void *insns,unsigned int len); + +/* +* Clear and restore the kernel write-protection flag on the local CPU. +* Allows the kernel to edit read-only pages. +* Side-effect: any interrupt handler running between save and restore will have +* the ability to write to read-only pages. +* +* Warning: +* Code patching in the UP case is safe if NMIs and MCE handlers are stopped and +* no thread can be preempted in the instructions being modified (no iret to an +* invalid instruction possible) or if the instructions are changed from a +* consistent state to another consistent state atomically. +* More care must be taken when modifying code in the SMP case because of +* Intel's errata. +* On the local CPU you need to be protected again NMI or MCE handlers seeing an +* inconsistent instruction while you patch. +* The _early version does not use read_cr0(), which can be paravirtualized. +*/ + +extern void *text_poke (void *addr,const void *opcode,size_t len); +extern void *text_poke_early (void *addr,const void *opcode,size_t len); + +#define kernel_wp_disable () \ +do { \ + preempt_disable(); \ + write_cr0(read_cr0() & ~X86_CR0_WP); \ +} while (0) + +#define kernel_wp_enable () \ +do { \ + write_cr0(read_cr0() | X86_CR0_WP); \ + preempt_enable(); \ +} while (0)#endif /* _I386_ALTERNATIVE_H */
include/asm-x86/alternative_64.h +35 -1
@@ -5,6 +5,7 @@#include <linux/types.h> #include <linux/stddef.h> +#include <asm/processor-flags.h> /* * Alternative inline assembly for SMP.
@@ -154,6 +155,39 @@ apply_paravirt(struct paravirt_patch *st#define __parainstructions_end NULL#endif -extern void text_poke (void *addr,unsigned char *opcode,int len); +extern void add_nops (void *insns,unsigned int len); + +/* +* Clear and restore the kernel write-protection flag on the local CPU. +* Allows the kernel to edit read-only pages. +* Side-effect: any interrupt handler running between save and restore will have +* the ability to write to read-only pages. +* +* Warning: +* Code patching in the UP case is safe if NMIs and MCE handlers are stopped and +* no thread can be preempted in the instructions being modified (no iret to an +* invalid instruction possible) or if the instructions are changed from a +* consistent state to another consistent state atomically. +* More care must be taken when modifying code in the SMP case because of +* Intel's errata. +* On the local CPU you need to be protected again NMI or MCE handlers seeing an +* inconsistent instruction while you patch. +* The _early version does not use read_cr0(), which can be paravirtualized. +*/ + +extern void *text_poke (void *addr,const void *opcode,size_t len); +extern void *text_poke_early (void *addr,const void *opcode,size_t len); + +#define kernel_wp_disable () \ +do { \ + preempt_disable(); \ + write_cr0(read_cr0() & ~X86_CR0_WP); \ +} while (0) + +#define kernel_wp_enable () \ +do { \ + write_cr0(read_cr0() | X86_CR0_WP); \ + preempt_enable(); \ +} while (0)#endif /* _X86_64_ALTERNATIVE_H */
Signature · 3 lines
Mathieu Desnoyers Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
PATCH 7/24 Text Edit Lock - kprobes architecture independent support
In reply to Mathieu DesnoyersUse the mutual exclusion provided by the text edit lock in the kprobes code. It allows coherent manipulation of the kernel code by other subsystems.
Changelog:
Move the kernel_text_lock/unlock out of the for loops.
--- kernel/kprobes.c | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-)
kernel/kprobes.c +13 -6
@@ -43,6 +43,7 @@#include <linux/seq_file.h> #include <linux/debugfs.h> #include <linux/kdebug.h> +#include <linux/memory.h> #include <asm-generic/sections.h> #include <asm/cacheflush.h>
@@ -568,9 +569,10 @@ static int __kprobes __register_kprobe(s goto out; } +kernel_text_lock (); ret =arch_prepare_kprobe (p);if (ret) - goto out; + goto out_unlock_text;INIT_HLIST_NODE (&p->hlist); hlist_add_head_rcu(&p->hlist,
@@ -578,7 +580,8 @@ static int __kprobes __register_kprobe(sif (kprobe_enabled)arch_arm_kprobe (p); - +out_unlock_text: +kernel_text_unlock (); out:mutex_unlock (&kprobe_mutex);
@@ -621,8 +624,11 @@ valid_p:* enabled - otherwise, the breakpoint would already have * been removed. We save on flushing icache. */ -if (kprobe_enabled) +if (kprobe_enabled) { +kernel_text_lock ();arch_disarm_kprobe (p); +kernel_text_unlock (); + }hlist_del_rcu (&old_p->hlist); cleanup_p =1 ; }else {
@@ -644,9 +650,7 @@ valid_p:list_del_rcu (&p->list);kfree (old_p); } -mutex_lock (&kprobe_mutex);arch_remove_kprobe (p); -mutex_unlock (&kprobe_mutex); }else {mutex_lock (&kprobe_mutex);if (p->break_handler)
@@ -717,7 +721,6 @@ static int __kprobes pre_handler_kretpro ri->rp = rp; ri->task = current;arch_prepare_kretprobe (ri, regs); -/* XXX(hch): why is there no hlist_move_head? */ hlist_del (&ri->uflist);hlist_add_head (&ri->uflist, &ri->rp->used_instances);
@@ -938,11 +941,13 @@ static void __kprobes enable_all_kprobesif (kprobe_enabled) goto already_enabled; +kernel_text_lock ();for (i =0 ; i <KPROBE_TABLE_SIZE ; i++) { head = &kprobe_table[i];hlist_for_each_entry_rcu (p, node, head, hlist)arch_arm_kprobe (p); } +kernel_text_unlock (); kprobe_enabled = true;printk (KERN_INFO "Kprobes globally enabled\n" );
@@ -967,6 +972,7 @@ static void __kprobes disable_all_kprobe kprobe_enabled = false;printk (KERN_INFO "Kprobes globally disabled\n" ); +kernel_text_lock ();for (i =0 ; i <KPROBE_TABLE_SIZE ; i++) { head = &kprobe_table[i];hlist_for_each_entry_rcu (p, node, head, hlist) {
@@ -974,6 +980,7 @@ static void __kprobes disable_all_kprobearch_disarm_kprobe (p); } } +kernel_text_unlock ();mutex_unlock (&kprobe_mutex);/* Allow all currently running kprobes to complete */
Signature · 3 lines
Mathieu Desnoyers Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
PATCH 8/24 Text Edit Lock - kprobes x86_32
In reply to Mathieu DesnoyersMake kprobes use INIT_ARRAY().
--- arch/x86/kernel/kprobes_32.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
arch/x86/kernel/kprobes_32.c +3 -2
@@ -176,12 +176,13 @@ int __kprobes arch_prepare_kprobe(structvoid __kprobes arch_arm_kprobe (struct kprobe *p) { -text_poke (p->addr, ((unsigned char []){BREAKPOINT_INSTRUCTION }),1 ); + text_poke(p->addr, INIT_ARRAY(unsigned char, BREAKPOINT_INSTRUCTION, 1), +1 ); }void __kprobes arch_disarm_kprobe (struct kprobe *p) { -text_poke (p->addr, &p->opcode,1 ); + text_poke(p->addr, INIT_ARRAY(unsigned char, p->opcode, 1), 1); }void __kprobes arch_remove_kprobe (struct kprobe *p)
Signature · 3 lines
Mathieu Desnoyers Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
PATCH 9/24 Text Edit Lock - kprobes x86_64
In reply to Mathieu DesnoyersMake kprobes use INIT_ARRAY().
--- arch/x86/kernel/kprobes_64.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
arch/x86/kernel/kprobes_64.c +3 -2
@@ -215,12 +215,13 @@ static void __kprobes arch_copy_kprobe(svoid __kprobes arch_arm_kprobe (struct kprobe *p) { -text_poke (p->addr, ((unsigned char []){BREAKPOINT_INSTRUCTION }),1 ); + text_poke(p->addr, INIT_ARRAY(unsigned char, BREAKPOINT_INSTRUCTION, 1), +1 ); }void __kprobes arch_disarm_kprobe (struct kprobe *p) { -text_poke (p->addr, &p->opcode,1 ); + text_poke(p->addr, INIT_ARRAY(unsigned char, p->opcode, 1), 1); }void __kprobes arch_remove_kprobe (struct kprobe *p)
Signature · 3 lines
Mathieu Desnoyers Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
PATCH 10/24 Text Edit Lock - x86_32 standardize debug rodata
In reply to Mathieu DesnoyersStandardize DEBUG_RODATA, removing special cases for hotplug and kprobes.
--- arch/x86/mm/init_32.c | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-)
arch/x86/mm/init_32.c +7 -13
@@ -784,28 +784,21 @@ static int noinline do_test_wp_bit(void) }#ifdef CONFIG_DEBUG_RODATA -void mark_rodata_ro (void ) {unsigned long start =PFN_ALIGN (_text);unsigned long size =PFN_ALIGN (_etext) - start; -#ifndef CONFIG_KPROBES -#ifdef CONFIG_HOTPLUG_CPU -/* It must still be possible to apply SMP alternatives. */ -if (num_possible_cpus () <=1 ) -#endif - { -change_page_attr (virt_to_page (start), - size >>PAGE_SHIFT ,PAGE_KERNEL_RX ); -printk ("Write protecting the kernel text: %luk\n" , size >>10 ); - } -#endif +change_page_attr (virt_to_page (start), + size >>PAGE_SHIFT ,PAGE_KERNEL_RX ); +printk (KERN_INFO "Write protecting the kernel text: %luk\n" , + size >>10 ); + start += size; size = (unsigned long )__end_rodata - start;change_page_attr (virt_to_page (start), size >>PAGE_SHIFT ,PAGE_KERNEL_RO ); -printk ("Write protecting the kernel read-only data: %luk\n" , +printk (KERN_INFO "Write protecting the kernel read-only data: %luk\n" , size >>10 );/*
@@ -816,6 +809,7 @@ void mark_rodata_ro(void)*/ global_flush_tlb(); } +#endif void free_init_pages (char *what,unsigned long begin,unsigned long end)
Signature · 3 lines
Mathieu Desnoyers Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
PATCH 11/24 Text Edit Lock - x86_64 standardize debug rodata
In reply to Mathieu DesnoyersStandardize DEBUG_RODATA, removing special cases for hotplug and kprobes.
--- arch/x86_64/mm/init.c | 23 +++++------------------ 1 file changed, 5 insertions(+), 18 deletions(-)
arch/x86/mm/init_64.c +5 -18
@@ -592,25 +592,11 @@ void free_initmem(void)void mark_rodata_ro (void ) { -unsigned long start = (unsigned long )_stext, end; +unsigned long start =PFN_ALIGN (_stext); +unsigned long end =PFN_ALIGN (__end_rodata); -#ifdef CONFIG_HOTPLUG_CPU -/* It must still be possible to apply SMP alternatives. */ -if (num_possible_cpus () >1 ) - start = (unsigned long )_etext; -#endif - -#ifdef CONFIG_KPROBES - start = (unsigned long )__start_rodata; -#endif - - end = (unsigned long )__end_rodata; - start = (start +PAGE_SIZE -1 ) &PAGE_MASK ; - end &=PAGE_MASK ; -if (end <= start) -return ; - -change_page_attr_addr (start, (end - start) >>PAGE_SHIFT ,PAGE_KERNEL_RO ); +change_page_attr_addr (start, (end - start) >>PAGE_SHIFT , +PAGE_KERNEL_RO );printk (KERN_INFO "Write protecting the kernel read-only data: %luk\n" , (end - start) >>10 );
@@ -623,6 +609,7 @@ void mark_rodata_ro(void)*/ global_flush_tlb(); } +#endif #ifdef CONFIG_BLK_DEV_INITRD
Signature · 3 lines
Mathieu Desnoyers Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
PATCH 12/24 Immediate Values - Architecture Independent Code
In reply to Mathieu DesnoyersImmediate values are used as read mostly variables that are rarely updated. They use code patching to modify the values inscribed in the instruction stream. It provides a way to save precious cache lines that would otherwise have to be used by these variables.
There is a generic _imv_read() version, which uses standard global variables, and optimized per architecture imv_read() implementations, which use a load immediate to remove a data cache hit. When the immediate values functionnality is disabled in the kernel, it falls back to global variables.
It adds a new rodata section "__imv" to place the pointers to the enable value. Immediate values activation functions sits in kernel/immediate.c.
Immediate values refer to the memory address of a previously declared integer. This integer holds the information about the state of the immediate values associated, and must be accessed through the API found in linux/immediate.h.
At module load time, each immediate value is checked to see if it must be enabled. It would be the case if the variable they refer to is exported from another module and already enabled.
In the early stages of start_kernel(), the immediate values are updated to reflect the state of the variable they refer to.
* Why should this be merged *
It improves performances on heavy memory I/O workloads.
An interesting result shows the potential this infrastructure has by showing the slowdown a simple system call such as getppid() suffers when it is used under heavy user-space cache trashing:
Random walk L1 and L2 trashing surrounding a getppid() call:
(note: in this test, do_syscal_trace was taken at each system call, see Documentation/immediate.txt in these patches for details)
- No memory pressure : getppid() takes 1573 cycles
- With memory pressure : getppid() takes 15589 cycles
We therefore have a slowdown of 10 times just to get the kernel variables from memory. Another test on the same architecture (Intel P4) measured the memory latency to be 559 cycles. Therefore, each cache line removed from the hot path would improve the syscall time of 3.5% in these conditions.
Changelog:
- section __imv is already SHF_ALLOC
- Because of the wonders of ELF, section 0 has sh_addr and sh_size 0. So the if (immediateindex) is unnecessary here.
- Remove module_mutex usage: depend on functions implemented in module.c for that.
- Does not update tainted module's immediate values.
- remove imv_*_t types, add DECLARE_IMV() and DEFINE_IMV().
- imv_read(&var) becomes imv_read(var) because of this.
- Adding a new EXPORT_IMV_SYMBOL(_GPL).
- remove imv_if(). Should use if (unlikely(imv_read(var))) instead.
- Wait until we have gcc support before we add the imv_if macro, since its form may have to change.
- Dont't declare the __imv section in vmlinux.lds.h, just put the content in the rodata section.
- Simplify interface : remove imv_set_early, keep track of kernel boot status internally.
- Remove the ALIGN(8) before the __imv section. It is packed now.
- Uses an IPI busy-loop on each CPU with interrupts disabled as a simple, architecture agnostic, update mechanism.
- Use imv_* instead of immediate_*.
--- include/asm-generic/vmlinux.lds.h | 3 include/linux/immediate.h | 94 +++++++++++++++++++ include/linux/module.h | 16 +++ init/main.c | 8 + kernel/Makefile | 1 kernel/immediate.c | 187 ++++++++++++++++++++++++++++++++++++++ kernel/module.c | 50 +++++++++- 7 files changed, 358 insertions(+), 1 deletion(-)
include/linux/immediate.h new file +94 -0
@@ -0,0 +1,94 @@ +#ifndef _LINUX_IMMEDIATE_H +#define _LINUX_IMMEDIATE_H + +/* +* Immediate values, can be updated at runtime and save cache lines. +* +* (C) Copyright 2007 Mathieu Desnoyers <[email protected]> +* +* This file is released under the GPLv2. +* See the file COPYING for more details. +*/ + +#ifdef CONFIG_IMMEDIATE + +struct __imv { +unsigned long var;/* Pointer to the identifier variable of the +* immediate value +*/ +unsigned long imv;/* +* Pointer to the memory location of the +* immediate value within the instruction. +*/ +unsigned char size;/* Type size. */ +} __attribute__ ((packed)); + +#include <asm/immediate.h> + +/** +* imv_set - set immediate variable (with locking) +* @name: immediate value name +* @i: required value +* +* Sets the value of @name, taking the module_mutex if required by +* the architecture. +*/ +#define imv_set (name, i) \ + do { \ + name##__imv = (i); \ + core_imv_update(); \ + module_imv_update(); \ + } while (0) + +/* +* Internal update functions. +*/ +extern void core_imv_update (void ); +extern void imv_update_range (const struct __imv *begin, +const struct __imv *end); + +#else + +/* +* Generic immediate values: a simple, standard, memory load. +*/ + +/** +* imv_read - read immediate variable +* @name: immediate value name +* +* Reads the value of @name. +*/ +#define imv_read (name) _imv_read(name) + +/** +* imv_set - set immediate variable (with locking) +* @name: immediate value name +* @i: required value +* +* Sets the value of @name, taking the module_mutex if required by +* the architecture. +*/ +#define imv_set (name, i) (name##__imv = (i)) + +static inline void core_imv_update (void ) { } +static inline void module_imv_update (void ) { } + +#endif + +#define DECLARE_IMV (type, name) extern __typeof__(type) name##__imv +#define DEFINE_IMV (type, name) __typeof__(type) name##__imv + +#define EXPORT_IMV_SYMBOL (name) EXPORT_SYMBOL(name##__imv) +#define EXPORT_IMV_SYMBOL_GPL (name) EXPORT_SYMBOL_GPL(name##__imv) + +/** +* _imv_read - Read immediate value with standard memory load. +* @name: immediate value name +* +* Force a data read of the immediate value instead of the immediate value +* based mechanism. Useful for __init and __exit section data read. +*/ +#define _imv_read (name) (name##__imv) + +#endif
include/linux/module.h +16 -0
@@ -15,6 +15,7 @@#include <linux/stringify.h> #include <linux/kobject.h> #include <linux/moduleparam.h> +#include <linux/immediate.h> #include <linux/marker.h> #include <asm/local.h>
@@ -355,6 +356,10 @@ struct module/* The command line arguments (may be mangled). People like keeping pointers to this stuff */ char *args; +#ifdef CONFIG_IMMEDIATE +const struct __imv *immediate; +unsigned int num_immediate; +#endif #ifdef CONFIG_MARKERS struct marker *markers;unsigned int num_markers;
@@ -464,6 +469,9 @@ extern void print_modules(void);extern void module_update_markers (void ); +extern void _module_imv_update (void ); +extern void module_imv_update (void ); +#else /* !CONFIG_MODULES... */ #define EXPORT_SYMBOL (sym)#define EXPORT_SYMBOL_GPL (sym)
@@ -568,6 +576,14 @@ static inline void module_update_markers { } +static inline void _module_imv_update (void ) +{ +} + +static inline void module_imv_update (void ) +{ +} +#endif /* CONFIG_MODULES */ struct device_driver ;
kernel/module.c +49 -1
87 lines not on this page · raw message
kernel/immediate.c new file +187 -0
187 lines not on this page · raw message
init/main.c +8 -0
@@ -57,6 +57,7 @@#include <linux/device.h> #include <linux/kthread.h> #include <linux/sched.h> +#include <linux/immediate.h> #include <asm/io.h> #include <asm/bugs.h>
@@ -101,6 +102,11 @@ static inline void mark_rodata_ro(void)#ifdef CONFIG_TC extern void tc_init (void );#endif +#ifdef CONFIG_IMMEDIATE +extern void imv_init_complete (void ); +#else +static inline void imv_init_complete (void ) { } +#endif enum system_states system_state;EXPORT_SYMBOL (system_state);
@@ -518,6 +524,7 @@ asmlinkage void __init start_kernel(voidunwind_init ();lockdep_init ();cgroup_init_early (); +core_imv_update ();local_irq_disable ();early_boot_irqs_off ();
@@ -639,6 +646,7 @@ asmlinkage void __init start_kernel(voidcpuset_init ();taskstats_init_early ();delayacct_init (); +imv_init_complete ();check_bugs ();
kernel/Makefile +1 -0
@@ -56,6 +56,7 @@ obj-$(CONFIG_RELAY) += relay.o obj-$(CONFIG_SYSCTL) += utsname_sysctl.o obj-$(CONFIG_TASK_DELAY_ACCT) += delayacct.o obj-$(CONFIG_TASKSTATS) += taskstats.o tsacct.o +obj-$(CONFIG_IMMEDIATE) += immediate.o obj-$(CONFIG_MARKERS) += marker.o ifneq ($(CONFIG_SCHED_NO_NO_OMIT_FRAME_POINTER),y)
include/asm-generic/vmlinux.lds.h +3 -0
@@ -25,6 +25,9 @@ *(.rodata) *(.rodata.*) \ *(__vermagic)/* Kernel version magic */ \ *(__markers_strings )/* Markers: strings */ \ + VMLINUX_SYMBOL(__start___imv) = .; \ + *(__imv )/* Immediate values: pointers */ \ + VMLINUX_SYMBOL(__stop___imv) = .; \ } \ \ .rodata1 : AT(ADDR(.rodata1) - LOAD_OFFSET) { \
Signature · 3 lines
Mathieu Desnoyers Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
PATCH 13/24 Immediate Values - Kconfig menu in EMBEDDED
In reply to Mathieu DesnoyersImmediate values provide a way to use dynamic code patching to update variables sitting within the instruction stream. It saves caches lines normally used by static read mostly variables. Enable it by default, but let users disable it through the EMBEDDED menu with the "Disable immediate values" submenu entry.
Note: Since I think that I really should let embedded systems developers using RO memory the option to disable the immediate values, I choose to leave this menu option there, in the EMBEDDED menu. Also, the "CONFIG_IMMEDIATE" makes sense because we want to compile out all the immediate code when we decide not to use optimized immediate values at all (it removes otherwise unused code).
Changelog:
- Change ARCH_SUPPORTS_IMMEDIATE for ARCH_HAS_IMMEDIATE
--- init/Kconfig | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+)
init/Kconfig +24 -0
@@ -435,6 +435,20 @@ config CC_OPTIMIZE_FOR_SIZE config SYSCTL bool +config IMMEDIATE + default y if !DISABLE_IMMEDIATE + depends on HAVE_IMMEDIATE + bool + help + Immediate values are used as read-mostly variables that are rarely + updated. They use code patching to modify the values inscribed in the + instruction stream. It provides a way to save precious cache lines + that would otherwise have to be used by these variables. They can be + disabled through the EMBEDDED menu. + +config HAVE_IMMEDIATE + def_bool n + menuconfig EMBEDDED bool "Configure standard kernel features (for small systems)" help
@@ -670,6 +684,16 @@ config MARKERS source "arch/Kconfig" +config DISABLE_IMMEDIATE + default y if EMBEDDED + bool "Disable immediate values" if EMBEDDED + depends on HAVE_IMMEDIATE + help + Disable code patching based immediate values for embedded systems. It + consumes slightly more memory and requires to modify the instruction + stream each time a variable is updated. Should really be disabled for + embedded systems with read-only text. + endmenu # General setup config RT_MUTEXES
Signature · 3 lines
Mathieu Desnoyers Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
PATCH 14/24 Immediate Values - x86 Optimization
In reply to Mathieu Desnoyersx86 optimization of the immediate values which uses a movl with code patching to set/unset the value used to populate the register used as variable source.
Changelog:
- Use text_poke_early with cr0 WP save/restore to patch the bypass. We are doing non atomic writes to a code region only touched by us (nobody can execute it since we are protected by the imv_mutex).
- Put imv_set and _imv_set in the architecture independent header.
- Use $0 instead of %2 with (0) operand.
- Add x86_64 support, ready for i386+x86_64 -> x86 merge.
- Use asm-x86/asm.h.
Ok, so the most flexible solution that I see, that should fit for both i386 and x86_64 would be :
1 byte : "=Q" : Any register accessible as rh: a, b, c, and d. 2, 4 bytes : "=R" : Legacy register—the eight integer registers available
on all i386 processors (a, b, c, d, si, di, bp, sp). 8
bytes : (only for x86_64)
"=r" : A register operand is allowed provided that it is in a
general register.
That should make sure x86_64 won't try to use REX prefixed opcodes for 1, 2 and 4 bytes values.
- Create the instruction in a discarded section to calculate its size. This is how we can align the beginning of the instruction on an address that will permit atomic modificatino of the immediate value without knowing the size of the opcode used by the compiler.
- Bugfix : 8 bytes 64 bits immediate value was declared as "4 bytes" in the immediate structure.
- Change the immediate.c update code to support variable length opcodes.
- Vastly simplified, using a busy looping IPI with interrupts disabled. Does not protect against NMI nor MCE.
- Pack the __imv section. Use smallest types required for size (char).
- Use imv_* instead of immediate_*.
--- arch/x86/Kconfig | 1 include/asm-x86/immediate.h | 77 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 78 insertions(+)
include/asm-x86/immediate.h new file +77 -0
@@ -0,0 +1,77 @@ +#ifndef _ASM_X86_IMMEDIATE_H +#define _ASM_X86_IMMEDIATE_H + +/* +* Immediate values. x86 architecture optimizations. +* +* (C) Copyright 2006 Mathieu Desnoyers <[email protected]> +* +* This file is released under the GPLv2. +* See the file COPYING for more details. +*/ + +#include <asm/asm.h> + +/** +* imv_read - read immediate variable +* @name: immediate value name +* +* Reads the value of @name. +* Optimized version of the immediate. +* Do not use in __init and __exit functions. Use _imv_read() instead. +* If size is bigger than the architecture long size, fall back on a memory +* read. +* +* Make sure to populate the initial static 64 bits opcode with a value +* what will generate an instruction with 8 bytes immediate value (not the REX.W +* prefixed one that loads a sign extended 32 bits immediate value in a r64 +* register). +*/ +#define imv_read (name) \ + ({ \ + __typeof__(name##__imv) value; \ + BUILD_BUG_ON(sizeof(value) > 8); \ + switch (sizeof(value)) { \ + case 1: \ + asm(".section __imv,\"a\",@progbits\n\t" \ + _ASM_PTR "%c1, (3f)-%c2\n\t" \ + ".byte %c2\n\t" \ + ".previous\n\t" \ + "mov $0,%0\n\t" \ + "3:\n\t" \ + : "=q" (value) \ + : "i" (&name##__imv), \ + "i" (sizeof(value))); \ + break; \ + case 2: \ + case 4: \ + asm(".section __imv,\"a\",@progbits\n\t" \ + _ASM_PTR "%c1, (3f)-%c2\n\t" \ + ".byte %c2\n\t" \ + ".previous\n\t" \ + "mov $0,%0\n\t" \ + "3:\n\t" \ + : "=r" (value) \ + : "i" (&name##__imv), \ + "i" (sizeof(value))); \ + break; \ + case 8: \ + if (sizeof(long) < 8) { \ + value = name##__imv; \ + break; \ + } \ + asm(".section __imv,\"a\",@progbits\n\t" \ + _ASM_PTR "%c1, (3f)-%c2\n\t" \ + ".byte %c2\n\t" \ + ".previous\n\t" \ + "mov $0xFEFEFEFE01010101,%0\n\t" \ + "3:\n\t" \ + : "=r" (value) \ + : "i" (&name##__imv), \ + "i" (sizeof(value))); \ + break; \ + }; \ + value; \ + }) + +#endif /* _ASM_X86_IMMEDIATE_H */
arch/x86/Kconfig +1 -0
@@ -21,6 +21,7 @@ config X86 default y select HAVE_OPROFILE select HAVE_KPROBES + select HAVE_IMMEDIATE config GENERIC_TIME bool
Signature · 3 lines
Mathieu Desnoyers Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
H. Peter Anvin
reviewerRe: [patch 14/24] Immediate Values - x86 Optimization
This patch is modified by another patch in the sequence. This feels needlessly confusing when reviewing (especially since the comment doesn't look to match the code, e.g. w.r.t to "Q" and "R" constraints); can you reorder the patchset to avoid that?
-hpa
Re: [patch 14/24] Immediate Values - x86 Optimization
Quoted 5 lines · * H. Peter Anvin ([email protected]) wrote:
This patch is modified by another patch in the sequence. This feels needlessly confusing when reviewing (especially since the comment doesn't look to match the code, e.g. w.r.t to "Q" and "R" constraints); can you reorder the patchset to avoid that?
Argh.. Rusty asked to have a simplified version first, and then to implement the "more complex" one on top of it. However, in order to get the reentrancy I need for the markers, I need the complex version of the immediate values. Therefore, you find, in this patchset, the simple version first, and then, the more complex one implemented on top.
About this patch header, the initial idea was to use the "Q" and "R" constraints, but, as stated just below, the "q" and "r" constraints are used instead to make sure the REX prefixed opcodes for 1, 2, and 4 bytes immediate values are never used. So the complete header follows the source code, it's just that this paragraph could be clearer.
Mathieu
Quoted 1 line
-hpa
Signature · 3 lines
Mathieu Desnoyers Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
H. Peter Anvin
reviewerRe: [patch 14/24] Immediate Values - x86 Optimization
Quoted 7 lines · Mathieu Desnoyers
Argh.. Rusty asked to have a simplified version first, and then to implement the "more complex" one on top of it. However, in order to get the reentrancy I need for the markers, I need the complex version of the immediate values. Therefore, you find, in this patchset, the simple version first, and then, the more complex one implemented on top.
Quoted 6 lines
About this patch header, the initial idea was to use the "Q" and "R" constraints, but, as stated just below, the "q" and "r" constraints are used instead to make sure the REX prefixed opcodes for 1, 2, and 4 bytes immediate values are never used. So the complete header follows the source code, it's just that this paragraph could be clearer.
Then you have it backwards. "Q" and "R" avoid REX prefixes, "q" and "r" DO NOT.
-hpa
Re: [patch 14/24] Immediate Values - x86 Optimization (updated)
Quoted 12 lines · * H. Peter Anvin ([email protected]) wrote:
Quoted 10 lines · Mathieu Desnoyers
Argh.. Rusty asked to have a simplified version first, and then to implement the "more complex" one on top of it. However, in order to get the reentrancy I need for the markers, I need the complex version of the immediate values. Therefore, you find, in this patchset, the simple version first, and then, the more complex one implemented on top. About this patch header, the initial idea was to use the "Q" and "R" constraints, but, as stated just below, the "q" and "r" constraints are used instead to make sure the REX prefixed opcodes for 1, 2, and 4 bytes immediate values are never used. So the complete header follows the source code, it's just that this paragraph could be clearer.
Quoted 4 lines
Then you have it backwards. "Q" and "R" avoid REX prefixes, "q" and "r" DO NOT.
-hpa
Right.. I did that 1 month ago, which is already far away in my memory. Looking back at this, here is what is the real situation. I attach the patches that fixes the comments accordingly as reply to my original posts.
- "Redux" immediate values : no need to put a breakpoint, therefore, no need to know where the instruction starts. It's therefore OK to have a REX prefix.
- More reentrant immediate value : uses a breakpoint. Needs to know the instruction's first byte. This is why we keep the "instruction size" variable, so we can support the REX prefixed instructions too.
Therefore, the "q" and "r" constraints are OK : they _allow_ REX prefixes.
Mathieu
Signature · 3 lines
Mathieu Desnoyers Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
Re: [patch 14/24] Immediate Values - x86 Optimization (updated)
In reply to H. Peter Anvinx86 optimization of the immediate values which uses a movl with code patching to set/unset the value used to populate the register used as variable source.
Changelog:
- Use text_poke_early with cr0 WP save/restore to patch the bypass. We are doing non atomic writes to a code region only touched by us (nobody can execute it since we are protected by the imv_mutex).
- Put imv_set and _imv_set in the architecture independent header.
- Use $0 instead of %2 with (0) operand.
- Add x86_64 support, ready for i386+x86_64 -> x86 merge.
- Use asm-x86/asm.h.
Ok, so the most flexible solution that I see, that should fit for both x86 and x86_64 would be :
1 byte : "=q" : "a", "b", "c", or "d" register for the i386. For
x86-64 it is equivalent to "r" class (for 8-bit instructions that do not use upper halves).
2, 4, 8 bytes : "=r" : A register operand is allowed provided that it is in a
general register.
- "Redux" immediate values : no need to put a breakpoint, therefore, no need to know where the instruction starts. It's therefore OK to have a REX prefix.
- Bugfix : 8 bytes 64 bits immediate value was declared as "4 bytes" in the immediate structure.
- Change the immediate.c update code to support variable length opcodes.
- Vastly simplified, using a busy looping IPI with interrupts disabled. Does not protect against NMI nor MCE.
- Pack the __imv section. Use smallest types required for size (char).
- Use imv_* instead of immediate_*.
--- arch/x86/Kconfig | 1 include/asm-x86/immediate.h | 77 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 78 insertions(+)
include/asm-x86/immediate.h new file +77 -0
@@ -0,0 +1,77 @@ +#ifndef _ASM_X86_IMMEDIATE_H +#define _ASM_X86_IMMEDIATE_H + +/* +* Immediate values. x86 architecture optimizations. +* +* (C) Copyright 2006 Mathieu Desnoyers <[email protected]> +* +* This file is released under the GPLv2. +* See the file COPYING for more details. +*/ + +#include <asm/asm.h> + +/** +* imv_read - read immediate variable +* @name: immediate value name +* +* Reads the value of @name. +* Optimized version of the immediate. +* Do not use in __init and __exit functions. Use _imv_read() instead. +* If size is bigger than the architecture long size, fall back on a memory +* read. +* +* Make sure to populate the initial static 64 bits opcode with a value +* what will generate an instruction with 8 bytes immediate value (not the REX.W +* prefixed one that loads a sign extended 32 bits immediate value in a r64 +* register). +*/ +#define imv_read (name) \ + ({ \ + __typeof__(name##__imv) value; \ + BUILD_BUG_ON(sizeof(value) > 8); \ + switch (sizeof(value)) { \ + case 1: \ + asm(".section __imv,\"a\",@progbits\n\t" \ + _ASM_PTR "%c1, (3f)-%c2\n\t" \ + ".byte %c2\n\t" \ + ".previous\n\t" \ + "mov $0,%0\n\t" \ + "3:\n\t" \ + : "=q" (value) \ + : "i" (&name##__imv), \ + "i" (sizeof(value))); \ + break; \ + case 2: \ + case 4: \ + asm(".section __imv,\"a\",@progbits\n\t" \ + _ASM_PTR "%c1, (3f)-%c2\n\t" \ + ".byte %c2\n\t" \ + ".previous\n\t" \ + "mov $0,%0\n\t" \ + "3:\n\t" \ + : "=r" (value) \ + : "i" (&name##__imv), \ + "i" (sizeof(value))); \ + break; \ + case 8: \ + if (sizeof(long) < 8) { \ + value = name##__imv; \ + break; \ + } \ + asm(".section __imv,\"a\",@progbits\n\t" \ + _ASM_PTR "%c1, (3f)-%c2\n\t" \ + ".byte %c2\n\t" \ + ".previous\n\t" \ + "mov $0xFEFEFEFE01010101,%0\n\t" \ + "3:\n\t" \ + : "=r" (value) \ + : "i" (&name##__imv), \ + "i" (sizeof(value))); \ + break; \ + }; \ + value; \ + }) + +#endif /* _ASM_X86_IMMEDIATE_H */
arch/x86/Kconfig +1 -0
@@ -20,6 +20,7 @@ config X86 def_bool y select HAVE_OPROFILE select HAVE_KPROBES + select HAVE_IMMEDIATE config GENERIC_TIME def_bool y
Signature · 3 lines
Mathieu Desnoyers Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
PATCH 15/24 Add text_poke and sync_core to powerpc
In reply to Mathieu Desnoyers- Needed on architectures where we must surround live instruction modification with "WP flag disable".
- Turns into a memcpy on powerpc since there is no WP flag activated for instruction pages (yet..).
- Add empty sync_core to powerpc so it can be used in architecture independent code.
--- include/asm-powerpc/cacheflush.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
include/asm-powerpc/cacheflush.h +3 -1
@@ -63,7 +63,9 @@ extern void flush_dcache_phys_range(unsi#define copy_from_user_page (vma, page, vaddr, dst, src, len) \ memcpy(dst, src, len) - +#define text_poke memcpy +#define text_poke_early text_poke +#define sync_core ()#ifdef CONFIG_DEBUG_PAGEALLOC /* internal debugging function */
Signature · 3 lines
Mathieu Desnoyers Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
PATCH 16/24 Immediate Values - Powerpc Optimization
In reply to Mathieu DesnoyersPowerPC optimization of the immediate values which uses a li instruction, patched with an immediate value.
Changelog:
- Put imv_set and _imv_set in the architecture independent header.
- Pack the __imv section. Use smallest types required for size (char).
- Remove architecture specific update code : now handled by architecture agnostic code.
- Use imv_* instead of immediate_*.
--- arch/powerpc/Kconfig | 1 include/asm-powerpc/immediate.h | 55 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+)
include/asm-powerpc/immediate.h new file +55 -0
@@ -0,0 +1,55 @@ +#ifndef _ASM_POWERPC_IMMEDIATE_H +#define _ASM_POWERPC_IMMEDIATE_H + +/* +* Immediate values. PowerPC architecture optimizations. +* +* (C) Copyright 2006 Mathieu Desnoyers <[email protected]> +* +* This file is released under the GPLv2. +* See the file COPYING for more details. +*/ + +#include <asm/asm-compat.h> + +/** +* imv_read - read immediate variable +* @name: immediate value name +* +* Reads the value of @name. +* Optimized version of the immediate. +* Do not use in __init and __exit functions. Use _imv_read() instead. +*/ +#define imv_read (name) \ + ({ \ + __typeof__(name##__imv) value; \ + BUILD_BUG_ON(sizeof(value) > 8); \ + switch (sizeof(value)) { \ + case 1: \ + asm(".section __imv,\"a\",@progbits\n\t" \ + PPC_LONG "%c1, ((1f)-1)\n\t" \ + ".byte 1\n\t" \ + ".previous\n\t" \ + "li %0,0\n\t" \ + "1:\n\t" \ + : "=r" (value) \ + : "i" (&name##__imv)); \ + break; \ + case 2: \ + asm(".section __imv,\"a\",@progbits\n\t" \ + PPC_LONG "%c1, ((1f)-2)\n\t" \ + ".byte 2\n\t" \ + ".previous\n\t" \ + "li %0,0\n\t" \ + "1:\n\t" \ + : "=r" (value) \ + : "i" (&name##__imv)); \ + break; \ + case 4: \ + case 8: value = name##__imv; \ + break; \ + }; \ + value; \ + }) + +#endif /* _ASM_POWERPC_IMMEDIATE_H */
arch/powerpc/Kconfig +1 -0
@@ -81,6 +81,7 @@ config PPC default y select HAVE_OPROFILE select HAVE_KPROBES + select HAVE_IMMEDIATE config EARLY_PRINTK bool
Signature · 3 lines
Mathieu Desnoyers Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
PATCH 17/24 Immediate Values - Documentation
In reply to Mathieu DesnoyersChangelog:
- Remove imv_set_early (removed from API).
- Use imv_* instead of immediate_*.
--- Documentation/immediate.txt | 221 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 221 insertions(+)
Documentation/immediate.txt new file +221 -0
@@ -0,0 +1,221 @@ + Using the Immediate Values + + Mathieu Desnoyers + + +This document introduces Immediate Values and their use. + + +* Purpose of immediate values + +An immediate value is used to compile into the kernel variables that sit within +the instruction stream. They are meant to be rarely updated but read often. +Using immediate values for these variables will save cache lines. + +This infrastructure is specialized in supporting dynamic patching of the values +in the instruction stream when multiple CPUs are running without disturbing the +normal system behavior. + +Compiling code meant to be rarely enabled at runtime can be done using +if (unlikely(imv_read(var))) as condition surrounding the code. The +smallest data type required for the test (an 8 bits char) is preferred, since +some architectures, such as powerpc, only allow up to 16 bits immediate values. + + +* Usage + +In order to use the "immediate" macros, you should include linux/immediate.h. + +#include <linux/immediate.h> + +DEFINE_IMV(char, this_immediate); +EXPORT_IMV_SYMBOL(this_immediate); + + +And use, in the body of a function: + +Use imv_set(this_immediate) to set the immediate value. + +Use imv_read(this_immediate) to read the immediate value. + +The immediate mechanism supports inserting multiple instances of the same +immediate. Immediate values can be put in inline functions, inlined static +functions, and unrolled loops. + +If you have to read the immediate values from a function declared as __init or +__exit, you should explicitly use _imv_read(), which will fall back on a +global variable read. Failing to do so will leave a reference to the __init +section after it is freed (it would generate a modpost warning). + +You can choose to set an initial static value to the immediate by using, for +instance: + +DEFINE_IMV(long, myptr) = 10; + + +* Optimization for a given architecture + +One can implement optimized immediate values for a given architecture by +replacing asm-$ARCH/immediate.h. + + +* Performance improvement + + + * Memory hit for a data-based branch + +Here are the results on a 3GHz Pentium 4: + +number of tests: 100 +number of branches per test: 100000 +memory hit cycles per iteration (mean): 636.611 +L1 cache hit cycles per iteration (mean): 89.6413 +instruction stream based test, cycles per iteration (mean): 85.3438 +Just getting the pointer from a modulo on a pseudo-random value, doing + nothing with it, cycles per iteration (mean): 77.5044 + +So: +Base case: 77.50 cycles +instruction stream based test: +7.8394 cycles +L1 cache hit based test: +12.1369 cycles +Memory load based test: +559.1066 cycles + +So let's say we have a ping flood coming at +(14014 packets transmitted, 14014 received, 0% packet loss, time 1826ms) +7674 packets per second. If we put 2 markers for irq entry/exit, it +brings us to 15348 markers sites executed per second. + +(15348 exec/s) * (559 cycles/exec) / (3G cycles/s) = 0.0029 +We therefore have a 0.29% slowdown just on this case. + +Compared to this, the instruction stream based test will cause a +slowdown of: + +(15348 exec/s) * (7.84 cycles/exec) / (3G cycles/s) = 0.00004 +For a 0.004% slowdown. + +If we plan to use this for memory allocation, spinlock, and all sorts of +very high event rate tracing, we can assume it will execute 10 to 100 +times more sites per second, which brings us to 0.4% slowdown with the +instruction stream based test compared to 29% slowdown with the memory +load based test on a system with high memory pressure. + + + + * Markers impact under heavy memory load + +Running a kernel with my LTTng instrumentation set, in a test that +generates memory pressure (from userspace) by trashing L1 and L2 caches +between calls to getppid() (note: syscall_trace is active and calls +a marker upon syscall entry and syscall exit; markers are disarmed). +This test is done in user-space, so there are some delays due to IRQs +coming and to the scheduler. (UP 2.6.22-rc6-mm1 kernel, task with -20 +nice level) + +My first set of results: Linear cache trashing, turned out not to be +very interesting, because it seems like the linearity of the memset on a +full array is somehow detected and it does not "really" trash the +caches. + +Now the most interesting result: Random walk L1 and L2 trashing +surrounding a getppid() call. + +- Markers compiled out (but syscall_trace execution forced) +number of tests: 10000 +No memory pressure +Reading timestamps takes 108.033 cycles +getppid: 1681.4 cycles +With memory pressure +Reading timestamps takes 102.938 cycles +getppid: 15691.6 cycles + + +- With the immediate values based markers: +number of tests: 10000 +No memory pressure +Reading timestamps takes 108.006 cycles +getppid: 1681.84 cycles +With memory pressure +Reading timestamps takes 100.291 cycles +getppid: 11793 cycles + + +- With global variables based markers: +number of tests: 10000 +No memory pressure +Reading timestamps takes 107.999 cycles +getppid: 1669.06 cycles +With memory pressure +Reading timestamps takes 102.839 cycles +getppid: 12535 cycles + +The result is quite interesting in that the kernel is slower without +markers than with markers. I explain it by the fact that the data +accessed is not laid out in the same manner in the cache lines when the +markers are compiled in or out. It seems that it aligns the function's +data better to compile-in the markers in this case. + +But since the interesting comparison is between the immediate values and +global variables based markers, and because they share the same memory +layout, except for the movl being replaced by a movz, we see that the +global variable based markers (2 markers) adds 742 cycles to each system +call (syscall entry and exit are traced and memory locations for both +global variables lie on the same cache line). + + +- Test redone with less iterations, but with error estimates + +10 runs of 100 iterations each: Tests done on a 3GHz P4. Here I run getppid with +syscall trace inactive, comparing the case with memory pressure and without +memory pressure. (sorry, my system is not setup to execute syscall_trace this +time, but it will make the point anyway). + +No memory pressure +Reading timestamps: 150.92 cycles, std dev. 1.01 cycles +getppid: 1462.09 cycles, std dev. 18.87 cycles + +With memory pressure +Reading timestamps: 578.22 cycles, std dev. 269.51 cycles +getppid: 17113.33 cycles, std dev. 1655.92 cycles + + +Now for memory read timing: (10 runs, branches per test: 100000) +Memory read based branch: + 644.09 cycles, std dev. 11.39 cycles +L1 cache hit based branch: + 88.16 cycles, std dev. 1.35 cycles + + +So, now that we have the raw results, let's calculate: + +Memory read: +644.09±11.39 - 88.16±1.35 = 555.93±11.46 cycles + +Getppid without memory pressure: +1462.09±18.87 - 150.92±1.01 = 1311.17±18.90 cycles + +Getppid with memory pressure: +17113.33±1655.92 - 578.22±269.51 = 16535.11±1677.71 cycles + +Therefore, if we add 2 markers not based on immediate values to the getppid +code, which would add 2 memory reads, we would add +2 * 555.93±12.74 = 1111.86±25.48 cycles + +Therefore, + +1111.86±25.48 / 16535.11±1677.71 = 0.0672 + relative error: sqrt(((25.48/1111.86)^2)+((1677.71/16535.11)^2)) + = 0.1040 + absolute error: 0.1040 * 0.0672 = 0.0070 + +Therefore: 0.0672±0.0070 * 100% = 6.72±0.70 % + +We can therefore affirm that adding 2 markers to getppid, on a system with high +memory pressure, would have a performance hit of at least 6.0% on the system +call time, all within the uncertainty limits of these tests. The same applies to +other kernel code paths. The smaller those code paths are, the highest the +impact ratio will be. + +Therefore, not only is it interesting to use the immediate values to dynamically +activate dormant code such as the markers, but I think it should also be +considered as a replacement for many of the "read-mostly" static variables.
Signature · 3 lines
Mathieu Desnoyers Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
PATCH 18/24 Scheduler Profiling - Use Immediate Values
In reply to Mathieu DesnoyersUse immediate values with lower d-cache hit in optimized version as a condition for scheduler profiling call.
Changelog :
- Use imv_* instead of immediate_*.
--- drivers/kvm/kvm_main.c | 3 ++- include/linux/profile.h | 5 +++-- kernel/profile.c | 22 +++++++++++----------- kernel/sched_fair.c | 6 +----- 4 files changed, 17 insertions(+), 19 deletions(-)
kernel/profile.c +11 -11
@@ -42,8 +42,8 @@ static int (*timer_hook)(struct pt_regsstatic atomic_t *prof_buffer;static unsigned long prof_len, prof_shift; -int prof_on__read_mostly ; -EXPORT_SYMBOL_GPL (prof_on); +DEFINE_IMV (char, prof_on)__read_mostly ; +EXPORT_IMV_SYMBOL_GPL (prof_on);static cpumask_t prof_cpu_mask =CPU_MASK_ALL ;#ifdef CONFIG_SMP
@@ -61,7 +61,7 @@ static int __init profile_setup(char * sif (!strncmp (str, sleepstr,strlen (sleepstr))) {#ifdef CONFIG_SCHEDSTATS - prof_on =SLEEP_PROFILING ; +imv_set (prof_on,SLEEP_PROFILING );if (str[strlen (sleepstr)] ==',' ) str +=strlen (sleepstr) +1 ;if (get_option (&str, &par))
@@ -74,7 +74,7 @@ static int __init profile_setup(char * s "kernel sleep profiling requires CONFIG_SCHEDSTATS\n");#endif /* CONFIG_SCHEDSTATS */ }else if (!strncmp (str, schedstr,strlen (schedstr))) { - prof_on =SCHED_PROFILING ; +imv_set (prof_on,SCHED_PROFILING );if (str[strlen (schedstr)] ==',' ) str +=strlen (schedstr) +1 ;if (get_option (&str, &par))
@@ -83,7 +83,7 @@ static int __init profile_setup(char * s"kernel schedule profiling enabled (shift: %ld)\n" , prof_shift); }else if (!strncmp (str, kvmstr,strlen (kvmstr))) { - prof_on =KVM_PROFILING ; +imv_set (prof_on,KVM_PROFILING );if (str[strlen (kvmstr)] ==',' ) str +=strlen (kvmstr) +1 ;if (get_option (&str, &par))
@@ -93,7 +93,7 @@ static int __init profile_setup(char * s prof_shift); }else if (get_option (&str, &par)) { prof_shift = par; - prof_on =CPU_PROFILING ; +imv_set (prof_on,CPU_PROFILING );printk (KERN_INFO "kernel profiling enabled (shift: %ld)\n" , prof_shift); }
@@ -104,7 +104,7 @@ __setup("profile=", profile_setup);void __init profile_init (void ) { -if (!prof_on) +if (!_imv_read (prof_on))return ;/* only text is profiled */
@@ -293,7 +293,7 @@ void profile_hits(int type, void *__pc,int i, j, cpu;struct profile_hit *hits; -if (prof_on != type || !prof_buffer) +if (!prof_buffer)return ; pc =min ((pc - (unsigned long )_stext) >> prof_shift, prof_len -1 ); i = primary = (pc & (NR_PROFILE_GRP -1 )) <<PROFILE_GRPSHIFT ;
@@ -403,7 +403,7 @@ void profile_hits(int type, void *__pc, {unsigned long pc; -if (prof_on != type || !prof_buffer) +if (!prof_buffer)return ; pc = ((unsigned long )__pc - (unsigned long )_stext) >> prof_shift;atomic_add (nr_hits, &prof_buffer[min (pc, prof_len -1 )]);
@@ -560,7 +560,7 @@ static int __init create_hash_tables(voi }return 0 ; out_cleanup: - prof_on =0 ; +imv_set (prof_on,0 );smp_mb ();on_each_cpu (profile_nop,NULL ,0 ,1 );for_each_online_cpu (cpu) {
@@ -587,7 +587,7 @@ static int __init create_proc_profile(vo {struct proc_dir_entry *entry; -if (!prof_on) +if (!_imv_read (prof_on))return 0 ;if (create_hash_tables ())return -1 ;
include/linux/profile.h +3 -2
@@ -7,10 +7,11 @@#include <linux/init.h> #include <linux/cpumask.h> #include <linux/cache.h> +#include <linux/immediate.h> #include <asm/errno.h> -extern int prof_on__read_mostly ; +DECLARE_IMV (char, prof_on)__read_mostly ;#define CPU_PROFILING 1#define SCHED_PROFILING 2
@@ -38,7 +39,7 @@ static inline void profile_hit(int type,/* * Speedup for the common (no profiling enabled) case: */ -if (unlikely (prof_on == type)) +if (unlikely (imv_read (prof_on) == type))profile_hits (type, ip,1 ); }
drivers/kvm/kvm_main.c +2 -1
@@ -2053,7 +2053,8 @@ again:/* * Profile KVM exit RIPs: */ -if (unlikely (prof_on ==KVM_PROFILING )) { + +if (unlikely (imv_read (prof_on) ==KVM_PROFILING )) { kvm_x86_ops->cache_regs (vcpu);profile_hit (KVM_PROFILING , (void *)vcpu->rip); }
kernel/sched_fair.c +1 -5
@@ -461,12 +461,8 @@ static void enqueue_sleeper(struct cfs_r* get a milliseconds-range estimation of the amount of * time that the task spent sleeping: */ -if (unlikely (prof_on ==SLEEP_PROFILING )) { -struct task_struct *tsk =task_of (se); - -profile_hits (SLEEP_PROFILING , (void *)get_wchan (tsk), +profile_hits (SLEEP_PROFILING , (void *)get_wchan (task_of (se)), delta >> 20); - } }#endif }
Signature · 3 lines
Mathieu Desnoyers Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
PATCH 19/24 Immediate Values - Move Kprobes x86 restore_interrupt to kdebug.h
In reply to Mathieu DesnoyersSince the breakpoint handler is useful both to kprobes and immediate values, it makes sense to make the required restore_interrupt() available through asm-i386/kdebug.h.
--- include/asm-x86/kdebug.h | 12 ++++++++++++ include/asm-x86/kprobes_32.h | 9 --------- include/asm-x86/kprobes_64.h | 9 --------- 3 files changed, 12 insertions(+), 18 deletions(-)
include/asm-x86/kdebug.h +12 -0
@@ -3,6 +3,9 @@#include <linux/notifier.h> +#include <linux/ptrace.h> +#include <asm/system.h> +struct pt_regs ;/* Grossly misnamed. */
@@ -30,4 +33,13 @@ extern void dump_pagetable(unsigned longextern unsigned long oops_begin (void );extern void oops_end (unsigned long ); +/* trap3/1 are intr gates for kprobes. So, restore the status of IF, +* if necessary, before executing the original int3/1 (trap) handler. +*/ +static inline void restore_interrupts (struct pt_regs *regs) +{ +if (regs->eflags &IF_MASK ) +local_irq_enable (); +} +#endif
include/asm-x86/kprobes_32.h +0 -9
@@ -79,15 +79,6 @@ struct kprobe_ctlblk {struct prev_kprobe prev_kprobe; }; -/* trap3/1 are intr gates for kprobes. So, restore the status of IF, -* if necessary, before executing the original int3/1 (trap) handler. -*/ -static inline void restore_interrupts (struct pt_regs *regs) -{ -if (regs->eflags &IF_MASK ) -local_irq_enable (); -} -extern int kprobe_exceptions_notify (struct notifier_block *self,unsigned long val,void *data);extern int kprobe_fault_handler (struct pt_regs *regs,int trapnr);
include/asm-x86/kprobes_64.h +0 -9
@@ -72,15 +72,6 @@ struct kprobe_ctlblk {struct prev_kprobe prev_kprobe; }; -/* trap3/1 are intr gates for kprobes. So, restore the status of IF, -* if necessary, before executing the original int3/1 (trap) handler. -*/ -static inline void restore_interrupts (struct pt_regs *regs) -{ -if (regs->eflags &IF_MASK ) -local_irq_enable (); -} -extern int post_kprobe_handler (struct pt_regs *regs);extern int kprobe_fault_handler (struct pt_regs *regs,int trapnr);extern int kprobe_handler (struct pt_regs *regs);
Signature · 3 lines
Mathieu Desnoyers Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
PATCH 20/24 Add __discard section to x86
In reply to Mathieu DesnoyersAdd a __discard sectionto the linker script. Code produced in this section will not be put in the vmlinux file. This is useful when we have to calculate the size of an instruction before actually declaring it (for alignment purposes for instance). This is used by the immediate values.
--- arch/x86/kernel/vmlinux_32.lds.S | 1 + arch/x86/kernel/vmlinux_64.lds.S | 1 + 2 files changed, 2 insertions(+)
arch/x86/kernel/vmlinux_32.lds.S +1 -0
@@ -205,6 +205,7 @@ SECTIONS /* Sections to be discarded */ /DISCARD/ : { *(.exitcall.exit) + *(__discard) } STABS_DEBUG
arch/x86/kernel/vmlinux_64.lds.S +1 -0
@@ -227,6 +227,7 @@ SECTIONS /DISCARD/ : { *(.exitcall.exit) *(.eh_frame) + *(__discard) } STABS_DEBUG
Signature · 3 lines
Mathieu Desnoyers Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
PATCH 21/24 Immediate Values - x86 Optimization NMI and MCE support
In reply to Mathieu Desnoyersx86 optimization of the immediate values which uses a movl with code patching to set/unset the value used to populate the register used as variable source. It uses a breakpoint to bypass the instruction being changed, which lessens the interrupt latency of the operation and protects against NMIs and MCE.
Changelog:
- Use text_poke_early with cr0 WP save/restore to patch the bypass. We are doing non atomic writes to a code region only touched by us (nobody can execute it since we are protected by the imv_mutex).
- Add x86_64 support, ready for i386+x86_64 -> x86 merge.
- Use asm-x86/asm.h.
- Change the immediate.c update code to support variable length opcodes.
- Use imv_* instead of immediate_*.
- Use kernel_wp_disable/enable instead of save/restore.
--- arch/x86/kernel/Makefile_32 | 1 arch/x86/kernel/Makefile_64 | 1 arch/x86/kernel/immediate.c | 277 ++++++++++++++++++++++++++++++++++++++++++++ arch/x86/kernel/traps_32.c | 10 - include/asm-x86/immediate.h | 42 +++++- 5 files changed, 322 insertions(+), 9 deletions(-)
include/asm-x86/immediate.h +37 -5
@@ -12,6 +12,18 @@#include <asm/asm.h> +struct __imv { +unsigned long var;/* Pointer to the identifier variable of the +* immediate value +*/ +unsigned long imv;/* +* Pointer to the memory location of the +* immediate value within the instruction. +*/ +unsigned char size;/* Type size. */ +unsigned char insn_size;/* Type size. */ +} __attribute__ ((packed)); +/** * imv_read - read immediate variable * @name: immediate value name
@@ -26,6 +38,11 @@* what will generate an instruction with 8 bytes immediate value (not the REX.W * prefixed one that loads a sign extended 32 bits immediate value in a r64 * register). +* +* Create the instruction in a discarded section to calculate its size. This is +* how we can align the beginning of the instruction on an address that will +* permit atomic modification of the immediate value without knowing the size of +* the opcode used by the compiler. The operand size is known in advance. */ #define imv_read (name) \ ({ \
@@ -35,8 +52,9 @@case 1 : \ asm(".section __imv,\"a\",@progbits\n\t" \ _ASM_PTR"%c1, (3f)-%c2\n\t" \ -".byte %c2\n\t" \ +".byte %c2, (3f-2f)\n\t" \".previous\n\t" \ +"2:\n\t" \"mov $0,%0\n\t" \"3:\n\t" \ : "=q" (value) \
@@ -45,10 +63,16 @@break ; \case 2 : \case 4 : \ - asm(".section __imv,\"a\",@progbits\n\t" \ + asm(".section __discard,\"\",@progbits\n\t" \ +"1:\n\t" \ +"mov $0,%0\n\t" \ +"2:\n\t" \ +".previous\n\t" \ +".section __imv,\"a\",@progbits\n\t" \ _ASM_PTR"%c1, (3f)-%c2\n\t" \ -".byte %c2\n\t" \ +".byte %c2, (2b-1b)\n\t" \".previous\n\t" \ +".org . + ((-.-(2b-1b)) & (%c2-1)), 0x90\n\t" \"mov $0,%0\n\t" \"3:\n\t" \ : "=r" (value) \
@@ -60,10 +84,16 @@ value = name##__imv; \break ; \ } \ - asm(".section __imv,\"a\",@progbits\n\t" \ + asm(".section __discard,\"\",@progbits\n\t" \ +"1:\n\t" \ +"mov $0xFEFEFEFE01010101,%0\n\t" \ +"2:\n\t" \ +".previous\n\t" \ +".section __imv,\"a\",@progbits\n\t" \ _ASM_PTR"%c1, (3f)-%c2\n\t" \ -".byte %c2\n\t" \ +".byte %c2, (2b-1b)\n\t" \".previous\n\t" \ +".org . + ((-.-(2b-1b)) & (%c2-1)), 0x90\n\t" \"mov $0xFEFEFEFE01010101,%0\n\t" \"3:\n\t" \ : "=r" (value) \
@@ -74,4 +104,6 @@ value; \ }) +extern int arch_imv_update (const struct __imv *imv,int early); +#endif /* _ASM_X86_IMMEDIATE_H */
arch/x86/kernel/traps_32.c +6 -4
@@ -549,7 +549,7 @@ fastcall void do_##name(struct pt_regs * } DO_VM86_ERROR_INFO( 0, SIGFPE, "divide error", divide_error, FPE_INTDIV, regs->eip) -#ifndef CONFIG_KPROBES +#if !defined(CONFIG_KPROBES ) && !defined(CONFIG_IMMEDIATE ) DO_VM86_ERROR( 3, SIGTRAP, "int3", int3)#endif DO_VM86_ERROR (4 ,SIGSEGV ,"overflow" , overflow)
@@ -791,7 +791,7 @@ void restart_nmi(void)acpi_nmi_enable (); } -#ifdef CONFIG_KPROBES +#if defined(CONFIG_KPROBES ) || defined(CONFIG_IMMEDIATE ) fastcall void __kprobes do_int3(struct pt_regs *regs, long error_code) {trace_hardirqs_fixup ();
@@ -799,8 +799,10 @@ fastcall void __kprobes do_int3(struct pif (notify_die (DIE_INT3 ,"int3" , regs, error_code,3 ,SIGTRAP ) ==NOTIFY_STOP )return ; -/* This is an interrupt gate, because kprobes wants interrupts -disabled. Normal trap handlers don't. */ +/* +* This is an interrupt gate, because kprobes and immediate values wants +* interrupts disabled. Normal trap handlers don't. +*/ restore_interrupts (regs);do_trap (3 ,SIGTRAP ,"int3" ,1 , regs, error_code,NULL ); }
arch/x86/kernel/Makefile_64 +1 -0
@@ -35,6 +35,7 @@ obj-$(CONFIG_X86_PM_TIMER) += pmtimer_64 obj-$(CONFIG_X86_VSMP) += vsmp_64.o obj-$(CONFIG_K8_NB) += k8.o obj-$(CONFIG_AUDIT) += audit_64.o +obj-$(CONFIG_IMMEDIATE) += immediate.o obj-$(CONFIG_MODULES) += module_64.o obj-$(CONFIG_PCI) += early-quirks.o
arch/x86/kernel/Makefile_32 +1 -0
@@ -35,6 +35,7 @@ obj-$(CONFIG_KPROBES) += kprobes_32.o obj-$(CONFIG_MODULES) += module_32.o obj-y += sysenter_32.o vsyscall_32.o obj-$(CONFIG_ACPI_SRAT) += srat_32.o +obj-$(CONFIG_IMMEDIATE) += immediate.o obj-$(CONFIG_EFI) += efi_32.o efi_stub_32.o obj-$(CONFIG_DOUBLEFAULT) += doublefault_32.o obj-$(CONFIG_VM86) += vm86_32.o
arch/x86/kernel/immediate.c new file +277 -0
277 lines not on this page · raw message
Signature · 3 lines
Mathieu Desnoyers Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
Re: [patch 21/24] Immediate Values - x86 Optimization NMI and MCE support (updated)
x86 optimization of the immediate values which uses a movl with code patching to set/unset the value used to populate the register used as variable source. It uses a breakpoint to bypass the instruction being changed, which lessens the interrupt latency of the operation and protects against NMIs and MCE.
- More reentrant immediate value : uses a breakpoint. Needs to know the instruction's first byte. This is why we keep the "instruction size" variable, so we can support the REX prefixed instructions too.
Changelog:
- Use text_poke_early with cr0 WP save/restore to patch the bypass. We are doing non atomic writes to a code region only touched by us (nobody can execute it since we are protected by the imv_mutex).
- Add x86_64 support, ready for i386+x86_64 -> x86 merge.
- Use asm-x86/asm.h.
- Change the immediate.c update code to support variable length opcodes.
- Use imv_* instead of immediate_*.
- Use kernel_wp_disable/enable instead of save/restore.
--- arch/x86/kernel/Makefile_32 | 1 arch/x86/kernel/Makefile_64 | 1 arch/x86/kernel/immediate.c | 281 ++++++++++++++++++++++++++++++++++++++++++++ arch/x86/kernel/traps_32.c | 10 - include/asm-x86/immediate.h | 42 +++++- 5 files changed, 326 insertions(+), 9 deletions(-)
include/asm-x86/immediate.h +37 -5
@@ -12,6 +12,18 @@#include <asm/asm.h> +struct __imv { +unsigned long var;/* Pointer to the identifier variable of the +* immediate value +*/ +unsigned long imv;/* +* Pointer to the memory location of the +* immediate value within the instruction. +*/ +unsigned char size;/* Type size. */ +unsigned char insn_size;/* Instruction size. */ +} __attribute__ ((packed)); +/** * imv_read - read immediate variable * @name: immediate value name
@@ -26,6 +38,11 @@* what will generate an instruction with 8 bytes immediate value (not the REX.W * prefixed one that loads a sign extended 32 bits immediate value in a r64 * register). +* +* Create the instruction in a discarded section to calculate its size. This is +* how we can align the beginning of the instruction on an address that will +* permit atomic modification of the immediate value without knowing the size of +* the opcode used by the compiler. The operand size is known in advance. */ #define imv_read (name) \ ({ \
@@ -35,8 +52,9 @@case 1 : \ asm(".section __imv,\"a\",@progbits\n\t" \ _ASM_PTR"%c1, (3f)-%c2\n\t" \ -".byte %c2\n\t" \ +".byte %c2, (3f-2f)\n\t" \".previous\n\t" \ +"2:\n\t" \"mov $0,%0\n\t" \"3:\n\t" \ : "=q" (value) \
@@ -45,10 +63,16 @@break ; \case 2 : \case 4 : \ - asm(".section __imv,\"a\",@progbits\n\t" \ + asm(".section __discard,\"\",@progbits\n\t" \ +"1:\n\t" \ +"mov $0,%0\n\t" \ +"2:\n\t" \ +".previous\n\t" \ +".section __imv,\"a\",@progbits\n\t" \ _ASM_PTR"%c1, (3f)-%c2\n\t" \ -".byte %c2\n\t" \ +".byte %c2, (2b-1b)\n\t" \".previous\n\t" \ +".org . + ((-.-(2b-1b)) & (%c2-1)), 0x90\n\t" \"mov $0,%0\n\t" \"3:\n\t" \ : "=r" (value) \
@@ -60,10 +84,16 @@ value = name##__imv; \break ; \ } \ - asm(".section __imv,\"a\",@progbits\n\t" \ + asm(".section __discard,\"\",@progbits\n\t" \ +"1:\n\t" \ +"mov $0xFEFEFEFE01010101,%0\n\t" \ +"2:\n\t" \ +".previous\n\t" \ +".section __imv,\"a\",@progbits\n\t" \ _ASM_PTR"%c1, (3f)-%c2\n\t" \ -".byte %c2\n\t" \ +".byte %c2, (2b-1b)\n\t" \".previous\n\t" \ +".org . + ((-.-(2b-1b)) & (%c2-1)), 0x90\n\t" \"mov $0xFEFEFEFE01010101,%0\n\t" \"3:\n\t" \ : "=r" (value) \
@@ -74,4 +104,6 @@ value; \ }) +extern int arch_imv_update (const struct __imv *imv,int early); +#endif /* _ASM_X86_IMMEDIATE_H */
arch/x86/kernel/traps_32.c +6 -4
@@ -550,7 +550,7 @@ fastcall void do_##name(struct pt_regs * } DO_VM86_ERROR_INFO( 0, SIGFPE, "divide error", divide_error, FPE_INTDIV, regs->ip) -#ifndef CONFIG_KPROBES +#if !defined(CONFIG_KPROBES ) && !defined(CONFIG_IMMEDIATE ) DO_VM86_ERROR( 3, SIGTRAP, "int3", int3)#endif DO_VM86_ERROR (4 ,SIGSEGV ,"overflow" , overflow)
@@ -792,7 +792,7 @@ void restart_nmi(void)acpi_nmi_enable (); } -#ifdef CONFIG_KPROBES +#if defined(CONFIG_KPROBES ) || defined(CONFIG_IMMEDIATE ) fastcall void __kprobes do_int3(struct pt_regs *regs, long error_code) {trace_hardirqs_fixup ();
@@ -800,8 +800,10 @@ fastcall void __kprobes do_int3(struct pif (notify_die (DIE_INT3 ,"int3" , regs, error_code,3 ,SIGTRAP ) ==NOTIFY_STOP )return ; -/* This is an interrupt gate, because kprobes wants interrupts -disabled. Normal trap handlers don't. */ +/* +* This is an interrupt gate, because kprobes and immediate values wants +* interrupts disabled. Normal trap handlers don't. +*/ restore_interrupts (regs);do_trap (3 ,SIGTRAP ,"int3" ,1 , regs, error_code,NULL ); }
arch/x86/kernel/Makefile_64 +1 -0
@@ -39,6 +39,7 @@ obj-$(CONFIG_X86_PM_TIMER) += pmtimer_64 obj-$(CONFIG_X86_VSMP) += vsmp_64.o obj-$(CONFIG_K8_NB) += k8.o obj-$(CONFIG_AUDIT) += audit_64.o +obj-$(CONFIG_IMMEDIATE) += immediate.o obj-$(CONFIG_EFI) += efi.o efi_64.o efi_stub_64.o obj-$(CONFIG_MODULES) += module_64.o
arch/x86/kernel/Makefile_32 +1 -0
@@ -38,6 +38,7 @@ obj-$(CONFIG_X86_SUMMIT_NUMA) += summit_ obj-$(CONFIG_KPROBES) += kprobes_32.o obj-$(CONFIG_MODULES) += module_32.o obj-$(CONFIG_ACPI_SRAT) += srat_32.o +obj-$(CONFIG_IMMEDIATE) += immediate.o obj-$(CONFIG_EFI) += efi.o efi_32.o efi_stub_32.o obj-$(CONFIG_DOUBLEFAULT) += doublefault_32.o obj-$(CONFIG_VM86) += vm86_32.o
arch/x86/kernel/immediate.c new file +281 -0
281 lines not on this page · raw message
Signature · 3 lines
Mathieu Desnoyers Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
PATCH 22/24 Immediate Values - Powerpc Optimization NMI MCE support
In reply to Mathieu DesnoyersUse an atomic update for immediate values.
--- arch/powerpc/kernel/Makefile | 1 arch/powerpc/kernel/immediate.c | 73 ++++++++++++++++++++++++++++++++++++++++ include/asm-powerpc/immediate.h | 18 +++++++++ 3 files changed, 92 insertions(+)
arch/powerpc/kernel/immediate.c new file +73 -0
@@ -0,0 +1,73 @@ +/* +* Powerpc optimized immediate values enabling/disabling. +* +* Mathieu Desnoyers <[email protected]> +*/ + +#include <linux/module.h> +#include <linux/immediate.h> +#include <linux/string.h> +#include <linux/kprobes.h> +#include <asm/cacheflush.h> +#include <asm/page.h> + +#define LI_OPCODE_LEN 2 + +/** +* arch_imv_update - update one immediate value +* @imv: pointer of type const struct __imv to update +* @early: early boot (1), normal (0) +* +* Update one immediate value. Must be called with imv_mutex held. +*/ +int arch_imv_update (const struct __imv *imv,int early) +{ +#ifdef CONFIG_KPROBES +kprobe_opcode_t *insn; +/* +* Fail if a kprobe has been set on this instruction. +* (TODO: we could eventually do better and modify all the (possibly +* nested) kprobes for this site if kprobes had an API for this. +*/ +switch (imv->size) { +case 1 :/* The uint8_t points to the 3rd byte of the +* instruction */ + insn = (void *)(imv->imv -1 -LI_OPCODE_LEN ); +break ; +case 2 : insn = (void *)(imv->imv -LI_OPCODE_LEN ); +break ; +default : +return -EINVAL ; + } + +if (unlikely (!early && *insn ==BREAKPOINT_INSTRUCTION )) { +printk (KERN_WARNING "Immediate value in conflict with kprobe. " +"Variable at %p, " +"instruction at %p, size %lu\n" , + (void *)imv->imv, + (void *)imv->var, imv->size); +return -EBUSY ; + } +#endif + +/* +* If the variable and the instruction have the same value, there is +* nothing to do. +*/ +switch (imv->size) { +case 1 :if (*(uint8_t *)imv->imv + == *(uint8_t *)imv->var) +return 0 ; +break ; +case 2 :if (*(uint16_t *)imv->imv + == *(uint16_t *)imv->var) +return 0 ; +break ; +default :return -EINVAL ; + } +memcpy ((void *)imv->imv, (void *)imv->var, + imv->size); +flush_icache_range (imv->imv, + imv->imv + imv->size); +return 0 ; +}
include/asm-powerpc/immediate.h +18 -0
@@ -12,6 +12,16 @@#include <asm/asm-compat.h> +struct __imv { +unsigned long var;/* Identifier variable of the immediate value */ +unsigned long imv;/* +* Pointer to the memory location that holds +* the immediate value within the load immediate +* instruction. +*/ +unsigned char size;/* Type size. */ +} __attribute__ ((packed)); +/** * imv_read - read immediate variable * @name: immediate value name
@@ -19,6 +29,11 @@* Reads the value of @name. * Optimized version of the immediate. * Do not use in __init and __exit functions. Use _imv_read() instead. +* Makes sure the 2 bytes update will be atomic by aligning the immediate +* value. Use a normal memory read for the 4 bytes immediate because there is no +* way to atomically update it without using a seqlock read side, which would +* cost more in term of total i-cache and d-cache space than a simple memory +* read. */ #define imv_read (name) \ ({ \
@@ -40,6 +55,7 @@PPC_LONG "%c1, ((1f)-2)\n\t" \".byte 2\n\t" \".previous\n\t" \ +".align 2\n\t" \"li %0,0\n\t" \"1:\n\t" \ : "=r" (value) \
@@ -52,4 +68,6 @@ value; \ }) +extern int arch_imv_update (const struct __imv *imv,int early); +#endif /* _ASM_POWERPC_IMMEDIATE_H */
arch/powerpc/kernel/Makefile +1 -0
@@ -91,3 +91,4 @@ obj-$(CONFIG_PPC64) += $(obj64-y) extra-$(CONFIG_PPC_FPU) += fpu.o extra-$(CONFIG_PPC64) += entry_64.o +obj-$(CONFIG_IMMEDIATE) += immediate.o
Signature · 3 lines
Mathieu Desnoyers Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
PATCH 23/24 Immediate Values Use Arch NMI and MCE Support
In reply to Mathieu DesnoyersRemove the architecture agnostic code now replaced by architecture specific, atomic instruction updates.
--- include/linux/immediate.h | 11 ---- kernel/immediate.c | 113 +--------------------------------------------- 2 files changed, 4 insertions(+), 120 deletions(-)
kernel/immediate.c +4 -109
@@ -19,9 +19,6 @@#include <linux/mutex.h> #include <linux/immediate.h> #include <linux/memory.h> -#include <linux/cpu.h> - -#include <asm/cacheflush.h> /* * Kernel ready to execute the SMP update that may depend on trap and ipi.
@@ -37,111 +34,6 @@ extern const struct __imv __stop___imv[]*/ static DEFINE_MUTEX (imv_mutex); -static atomic_t wait_sync; - -struct ipi_loop_data { -long value; -const struct __imv *imv; -} loop_data; - -static void ipi_busy_loop (void *arg) -{ -unsigned long flags; - -local_irq_save (flags); -atomic_dec (&wait_sync); -do { -/* Make sure the wait_sync gets re-read */ -smp_mb (); - }while (atomic_read (&wait_sync) > loop_data.value); -atomic_dec (&wait_sync); -do { -/* Make sure the wait_sync gets re-read */ -smp_mb (); - }while (atomic_read (&wait_sync) >0 ); -/* -* Issuing a synchronizing instruction must be done on each CPU before -* reenabling interrupts after modifying an instruction. Required by -* Intel's errata. -*/ -sync_core (); -flush_icache_range (loop_data.imv->imv, - loop_data.imv->imv + loop_data.imv->size); -local_irq_restore (flags); -} - -/** -* apply_imv_update - update one immediate value -* @imv: pointer of type const struct __imv to update -* -* Update one immediate value. Must be called with imv_mutex held. -* It makes sure all CPUs are not executing the modified code by having them -* busy looping with interrupts disabled. -* It does _not_ protect against NMI and MCE (could be a problem with Intel's -* errata if we use immediate values in their code path). -*/ -static int apply_imv_update (const struct __imv *imv) -{ -unsigned long flags; -long online_cpus; - -/* -* If the variable and the instruction have the same value, there is -* nothing to do. -*/ -switch (imv->size) { -case 1 :if (*(uint8_t *)imv->imv - == *(uint8_t *)imv->var) -return 0 ; -break ; -case 2 :if (*(uint16_t *)imv->imv - == *(uint16_t *)imv->var) -return 0 ; -break ; -case 4 :if (*(uint32_t *)imv->imv - == *(uint32_t *)imv->var) -return 0 ; -break ; -case 8 :if (*(uint64_t *)imv->imv - == *(uint64_t *)imv->var) -return 0 ; -break ; -default :return -EINVAL ; - } - -if (imv_early_boot_complete) { -kernel_text_lock (); -lock_cpu_hotplug (); - online_cpus =num_online_cpus (); -atomic_set (&wait_sync,2 * online_cpus); - loop_data.value = online_cpus; - loop_data.imv = imv; -smp_call_function (ipi_busy_loop,NULL ,1 ,0 ); -local_irq_save (flags); -atomic_dec (&wait_sync); -do { -/* Make sure the wait_sync gets re-read */ -smp_mb (); - }while (atomic_read (&wait_sync) > online_cpus); -text_poke ((void *)imv->imv, (void *)imv->var, - imv->size); -/* -* Make sure the modified instruction is seen by all CPUs before -* we continue (visible to other CPUs and local interrupts). -*/ -wmb (); -atomic_dec (&wait_sync); -flush_icache_range (imv->imv, - imv->imv + imv->size); -local_irq_restore (flags); -unlock_cpu_hotplug (); -kernel_text_unlock (); - }else -text_poke_early ((void *)imv->imv, (void *)imv->var, - imv->size); -return 0 ; -} -/** * imv_update_range - Update immediate values in a range * @begin: pointer to the beginning of the range
@@ -154,9 +46,12 @@ void imv_update_range(const struct __imv {const struct __imv *iter;int ret; +for (iter = begin; iter < end; iter++) {mutex_lock (&imv_mutex); - ret =apply_imv_update (iter); +kernel_text_lock (); + ret =arch_imv_update (iter, !imv_early_boot_complete); +kernel_text_unlock ();if (imv_early_boot_complete && ret)printk (KERN_WARNING "Invalid immediate value. "
include/linux/immediate.h +0 -11
@@ -12,17 +12,6 @@#ifdef CONFIG_IMMEDIATE -struct __imv { -unsigned long var;/* Pointer to the identifier variable of the -* immediate value -*/ -unsigned long imv;/* -* Pointer to the memory location of the -* immediate value within the instruction. -*/ -unsigned char size;/* Type size. */ -} __attribute__ ((packed)); -#include <asm/immediate.h> /**
Signature · 3 lines
Mathieu Desnoyers Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
PATCH 24/24 Linux Kernel Markers - Use Immediate Values
In reply to Mathieu DesnoyersMake markers use immediate values.
Changelog :
- Use imv_* instead of immediate_*.
--- Documentation/markers.txt | 17 +++++++++++++---- include/linux/marker.h | 42 ++++++++++++++++++++++++++++++++---------- kernel/marker.c | 8 ++++++-- kernel/module.c | 1 + 4 files changed, 52 insertions(+), 16 deletions(-)
include/linux/marker.h +32 -10
@@ -12,6 +12,7 @@* See the file COPYING for more details. */ +#include <linux/immediate.h> #include <linux/types.h> struct module ;
@@ -42,7 +43,7 @@ struct marker {const char *format;/* Marker format string, describing the * variable argument list. */ -char state;/* Marker state. */ +DEFINE_IMV (char, state);/* Immediate value state. */ char ptype;/* probe type : 0 : single, 1 : multi */ void (*call)(const struct marker *mdata,/* Probe wrapper */ void *call_private,const char *fmt, ...);
@@ -53,13 +54,14 @@ struct marker {#ifdef CONFIG_MARKERS /* +* Generic marker flavor always available. * Note : the empty asm volatile with read constraint is used here instead of a * "used" attribute to fix a gcc 4.1.x bug. * Make sure the alignment of the structure in the __markers section will * not add unwanted padding between the beginning of the section and the * structure. Force alignment to the same alignment as the section start. */ -#define __trace_mark(name, call_private, format, args...) \ +#define __trace_mark(generic, name, call_private, format, args...) \ do { \ static const char __mstrtab_##name[] \ __attribute__((section("__markers_strings"))) \
@@ -70,17 +72,23 @@ struct marker {0 ,0 , marker_probe_cb, \ { __mark_empty_function, NULL}, NULL }; \ __mark_check_format(format, ## args); \ - if (unlikely(__mark_##name.state)) { \ - (*__mark_##name.call) \ - (&__mark_##name, call_private, \ - format, ## args); \ + if (!generic) { \ + if (unlikely(imv_read(__mark_##name.state))) \ + (*__mark_##name.call) \ + (&__mark_##name, call_private, \ + format, ## args); \ + } else { \ + if (unlikely(_imv_read(__mark_##name.state))) \ + (*__mark_##name.call) \ + (&__mark_##name, call_private, \ + format, ## args); \ } \ } while (0)extern void marker_update_probe_range (struct marker *begin,struct marker *end);#else /* !CONFIG_MARKERS */ -#define __trace_mark(name, call_private, format, args...) \ +#define __trace_mark(generic, name, call_private, format, args...) \ __mark_check_format(format, ## args)static inline void marker_update_probe_range (struct marker *begin,struct marker *end)
@@ -88,15 +96,29 @@ static inline void marker_update_probe_r#endif /* CONFIG_MARKERS */ /** -* trace_mark - Marker +* trace_mark - Marker using code patching * @name: marker name, not quoted. * @format: format string * @args...: variable argument list * -* Places a marker. +* Places a marker using optimized code patching technique (imv_read()) +* to be enabled. */ #define trace_mark(name, format, args...) \ - __trace_mark(name, NULL, format, ## args) + __trace_mark(0, name, NULL, format, ## args) + +/** +* _trace_mark - Marker using variable read +* @name: marker name, not quoted. +* @format: format string +* @args...: variable argument list +* +* Places a marker using a standard memory read (_imv_read()) to be +* enabled. Should be used for markers in __init and __exit functions and in +* lockdep code. +*/ +#define _trace_mark(name, format, args...) \ + __trace_mark(1, name, NULL, format, ## args)/** * MARK_NOARGS - Format string for a marker with no argument.
kernel/marker.c +6 -2
@@ -23,6 +23,7 @@#include <linux/rcupdate.h> #include <linux/marker.h> #include <linux/err.h> +#include <linux/immediate.h> extern struct marker __start___markers[];extern struct marker __stop___markers[];
@@ -544,7 +545,7 @@ static int set_marker(struct marker_entr*/ smp_wmb (); elem->ptype = (*entry)->ptype; - elem->state = active; + elem->state__imv = active;return 0 ; }
@@ -558,7 +559,7 @@ static int set_marker(struct marker_entrstatic void disable_marker (struct marker *elem) {/* leave "call" as is. It is known statically. */ - elem->state =0 ; + elem->state__imv =0 ; elem->single.func = __mark_empty_function;/* Update the function before setting the ptype */ smp_wmb ();
@@ -625,6 +626,9 @@ static void marker_update_probes(void)marker_update_probe_range (__start___markers, __stop___markers);/* Markers in modules. */ module_update_markers (); +/* Update immediate values */ +core_imv_update (); +module_imv_update (); }/**
Documentation/markers.txt +13 -4
@@ -15,10 +15,12 @@ provide at runtime. A marker can be "on" (no probe is attached). When a marker is "off" it has no effect, except for adding a tiny time penalty (checking a condition for a branch) and space penalty (adding a few bytes for the function call at the end of the -instrumented function and adds a data structure in a separate section). When a -marker is "on", the function you provide is called each time the marker is -executed, in the execution context of the caller. When the function provided -ends its execution, it returns to the caller (continuing from the marker site). +instrumented function and adds a data structure in a separate section). The +immediate values are used to minimize the impact on data cache, encoding the +condition in the instruction stream. When a marker is "on", the function you +provide is called each time the marker is executed, in the execution context of +the caller. When the function provided ends its execution, it returns to the +caller (continuing from the marker site). You can put markers at important locations in the code. Markers are lightweight hooks that can pass an arbitrary number of parameters,
@@ -69,6 +71,13 @@ a printk warning which identifies the in "Format mismatch for probe probe_name (format), marker (format)" +* Optimization for a given architecture + +To force use of a non-optimized version of the markers, _trace_mark() should be +used. It takes the same parameters as the normal markers, but it does not use +the immediate values based on code patching. + + * Probe / marker example See the example provided in samples/markers/src
kernel/module.c +1 -0
@@ -2005,6 +2005,7 @@ static struct module *load_module(void _ mod->markers + mod->num_markers);#endif #ifdef CONFIG_IMMEDIATE +/* Immediate values must be updated after markers */ imv_update_range (mod->immediate, mod->immediate + mod->num_immediate);#endif
Signature · 3 lines
Mathieu Desnoyers Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68