[patch 0/8] Text Edit Lock

[patch 0/8] Text Edit Lock

[Mathieu Desnoyers]

Hi Andrew,

After refactoring of architecture independant code (mutex to protect text
edition) vs architecture dependant code (marking/unmarking pages RW) and
thorough testing, here is the new version of the text edit lock.

Mathieu

-- 
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/8] Kprobes - do not use kprobes mutex in arch code

[Mathieu Desnoyers]

[-- Attachment #1: kprobes-dont-use-kprobes-mutex-in-arch-code.patch --]
[-- Type: text/plain, Size: 4846 bytes --]

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

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
CC: prasanna@in.ibm.com
CC: ananth@in.ibm.com
CC: anil.s.keshavamurthy@intel.com
CC: davem@davemloft.net
---
 arch/i386/kernel/kprobes.c    |    2 --
 arch/ia64/kernel/kprobes.c    |    2 --
 arch/powerpc/kernel/kprobes.c |    2 --
 arch/s390/kernel/kprobes.c    |    2 --
 arch/x86_64/kernel/kprobes.c  |    2 --
 include/linux/kprobes.h       |    2 --
 kernel/kprobes.c              |    2 ++
 7 files changed, 2 insertions(+), 12 deletions(-)

Index: linux-2.6-lttng/include/linux/kprobes.h
===================================================================
--- linux-2.6-lttng.orig/include/linux/kprobes.h	2007-07-13 18:42:08.000000000 -0400
+++ linux-2.6-lttng/include/linux/kprobes.h	2007-07-13 18:42:52.000000000 -0400
@@ -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>
@@ -177,7 +176,6 @@
 }
 
 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);
Index: linux-2.6-lttng/arch/i386/kernel/kprobes.c
===================================================================
--- linux-2.6-lttng.orig/arch/i386/kernel/kprobes.c	2007-07-13 18:42:08.000000000 -0400
+++ linux-2.6-lttng/arch/i386/kernel/kprobes.c	2007-07-13 18:42:52.000000000 -0400
@@ -209,9 +209,7 @@
 
 void __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)
Index: linux-2.6-lttng/kernel/kprobes.c
===================================================================
--- linux-2.6-lttng.orig/kernel/kprobes.c	2007-07-13 18:42:10.000000000 -0400
+++ linux-2.6-lttng/kernel/kprobes.c	2007-07-13 18:43:17.000000000 -0400
@@ -641,7 +641,9 @@
 			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)
Index: linux-2.6-lttng/arch/ia64/kernel/kprobes.c
===================================================================
--- linux-2.6-lttng.orig/arch/ia64/kernel/kprobes.c	2007-07-13 18:43:59.000000000 -0400
+++ linux-2.6-lttng/arch/ia64/kernel/kprobes.c	2007-07-13 18:44:03.000000000 -0400
@@ -565,9 +565,7 @@
 
 void __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
Index: linux-2.6-lttng/arch/powerpc/kernel/kprobes.c
===================================================================
--- linux-2.6-lttng.orig/arch/powerpc/kernel/kprobes.c	2007-07-13 18:44:20.000000000 -0400
+++ linux-2.6-lttng/arch/powerpc/kernel/kprobes.c	2007-07-13 18:44:26.000000000 -0400
@@ -86,9 +86,7 @@
 
 void __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)
Index: linux-2.6-lttng/arch/s390/kernel/kprobes.c
===================================================================
--- linux-2.6-lttng.orig/arch/s390/kernel/kprobes.c	2007-07-13 18:44:43.000000000 -0400
+++ linux-2.6-lttng/arch/s390/kernel/kprobes.c	2007-07-13 18:44:46.000000000 -0400
@@ -218,9 +218,7 @@
 
 void __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)
Index: linux-2.6-lttng/arch/x86_64/kernel/kprobes.c
===================================================================
--- linux-2.6-lttng.orig/arch/x86_64/kernel/kprobes.c	2007-07-13 18:45:08.000000000 -0400
+++ linux-2.6-lttng/arch/x86_64/kernel/kprobes.c	2007-07-13 18:45:14.000000000 -0400
@@ -249,9 +249,7 @@
 
 void __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)

-- 
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/8] Text Edit Lock - Architecture Independent Code

[Mathieu Desnoyers]

[-- Attachment #1: text-edit-lock-architecture-independent-code.patch --]
[-- Type: text/plain, Size: 2547 bytes --]

This 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 users of this interface: paravirt, alternatives and immediate values.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
---
 include/linux/memory.h |    7 +++++++
 mm/memory.c            |   24 ++++++++++++++++++++++++
 2 files changed, 31 insertions(+)

Index: linux-2.6-lttng/include/linux/memory.h
===================================================================
--- linux-2.6-lttng.orig/include/linux/memory.h	2007-07-13 17:01:24.000000000 -0400
+++ linux-2.6-lttng/include/linux/memory.h	2007-07-13 17:14:38.000000000 -0400
@@ -86,4 +86,11 @@
 	register_memory_notifier(&fn##_mem_nb);			\
 }
 
+/*
+ * 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_ */
Index: linux-2.6-lttng/mm/memory.c
===================================================================
--- linux-2.6-lttng.orig/mm/memory.c	2007-07-13 17:01:24.000000000 -0400
+++ linux-2.6-lttng/mm/memory.c	2007-07-13 17:01:26.000000000 -0400
@@ -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 @@
 
 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;
@@ -2811,3 +2819,19 @@
 	return buf - old_buf;
 }
 EXPORT_SYMBOL_GPL(access_process_vm);
+
+/*
+ * Lock the kernel text for mutual write exclusion. Used for dynamic code
+ * patching.
+ */
+void __kprobes kernel_text_lock(void)
+{
+	mutex_lock(&text_mutex);
+}
+EXPORT_SYMBOL_GPL(kernel_text_lock);
+
+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 Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68

[patch 3/8] Text Edit Lock - i386

[Mathieu Desnoyers]

[-- Attachment #1: text-edit-lock-i386.patch --]
[-- Type: text/plain, Size: 4052 bytes --]

Interface to use for code patching : makes sure the page is writable
between calls to kernel_text_mark_rw() and kernel_text_unmark().

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
---
 arch/i386/mm/init.c           |   60 ++++++++++++++++++++++++++++++++----------
 include/asm-i386/cacheflush.h |    9 ++++++
 2 files changed, 56 insertions(+), 13 deletions(-)

Index: linux-2.6-lttng/arch/i386/mm/init.c
===================================================================
--- linux-2.6-lttng.orig/arch/i386/mm/init.c	2007-07-13 17:01:24.000000000 -0400
+++ linux-2.6-lttng/arch/i386/mm/init.c	2007-07-13 17:02:31.000000000 -0400
@@ -31,6 +31,7 @@
 #include <linux/memory_hotplug.h>
 #include <linux/initrd.h>
 #include <linux/cpumask.h>
+#include <linux/kprobes.h>
 
 #include <asm/processor.h>
 #include <asm/system.h>
@@ -53,6 +54,10 @@
 
 static int noinline do_test_wp_bit(void);
 
+#ifdef CONFIG_DEBUG_RODATA
+static int rodata_marked;
+#endif
+
 /*
  * Creates a middle page table and puts a pointer to it in the
  * given global directory entry. This only returns the gd entry
@@ -796,24 +801,16 @@
 }
 
 #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);
-		kernel_text_is_ro = 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);
+	kernel_text_is_ro = 1;
+
 	start += size;
 	size = (unsigned long)__end_rodata - start;
 	change_page_attr(virt_to_page(start),
@@ -828,9 +825,46 @@
 	 * of who is the culprit.
 	 */
 	global_flush_tlb();
+	rodata_marked = 1;
+}
+
+/*
+ * Lock the kernel text for mutual write exclusion.
+ * Make sure the pages are writable.
+ */
+void __kprobes kernel_text_mark_rw(unsigned long address, size_t len)
+{
+	if (rodata_marked && address >= PFN_ALIGN(_text)
+			&& (address + len) <= PFN_ALIGN(_etext)) {
+		unsigned long nr_pages;
+		nr_pages = ((address + len) >> PAGE_SHIFT)
+				- (address >> PAGE_SHIFT) + 1;
+		change_page_attr(virt_to_page(address), nr_pages,
+				PAGE_KERNEL_EXEC);
+		mb();
+		global_flush_tlb();
+		mb();
+	}
 }
+EXPORT_SYMBOL_GPL(kernel_text_mark_rw);
 #endif
 
+void __kprobes kernel_text_unmark(unsigned long address, size_t len)
+{
+	if (rodata_marked && address >= PFN_ALIGN(_text)
+			&& (address + len) <= PFN_ALIGN(_etext)) {
+		unsigned long nr_pages;
+		nr_pages = ((address + len) >> PAGE_SHIFT)
+				- (address >> PAGE_SHIFT) + 1;
+		change_page_attr(virt_to_page(address), nr_pages,
+				PAGE_KERNEL_RX);
+		mb();
+		global_flush_tlb();
+		mb();
+	}
+}
+EXPORT_SYMBOL_GPL(kernel_text_unmark);
+
 void free_init_pages(char *what, unsigned long begin, unsigned long end)
 {
 	unsigned long addr;
Index: linux-2.6-lttng/include/asm-i386/cacheflush.h
===================================================================
--- linux-2.6-lttng.orig/include/asm-i386/cacheflush.h	2007-07-13 17:01:24.000000000 -0400
+++ linux-2.6-lttng/include/asm-i386/cacheflush.h	2007-07-13 17:01:26.000000000 -0400
@@ -36,4 +36,13 @@
 void mark_rodata_ro(void);
 #endif
 
+#if defined(CONFIG_DEBUG_RODATA)
+/* mark kernel text pages writable */
+extern void kernel_text_mark_rw(unsigned long address, size_t len);
+extern void kernel_text_unmark(unsigned long address, size_t len);
+#else
+void kernel_text_mark_rw(unsigned long address, size_t len) { }
+void kernel_text_unmark(unsigned long address, size_t len) { }
+#endif
+
 #endif /* _I386_CACHEFLUSH_H */

-- 
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/8] Text Edit Lock - x86_64

[Mathieu Desnoyers]

[-- Attachment #1: text-edit-lock-x86_64.patch --]
[-- Type: text/plain, Size: 3888 bytes --]

Interface to use for code patching : makes sure the page is writable
between calls to kernel_text_mark_rw() and kernel_text_unmark().

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
CC: Andi Kleen <andi@firstfloor.org>
---
 arch/x86_64/mm/init.c           |   64 +++++++++++++++++++++++++++-------------
 include/asm-x86_64/cacheflush.h |    9 +++++
 2 files changed, 53 insertions(+), 20 deletions(-)

Index: linux-2.6-lttng/arch/x86_64/mm/init.c
===================================================================
--- linux-2.6-lttng.orig/arch/x86_64/mm/init.c	2007-07-13 16:08:14.000000000 -0400
+++ linux-2.6-lttng/arch/x86_64/mm/init.c	2007-07-13 16:36:40.000000000 -0400
@@ -56,6 +56,10 @@
 
 DEFINE_PER_CPU(struct mmu_gather, mmu_gathers);
 
+#ifdef CONFIG_DEBUG_RODATA
+static int rodata_marked;
+#endif
+
 /*
  * NOTE: pagetable_init alloc all the fixmap pagetables contiguous on the
  * physical space so we can cache the place of the first one and move
@@ -596,31 +600,15 @@
 }
 
 #ifdef CONFIG_DEBUG_RODATA
-
 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);
 
 	kernel_text_is_ro = 1;
-#ifdef CONFIG_HOTPLUG_CPU
-	/* It must still be possible to apply SMP alternatives. */
-	if (num_possible_cpus() > 1) {
-		start = (unsigned long)_etext;
-		kernel_text_is_ro = 0;
-	}
-#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);
@@ -632,7 +620,43 @@
 	 * of who is the culprit.
 	 */
 	global_flush_tlb();
+	rodata_marked = 1;
+}
+
+/*
+ * Mark kernel text pages writable.
+ */
+void kernel_text_mark_rw(unsigned long address, size_t len)
+{
+	if (rodata_marked && address >= PFN_ALIGN(_text)
+			&& (address + len) <= PFN_ALIGN(_etext)) {
+		unsigned long nr_pages;
+		nr_pages = ((address + len) >> PAGE_SHIFT)
+				- (address >> PAGE_SHIFT) + 1;
+		change_page_attr(virt_to_page(address), nr_pages,
+				PAGE_KERNEL_EXEC);
+		mb();
+		global_flush_tlb();
+		mb();
+	}
+}
+EXPORT_SYMBOL_GPL(kernel_text_mark_rw);
+
+void kernel_text_unmark(unsigned long address, size_t len)
+{
+	if (rodata_marked && address >= PFN_ALIGN(_text)
+			&& (address + len) <= PFN_ALIGN(_etext)) {
+		unsigned long nr_pages;
+		nr_pages = ((address + len) >> PAGE_SHIFT)
+				- (address >> PAGE_SHIFT) + 1;
+		mb();
+		change_page_attr(virt_to_page(address), nr_pages,
+				PAGE_KERNEL_RO);
+		mb();
+		global_flush_tlb();
+	}
 }
+EXPORT_SYMBOL_GPL(kernel_text_unmark);
 #endif
 
 #ifdef CONFIG_BLK_DEV_INITRD
Index: linux-2.6-lttng/include/asm-x86_64/cacheflush.h
===================================================================
--- linux-2.6-lttng.orig/include/asm-x86_64/cacheflush.h	2007-07-13 16:08:14.000000000 -0400
+++ linux-2.6-lttng/include/asm-x86_64/cacheflush.h	2007-07-13 16:34:32.000000000 -0400
@@ -33,4 +33,13 @@
 void mark_rodata_ro(void);
 #endif
 
+#if defined(CONFIG_DEBUG_RODATA)
+/* mark kernel text pages writable */
+extern void kernel_text_mark_rw(unsigned long address, size_t len);
+extern void kernel_text_unmark(unsigned long address, size_t len);
+#else
+void kernel_text_mark_rw(unsigned long address, size_t len) { }
+void kernel_text_unmark(unsigned long address, size_t len) { }
+#endif
+
 #endif /* _X8664_CACHEFLUSH_H */

-- 
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/8] Text Edit Lock - Alternative code for i386 and x86_64

[Mathieu Desnoyers]

[-- Attachment #1: text-edit-lock-alternative-i386.patch --]
[-- Type: text/plain, Size: 1859 bytes --]

alternative.c can use the text edit lock to remove CPU HOTPLUG special cases.
Also used by x86_64.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
---
 arch/i386/kernel/alternative.c |   10 ++++++++++
 1 file changed, 10 insertions(+)

Index: linux-2.6-lttng/arch/i386/kernel/alternative.c
===================================================================
--- linux-2.6-lttng.orig/arch/i386/kernel/alternative.c	2007-07-13 16:08:14.000000000 -0400
+++ linux-2.6-lttng/arch/i386/kernel/alternative.c	2007-07-13 16:39:26.000000000 -0400
@@ -2,8 +2,10 @@
 #include <linux/sched.h>
 #include <linux/spinlock.h>
 #include <linux/list.h>
+#include <linux/memory.h>
 #include <asm/alternative.h>
 #include <asm/sections.h>
+#include <asm/cacheflush.h>
 
 #ifdef CONFIG_HOTPLUG_CPU
 static int smp_alt_once;
@@ -185,9 +187,13 @@
 				__FUNCTION__, a->instr, instr);
 		}
 #endif
+		kernel_text_lock();
+		kernel_text_mark_rw((unsigned long)instr, a->instrlen);
 		memcpy(instr, a->replacement, a->replacementlen);
 		diff = a->instrlen - a->replacementlen;
 		nop_out(instr + a->replacementlen, diff);
+		kernel_text_unmark((unsigned long)instr, a->instrlen);
+		kernel_text_unlock();
 	}
 }
 
@@ -352,6 +358,8 @@
 	for (p = start; p < end; p++) {
 		unsigned int used;
 
+		kernel_text_lock();
+		kernel_text_mark_rw((unsigned long)p->instr, p->len);
 		used = paravirt_ops.patch(p->instrtype, p->clobbers, p->instr,
 					  p->len);
 
@@ -359,6 +367,8 @@
 
 		/* Pad the rest with nops */
 		nop_out(p->instr + used, p->len - used);
+		kernel_text_unmark((unsigned long)p->instr, p->len);
+		kernel_text_unlock();
 	}
 
 	/* Sync to be conservative, in case we patched following

-- 
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/8] Text Edit Lock - Kprobes architecture independent support

[Mathieu Desnoyers]

[-- Attachment #1: text-edit-lock-kprobes-architecture-independent.patch --]
[-- Type: text/plain, Size: 2850 bytes --]

Use the mutual exclusion provided by the text edit lock in the kprobes code. It
allows coherent manipulation of the kernel code by other subsystems.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
CC: prasanna@in.ibm.com
CC: ananth@in.ibm.com
CC: anil.s.keshavamurthy@intel.com
CC: davem@davemloft.net
---
 kernel/kprobes.c |   20 +++++++++++++++++---
 1 file changed, 17 insertions(+), 3 deletions(-)

Index: linux-2.6-lttng/kernel/kprobes.c
===================================================================
--- linux-2.6-lttng.orig/kernel/kprobes.c	2007-07-13 18:47:57.000000000 -0400
+++ linux-2.6-lttng/kernel/kprobes.c	2007-07-13 18:48:32.000000000 -0400
@@ -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>
@@ -561,9 +562,10 @@
 		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,
@@ -576,6 +578,8 @@
 
 		arch_arm_kprobe(p);
 	}
+out_unlock_text:
+	kernel_text_unlock();
 out:
 	mutex_unlock(&kprobe_mutex);
 
@@ -618,8 +622,11 @@
 		 * 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 {
@@ -642,7 +649,9 @@
 			kfree(old_p);
 		}
 		mutex_lock(&kprobe_mutex);
+		kernel_text_lock();
 		arch_remove_kprobe(p);
+		kernel_text_unlock();
 		mutex_unlock(&kprobe_mutex);
 	} else {
 		mutex_lock(&kprobe_mutex);
@@ -723,8 +732,9 @@
 				 struct kretprobe_instance, uflist);
 		ri->rp = rp;
 		ri->task = current;
+		kernel_text_lock();
 		arch_prepare_kretprobe(ri, regs);
-
+		kernel_text_unlock();
 		/* XXX(hch): why is there no hlist_move_head? */
 		hlist_del(&ri->uflist);
 		hlist_add_head(&ri->uflist, &ri->rp->used_instances);
@@ -932,8 +942,10 @@
 
 	for (i = 0; i < KPROBE_TABLE_SIZE; i++) {
 		head = &kprobe_table[i];
+		kernel_text_lock();
 		hlist_for_each_entry_rcu(p, node, head, hlist)
 			arch_arm_kprobe(p);
+		kernel_text_unlock();
 	}
 
 	kprobe_enabled = true;
@@ -961,10 +973,12 @@
 	printk(KERN_INFO "Kprobes globally disabled\n");
 	for (i = 0; i < KPROBE_TABLE_SIZE; i++) {
 		head = &kprobe_table[i];
+		kernel_text_lock();
 		hlist_for_each_entry_rcu(p, node, head, hlist) {
 			if (!arch_trampoline_kprobe(p))
 				arch_disarm_kprobe(p);
 		}
+		kernel_text_unlock();
 	}
 
 	mutex_unlock(&kprobe_mutex);

-- 
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/8] Text Edit Lock - kprobes i386

[Mathieu Desnoyers]

[-- Attachment #1: text-edit-lock-kprobes-i386.patch --]
[-- Type: text/plain, Size: 2255 bytes --]

Kprobes can use the text edit lock to insure mutual exclusion when editing the
code.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
CC: prasanna@in.ibm.com
CC: ananth@in.ibm.com
CC: anil.s.keshavamurthy@intel.com
CC: davem@davemloft.net
---
 arch/i386/kernel/kprobes.c |   30 ++++--------------------------
 1 file changed, 4 insertions(+), 26 deletions(-)

Index: linux-2.6-lttng/arch/i386/kernel/kprobes.c
===================================================================
--- linux-2.6-lttng.orig/arch/i386/kernel/kprobes.c	2007-07-13 16:44:37.000000000 -0400
+++ linux-2.6-lttng/arch/i386/kernel/kprobes.c	2007-07-13 16:45:03.000000000 -0400
@@ -169,42 +169,20 @@
 
 void __kprobes arch_arm_kprobe(struct kprobe *p)
 {
-	unsigned long addr = (unsigned long) p->addr;
-	int page_readonly = 0;
-
-	if (kernel_readonly_text(addr)) {
-		page_readonly = 1;
-		change_page_attr(virt_to_page(addr), 1, PAGE_KERNEL_RWX);
-		global_flush_tlb();
-	}
-
+	kernel_text_mark_rw((unsigned long) p->addr, sizeof(kprobe_opcode_t));
 	*p->addr = BREAKPOINT_INSTRUCTION;
+	kernel_text_unmark((unsigned long) p->addr, sizeof(kprobe_opcode_t));
 	flush_icache_range((unsigned long) p->addr,
 			   (unsigned long) p->addr + sizeof(kprobe_opcode_t));
-
-	if (page_readonly) {
-		change_page_attr(virt_to_page(addr), 1, PAGE_KERNEL_RX);
-		global_flush_tlb();
-	}
 }
 
 void __kprobes arch_disarm_kprobe(struct kprobe *p)
 {
-	unsigned long addr = (unsigned long) p->addr;
-	int page_readonly = 0;
-
-	if (kernel_readonly_text(addr)) {
-		page_readonly = 1;
-		change_page_attr(virt_to_page(addr), 1, PAGE_KERNEL_RWX);
-		global_flush_tlb();
-	}
+	kernel_text_mark_rw((unsigned long) p->addr, sizeof(kprobe_opcode_t));
 	*p->addr = p->opcode;
+	kernel_text_unmark((unsigned long) p->addr, sizeof(kprobe_opcode_t));
 	flush_icache_range((unsigned long) p->addr,
 			   (unsigned long) p->addr + sizeof(kprobe_opcode_t));
-	if (page_readonly) {
-		change_page_attr(virt_to_page(addr), 1, PAGE_KERNEL_RX);
-		global_flush_tlb();
-	}
 }
 
 void __kprobes arch_remove_kprobe(struct kprobe *p)

-- 
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/8] Text Edit Lock - kprobes x86_64

[Mathieu Desnoyers]

[-- Attachment #1: text-edit-lock-kprobes-x86_64.patch --]
[-- Type: text/plain, Size: 2227 bytes --]

Kprobes can use the text edit lock to insure mutual exclusion when editing the
code.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
CC: prasanna@in.ibm.com
CC: ananth@in.ibm.com
CC: anil.s.keshavamurthy@intel.com
CC: davem@davemloft.net
---
 arch/x86_64/kernel/kprobes.c |   30 ++++--------------------------
 1 file changed, 4 insertions(+), 26 deletions(-)

Index: linux-2.6-lttng/arch/x86_64/kernel/kprobes.c
===================================================================
--- linux-2.6-lttng.orig/arch/x86_64/kernel/kprobes.c	2007-07-13 16:08:14.000000000 -0400
+++ linux-2.6-lttng/arch/x86_64/kernel/kprobes.c	2007-07-13 16:41:49.000000000 -0400
@@ -209,42 +209,20 @@
 
 void __kprobes arch_arm_kprobe(struct kprobe *p)
 {
-	unsigned long addr = (unsigned long)p->addr;
-	int page_readonly = 0;
-
-	if (kernel_readonly_text(addr)) {
-		change_page_attr_addr(addr, 1, PAGE_KERNEL_EXEC);
-		global_flush_tlb();
-		page_readonly = 1;
-	}
+	kernel_text_mark_rw((unsigned long) p->addr, sizeof(kprobe_opcode_t));
 	*p->addr = BREAKPOINT_INSTRUCTION;
+	kernel_text_unmark((unsigned long) p->addr, sizeof(kprobe_opcode_t));
 	flush_icache_range((unsigned long) p->addr,
 			   (unsigned long) p->addr + sizeof(kprobe_opcode_t));
-	if (page_readonly) {
-		change_page_attr_addr(addr, 1, PAGE_KERNEL_RO);
-		global_flush_tlb();
-	}
 }
 
 void __kprobes arch_disarm_kprobe(struct kprobe *p)
 {
-	unsigned long addr = (unsigned long)p->addr;
-	int page_readonly = 0;
-
-	if (kernel_readonly_text(addr)) {
-		change_page_attr_addr(addr, 1, PAGE_KERNEL_EXEC);
-		global_flush_tlb();
-		page_readonly = 1;
-	}
-
+	kernel_text_mark_rw((unsigned long) p->addr, sizeof(kprobe_opcode_t));
 	*p->addr = p->opcode;
+	kernel_text_unmark((unsigned long) p->addr, sizeof(kprobe_opcode_t));
 	flush_icache_range((unsigned long) p->addr,
 			   (unsigned long) p->addr + sizeof(kprobe_opcode_t));
-
-	if (page_readonly) {
-		change_page_attr_addr(addr, 1, PAGE_KERNEL_RO);
-		global_flush_tlb();
-	}
 }
 
 void __kprobes arch_remove_kprobe(struct kprobe *p)

-- 
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 1/8] Kprobes - do not use kprobes mutex in arch code

[Christoph Hellwig]

On Fri, Jul 13, 2007 at 09:21:34PM -0400, Mathieu Desnoyers wrote:
> Remove 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.

This is not very nice for avr32/sparc64 which have a noop arch_remove_kprobe
and now need to take a mutex to do nothing.  Maybe you can find a nice
way to avoid that?

Except for this issue making kprobes_mutex static to kprobes.c sounds like
a good improvement.

Re: [patch 0/8] Text Edit Lock

[Christoph Hellwig]

On Fri, Jul 13, 2007 at 09:21:33PM -0400, Mathieu Desnoyers wrote:
> Hi Andrew,
> 
> After refactoring of architecture independant code (mutex to protect text
> edition) vs architecture dependant code (marking/unmarking pages RW) and
> thorough testing, here is the new version of the text edit lock.

Do you have a tarball of your quilt series for markers and all the patches
leading to them?  Trying to hand apply half a dozend patch series is not
too nice :)

Re: [patch 0/8] Text Edit Lock

[Mathieu Desnoyers]

* Christoph Hellwig (hch@infradead.org) wrote:
> On Fri, Jul 13, 2007 at 09:21:33PM -0400, Mathieu Desnoyers wrote:
> > Hi Andrew,
> > 
> > After refactoring of architecture independant code (mutex to protect text
> > edition) vs architecture dependant code (marking/unmarking pages RW) and
> > thorough testing, here is the new version of the text edit lock.
> 
> Do you have a tarball of your quilt series for markers and all the patches
> leading to them?  Trying to hand apply half a dozend patch series is not
> too nice :)
> 

Sure,

Here is the tarball:

http://ltt.polymtl.ca/markers/markers-patches-for-2.6.22-rc6-mm1-07-14-2007.tar.gz

Mathieu

-- 
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 3/8] Text Edit Lock - i386

[Christoph Hellwig]

On Fri, Jul 13, 2007 at 09:21:36PM -0400, Mathieu Desnoyers wrote:
> +#ifdef CONFIG_DEBUG_RODATA
> +static int rodata_marked;
> +#endif

Maybe I'm missing something, but the rodata section should always stay
r/o, only text needs locking/unlocking, no?

Given that all code in this patch only operates on .text this seems fine,
but the variables/config options are grossly misnamed then.  And actually
making rodata r/o doesn't seem like a pure debugging thing to me either.

Re: [patch 1/8] Kprobes - do not use kprobes mutex in arch code

[Mathieu Desnoyers]

* Christoph Hellwig (hch@infradead.org) wrote:
> On Fri, Jul 13, 2007 at 09:21:34PM -0400, Mathieu Desnoyers wrote:
> > Remove 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.
> 
> This is not very nice for avr32/sparc64 which have a noop arch_remove_kprobe
> and now need to take a mutex to do nothing.  Maybe you can find a nice
> way to avoid that?
> 
> Except for this issue making kprobes_mutex static to kprobes.c sounds like
> a good improvement.
> 

Since only unregister_kprobe() calls arch_remove_kprobe(), and only
after having removed the struct kprobe from the kprobes list (while the
kprobes mutex is held), I wonder if there is any need to hold the
kprobes mutex at all when calling arch_remove_kprobe(). It turns out
that only get_insn_slot()/free_insn_slot() (which is in
kernel/kprobes.c, but called from arch specific code) seems to really
use protection of this mutex.

Would it make sense to protect the kprobe_insn_pages list with a
new kprobe_insn_mutex, nestable in the kprobe_mutex ?

-- 
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 1/8] Kprobes - do not use kprobes mutex in arch code

[Mathieu Desnoyers]

* Christoph Hellwig (hch@infradead.org) wrote:
> On Fri, Jul 13, 2007 at 09:21:34PM -0400, Mathieu Desnoyers wrote:
> > Remove 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.
> 
> This is not very nice for avr32/sparc64 which have a noop arch_remove_kprobe
> and now need to take a mutex to do nothing.  Maybe you can find a nice
> way to avoid that?
> 
> Except for this issue making kprobes_mutex static to kprobes.c sounds like
> a good improvement.
> 

While we are here:

The whole check_safety() in kprobes.c seems awkward.. freezing processes
is probably costly, and the check:

if (p != current && p->state == TASK_RUNNING && p->pid != 0) {

Adds restrictions about where a probe can be safely put.. the idle
thread becomes a restriction.

I suggest disabling preemption in the int3 handler, just before
single-stepping, then reenabling it in the breakpoint handler executed
right after the single-step. A synchronize_sched() could then replace
the whole check_safety() and would never fail. The side-effect would be
to disable preemption in the single-step, it's no big deal.

Mathieu


-- 
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] Kprobes - use a mutex to protect the instruction pages list.

[Mathieu Desnoyers]

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.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
CC: hch@infradead.org
CC: prasanna@in.ibm.com
CC: ananth@in.ibm.com
CC: anil.s.keshavamurthy@intel.com
CC: davem@davemloft.net
---
 kernel/kprobes.c |   27 +++++++++++++++++++++------
 1 file changed, 21 insertions(+), 6 deletions(-)

Index: linux-2.6-lttng/kernel/kprobes.c
===================================================================
--- linux-2.6-lttng.orig/kernel/kprobes.c	2007-07-14 15:47:19.000000000 -0400
+++ linux-2.6-lttng/kernel/kprobes.c	2007-07-14 15:48:51.000000000 -0400
@@ -101,6 +101,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);
@@ -137,7 +141,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) {
@@ -146,7 +152,8 @@ kprobe_opcode_t __kprobes *get_insn_slot
 				if (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. */
@@ -160,8 +167,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
@@ -171,7 +180,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);
@@ -179,7 +189,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. */
@@ -213,7 +226,7 @@ static int __kprobes collect_garbage_slo
 	struct 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;
 
@@ -237,6 +250,7 @@ void __kprobes free_insn_slot(kprobe_opc
 	struct 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)) {
@@ -253,6 +267,7 @@ void __kprobes free_insn_slot(kprobe_opc
 
 	if (dirty && ++kprobe_garbage_slots > INSNS_PER_PAGE)
 		collect_garbage_slots();
+	mutex_unlock(&kprobe_insn_mutex);
 }
 #endif
 

-- 
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] Kprobes - Declare kprobe_mutex static

[Mathieu Desnoyers]

Kprobes - Declare kprobe_mutex static

Since it will not be used by other kernel objects, it makes sense to declare it
static.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
CC: hch@infradead.org
CC: prasanna@in.ibm.com
CC: ananth@in.ibm.com
CC: anil.s.keshavamurthy@intel.com
CC: davem@davemloft.net
---
 kernel/kprobes.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: linux-2.6-lttng/kernel/kprobes.c
===================================================================
--- linux-2.6-lttng.orig/kernel/kprobes.c	2007-07-14 15:49:17.000000000 -0400
+++ linux-2.6-lttng/kernel/kprobes.c	2007-07-14 15:49:27.000000000 -0400
@@ -69,7 +69,7 @@ static atomic_t kprobe_count;
 /* 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;
 
-- 
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] Kprobes - no kprobes_mutex needed around arch_remove_kprobe calls

[Mathieu Desnoyers]

Kprobes - no kprobes_mutex needed around arch_remove_kprobe calls.

Since we have the new insn pages mutex, there is no need to protect
arch_remove_kprobe() with the kprobes_mutex anymore. We don't edit the
kernel text there neither, so don't take the text edit lock.

This patch applies after
text-edit-lock-kprobes-architecture-independent.patch

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
CC: hch@infradead.org
CC: prasanna@in.ibm.com
CC: ananth@in.ibm.com
CC: anil.s.keshavamurthy@intel.com
CC: davem@davemloft.net
---
 kernel/kprobes.c |    4 ----
 1 file changed, 4 deletions(-)

Index: linux-2.6-lttng/kernel/kprobes.c
===================================================================
--- linux-2.6-lttng.orig/kernel/kprobes.c	2007-07-14 15:44:57.000000000 -0400
+++ linux-2.6-lttng/kernel/kprobes.c	2007-07-14 15:45:49.000000000 -0400
@@ -663,11 +663,7 @@ valid_p:
 			list_del_rcu(&p->list);
 			kfree(old_p);
 		}
-		mutex_lock(&kprobe_mutex);
-		kernel_text_lock();
 		arch_remove_kprobe(p);
-		kernel_text_unlock();
-		mutex_unlock(&kprobe_mutex);
 	} else {
 		mutex_lock(&kprobe_mutex);
 		if (p->break_handler)

-- 
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] Text Edit Lock - i386 Use kernel_text_is_ro

[Mathieu Desnoyers]

Text Edit Lock - i386 use kernel_text_is_ro

* Christoph Hellwig (hch@infradead.org) wrote:
> On Fri, Jul 13, 2007 at 09:21:36PM -0400, Mathieu Desnoyers wrote:
> > +#ifdef CONFIG_DEBUG_RODATA
> > +static int rodata_marked;
> > +#endif
> 
> Maybe I'm missing something, but the rodata section should always stay
> r/o, only text needs locking/unlocking, no?
> 
> Given that all code in this patch only operates on .text this seems fine,
> but the variables/config options are grossly misnamed then.  And actually
> making rodata r/o doesn't seem like a pure debugging thing to me either.

I think it will change to two different config options eventually (RO
data and RO text). But for now, it makes more sense to use the
kernel_text_is_ro variable.

And since we do not plan to call kernel_text_mark_rw()/kernel_text_unmark()
in the middle of mark_rodata_ro (we are still at boot time), there is no
need to wait until after global_flush_tlb() mark the rodata. And it makes to
rename mark_rodata_ro into kernel_rodata_is_ro.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
CC: hch@infradead.org
---
 arch/i386/mm/init.c |   10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

Index: linux-2.6-lttng/arch/i386/mm/init.c
===================================================================
--- linux-2.6-lttng.orig/arch/i386/mm/init.c	2007-07-14 16:04:20.000000000 -0400
+++ linux-2.6-lttng/arch/i386/mm/init.c	2007-07-14 16:05:45.000000000 -0400
@@ -47,6 +47,7 @@
 #include <asm/paravirt.h>
 
 int kernel_text_is_ro;
+int kernel_rodata_is_ro;
 unsigned int __VMALLOC_RESERVE = 128 << 20;
 
 DEFINE_PER_CPU(struct mmu_gather, mmu_gathers);
@@ -54,9 +55,6 @@ unsigned long highstart_pfn, highend_pfn
 
 static int noinline do_test_wp_bit(void);
 
-#ifdef CONFIG_DEBUG_RODATA
-static int rodata_marked;
-#endif
 
 /*
  * Creates a middle page table and puts a pointer to it in the
@@ -817,6 +815,7 @@ void mark_rodata_ro(void)
 	                 size >> PAGE_SHIFT, PAGE_KERNEL_RO);
 	printk("Write protecting the kernel read-only data: %luk\n",
 	       size >> 10);
+	kernel_rodata_is_ro = 1;
 
 	/*
 	 * change_page_attr() requires a global_flush_tlb() call after it.
@@ -825,7 +824,6 @@ void mark_rodata_ro(void)
 	 * of who is the culprit.
 	 */
 	global_flush_tlb();
-	rodata_marked = 1;
 }
 
 /*
@@ -834,7 +832,7 @@ void mark_rodata_ro(void)
  */
 void __kprobes kernel_text_mark_rw(unsigned long address, size_t len)
 {
-	if (rodata_marked && address >= PFN_ALIGN(_text)
+	if (kernel_text_is_ro && address >= PFN_ALIGN(_text)
 			&& (address + len) <= PFN_ALIGN(_etext)) {
 		unsigned long nr_pages;
 		nr_pages = ((address + len) >> PAGE_SHIFT)
@@ -851,7 +849,7 @@ EXPORT_SYMBOL_GPL(kernel_text_mark_rw);
 
 void __kprobes kernel_text_unmark(unsigned long address, size_t len)
 {
-	if (rodata_marked && address >= PFN_ALIGN(_text)
+	if (kernel_text_is_ro && address >= PFN_ALIGN(_text)
 			&& (address + len) <= PFN_ALIGN(_etext)) {
 		unsigned long nr_pages;
 		nr_pages = ((address + len) >> PAGE_SHIFT)

-- 
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] Text Edit Lock - x86_64 Use kernel_tex_is_ro

[Mathieu Desnoyers]

Text Edit Lock - x86_64 Use kernel_tex_is_ro

It makes more sense to use the kernel_text_is_ro variable instead of
duplicating it.

And since we do not plan to call kernel_text_mark_rw()/kernel_text_unmark()
in the middle of mark_rodata_ro (we are still at boot time), there is no
need to wait until after global_flush_tlb() mark the rodata.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
CC: Andi Kleen <andi@firstfloor.org>
CC: Christoph Hellwig <hch@infradead.org>
---
 arch/x86_64/mm/init.c |   12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

Index: linux-2.6-lttng/arch/x86_64/mm/init.c
===================================================================
--- linux-2.6-lttng.orig/arch/x86_64/mm/init.c	2007-07-14 16:10:21.000000000 -0400
+++ linux-2.6-lttng/arch/x86_64/mm/init.c	2007-07-14 16:11:47.000000000 -0400
@@ -56,10 +56,6 @@ static unsigned long dma_reserve __initd
 
 DEFINE_PER_CPU(struct mmu_gather, mmu_gathers);
 
-#ifdef CONFIG_DEBUG_RODATA
-static int rodata_marked;
-#endif
-
 /*
  * NOTE: pagetable_init alloc all the fixmap pagetables contiguous on the
  * physical space so we can cache the place of the first one and move
@@ -605,13 +601,12 @@ void mark_rodata_ro(void)
 	unsigned long start = PFN_ALIGN(_stext);
 	unsigned long end = PFN_ALIGN(__end_rodata);
 
-	kernel_text_is_ro = 1;
-
 	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);
+	kernel_text_is_ro = 1;
 
 	/*
 	 * change_page_attr_addr() requires a global_flush_tlb() call after it.
@@ -620,7 +615,6 @@ void mark_rodata_ro(void)
 	 * of who is the culprit.
 	 */
 	global_flush_tlb();
-	rodata_marked = 1;
 }
 
 /*
@@ -628,7 +622,7 @@ void mark_rodata_ro(void)
  */
 void kernel_text_mark_rw(unsigned long address, size_t len)
 {
-	if (rodata_marked && address >= PFN_ALIGN(_text)
+	if (kernel_text_is_ro && address >= PFN_ALIGN(_text)
 			&& (address + len) <= PFN_ALIGN(_etext)) {
 		unsigned long nr_pages;
 		nr_pages = ((address + len) >> PAGE_SHIFT)
@@ -644,7 +638,7 @@ EXPORT_SYMBOL_GPL(kernel_text_mark_rw);
 
 void kernel_text_unmark(unsigned long address, size_t len)
 {
-	if (rodata_marked && address >= PFN_ALIGN(_text)
+	if (kernel_text_is_ro && address >= PFN_ALIGN(_text)
 			&& (address + len) <= PFN_ALIGN(_etext)) {
 		unsigned long nr_pages;
 		nr_pages = ((address + len) >> PAGE_SHIFT)

-- 
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] Immediate Values - Architecture Independent Code - Fixes following HCH comments

[Mathieu Desnoyers]

Immediate Values - Architecture Independent Code - Fixes following HCH comments

Various coding style fixes following Christoph's comments
Put asm-generic/immediate.h content into linux/immediate.h.
Remove ifdef __KERNEL__ in .h never exported to user space.
Make immediate_mutex static.

It applies after "Immediate Values - Architecture Independent Code".

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
CC: Christoph Hellwig <hch@infradead.org>
---
 include/linux/immediate.h |   18 ++++++++++++++----
 kernel/immediate.c        |   15 +++++++--------
 2 files changed, 21 insertions(+), 12 deletions(-)

Index: linux-2.6-lttng/include/linux/immediate.h
===================================================================
--- linux-2.6-lttng.orig/include/linux/immediate.h	2007-07-14 18:13:58.000000000 -0400
+++ linux-2.6-lttng/include/linux/immediate.h	2007-07-14 18:16:26.000000000 -0400
@@ -10,12 +10,23 @@
  * See the file COPYING for more details.
  */
 
-#ifdef __KERNEL__
-
 #ifdef CONFIG_IMMEDIATE
 #include <asm/immediate.h>
 #else
-#include <asm-generic/immediate.h>
+/*
+ * Generic immediate values: a simple, standard, memory load.
+ */
+
+struct module;
+
+#define immediate_read(var)		_immediate_read(var)
+#define immediate_set(var, i)		((var)->value = (i))
+#define _immediate_set(var, i)		immediate_set(var, i)
+#define immediate_set_early(var, i)	immediate_set(var, i)
+#define immediate_if(var)		if (immediate_read(var))
+
+static inline void module_immediate_setup(struct module *mod) { }
+static inline void immediate_update_early(void) { }
 #endif
 
 /*
@@ -49,5 +60,4 @@ DEFINE_IMMEDIATE_TYPE(void*, immediate_v
  */
 #define _immediate_if(var)		if (_immediate_read(var))
 
-#endif /* __KERNEL__ */
 #endif
Index: linux-2.6-lttng/kernel/immediate.c
===================================================================
--- linux-2.6-lttng.orig/kernel/immediate.c	2007-07-14 18:13:58.000000000 -0400
+++ linux-2.6-lttng/kernel/immediate.c	2007-07-14 18:27:03.000000000 -0400
@@ -27,13 +27,13 @@ extern const struct __immediate __stop__
  * modules_mutex nests inside immediate_mutex. immediate_mutex protects builtin
  * immediates and module immediates.
  */
-DEFINE_MUTEX(immediate_mutex);
+static DEFINE_MUTEX(immediate_mutex);
 
 /*
  * Sets a range of immediates to a enabled state : set the enable bit.
  */
-static void _immediate_update_range(
-	const struct __immediate *begin, const struct __immediate *end)
+static inline void _immediate_update_range(const struct __immediate *begin,
+		const struct __immediate *end)
 {
 	const struct __immediate *iter;
 	int ret;
@@ -63,11 +63,10 @@ static void _immediate_update_range(
  */
 void module_immediate_setup(struct module *mod)
 {
-	_immediate_update_range(mod->immediate, mod->immediate+mod->num_immediate);
+	_immediate_update_range(mod->immediate,
+				mod->immediate+mod->num_immediate);
 }
-#endif
 
-#ifdef CONFIG_MODULES
 /*
  * immediate mutex nests inside the modules mutex.
  */
@@ -103,8 +102,8 @@ EXPORT_SYMBOL_GPL(immediate_update);
  * Update the immediate values to the state of the variables they refer to. It
  * is done before SMP is active, at the very beginning of start_kernel().
  */
-void __init immediate_update_early_range(
-	const struct __immediate *begin, const struct __immediate *end)
+void __init immediate_update_early_range(const struct __immediate *begin,
+		const struct __immediate *end)
 {
 	const struct __immediate *iter;
 

-- 
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: (drop : wrong email thread) [PATCH] Immediate Values - Architecture Independent Code - Fixes following HCH comments

[Mathieu Desnoyers]

Sorry, this email is sent in the wrong thread. Please drop, I will
resend in the right thread.

* Mathieu Desnoyers (mathieu.desnoyers@polymtl.ca) wrote:
> Immediate Values - Architecture Independent Code - Fixes following HCH comments
> 
> Various coding style fixes following Christoph's comments
> Put asm-generic/immediate.h content into linux/immediate.h.
> Remove ifdef __KERNEL__ in .h never exported to user space.
> Make immediate_mutex static.
> 
> It applies after "Immediate Values - Architecture Independent Code".
> 
> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
> CC: Christoph Hellwig <hch@infradead.org>
> ---
>  include/linux/immediate.h |   18 ++++++++++++++----
>  kernel/immediate.c        |   15 +++++++--------
>  2 files changed, 21 insertions(+), 12 deletions(-)
> 
> Index: linux-2.6-lttng/include/linux/immediate.h
> ===================================================================
> --- linux-2.6-lttng.orig/include/linux/immediate.h	2007-07-14 18:13:58.000000000 -0400
> +++ linux-2.6-lttng/include/linux/immediate.h	2007-07-14 18:16:26.000000000 -0400
> @@ -10,12 +10,23 @@
>   * See the file COPYING for more details.
>   */
>  
> -#ifdef __KERNEL__
> -
>  #ifdef CONFIG_IMMEDIATE
>  #include <asm/immediate.h>
>  #else
> -#include <asm-generic/immediate.h>
> +/*
> + * Generic immediate values: a simple, standard, memory load.
> + */
> +
> +struct module;
> +
> +#define immediate_read(var)		_immediate_read(var)
> +#define immediate_set(var, i)		((var)->value = (i))
> +#define _immediate_set(var, i)		immediate_set(var, i)
> +#define immediate_set_early(var, i)	immediate_set(var, i)
> +#define immediate_if(var)		if (immediate_read(var))
> +
> +static inline void module_immediate_setup(struct module *mod) { }
> +static inline void immediate_update_early(void) { }
>  #endif
>  
>  /*
> @@ -49,5 +60,4 @@ DEFINE_IMMEDIATE_TYPE(void*, immediate_v
>   */
>  #define _immediate_if(var)		if (_immediate_read(var))
>  
> -#endif /* __KERNEL__ */
>  #endif
> Index: linux-2.6-lttng/kernel/immediate.c
> ===================================================================
> --- linux-2.6-lttng.orig/kernel/immediate.c	2007-07-14 18:13:58.000000000 -0400
> +++ linux-2.6-lttng/kernel/immediate.c	2007-07-14 18:27:03.000000000 -0400
> @@ -27,13 +27,13 @@ extern const struct __immediate __stop__
>   * modules_mutex nests inside immediate_mutex. immediate_mutex protects builtin
>   * immediates and module immediates.
>   */
> -DEFINE_MUTEX(immediate_mutex);
> +static DEFINE_MUTEX(immediate_mutex);
>  
>  /*
>   * Sets a range of immediates to a enabled state : set the enable bit.
>   */
> -static void _immediate_update_range(
> -	const struct __immediate *begin, const struct __immediate *end)
> +static inline void _immediate_update_range(const struct __immediate *begin,
> +		const struct __immediate *end)
>  {
>  	const struct __immediate *iter;
>  	int ret;
> @@ -63,11 +63,10 @@ static void _immediate_update_range(
>   */
>  void module_immediate_setup(struct module *mod)
>  {
> -	_immediate_update_range(mod->immediate, mod->immediate+mod->num_immediate);
> +	_immediate_update_range(mod->immediate,
> +				mod->immediate+mod->num_immediate);
>  }
> -#endif
>  
> -#ifdef CONFIG_MODULES
>  /*
>   * immediate mutex nests inside the modules mutex.
>   */
> @@ -103,8 +102,8 @@ EXPORT_SYMBOL_GPL(immediate_update);
>   * Update the immediate values to the state of the variables they refer to. It
>   * is done before SMP is active, at the very beginning of start_kernel().
>   */
> -void __init immediate_update_early_range(
> -	const struct __immediate *begin, const struct __immediate *end)
> +void __init immediate_update_early_range(const struct __immediate *begin,
> +		const struct __immediate *end)
>  {
>  	const struct __immediate *iter;
>  
> 
> -- 
> Mathieu Desnoyers
> Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
> OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68

-- 
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] Text Edit Lock - i386 Fix endif CONFIG_DEBUG_RODATA

[Mathieu Desnoyers]

Text Edit Lock - i386 fix endif CONFIG_DEBUG_RODATA

The #endif would also include kernel_text_unmark().
The replacement empty functions should be static inline.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
---
 arch/i386/mm/init.c           |    2 +-
 include/asm-i386/cacheflush.h |    6 ++----
 2 files changed, 3 insertions(+), 5 deletions(-)

Index: linux-2.6-lttng/arch/i386/mm/init.c
===================================================================
--- linux-2.6-lttng.orig/arch/i386/mm/init.c	2007-07-14 19:24:27.000000000 -0400
+++ linux-2.6-lttng/arch/i386/mm/init.c	2007-07-14 19:24:27.000000000 -0400
@@ -845,7 +845,6 @@ void __kprobes kernel_text_mark_rw(unsig
 	}
 }
 EXPORT_SYMBOL_GPL(kernel_text_mark_rw);
-#endif
 
 void __kprobes kernel_text_unmark(unsigned long address, size_t len)
 {
@@ -862,6 +861,7 @@ void __kprobes kernel_text_unmark(unsign
 	}
 }
 EXPORT_SYMBOL_GPL(kernel_text_unmark);
+#endif
 
 void free_init_pages(char *what, unsigned long begin, unsigned long end)
 {
Index: linux-2.6-lttng/include/asm-i386/cacheflush.h
===================================================================
--- linux-2.6-lttng.orig/include/asm-i386/cacheflush.h	2007-07-14 19:24:36.000000000 -0400
+++ linux-2.6-lttng/include/asm-i386/cacheflush.h	2007-07-14 19:25:04.000000000 -0400
@@ -34,15 +34,13 @@ void kernel_map_pages(struct page *page,
 
 #ifdef CONFIG_DEBUG_RODATA
 void mark_rodata_ro(void);
-#endif
 
-#if defined(CONFIG_DEBUG_RODATA)
 /* mark kernel text pages writable */
 extern void kernel_text_mark_rw(unsigned long address, size_t len);
 extern void kernel_text_unmark(unsigned long address, size_t len);
 #else
-void kernel_text_mark_rw(unsigned long address, size_t len) { }
-void kernel_text_unmark(unsigned long address, size_t len) { }
+static inline void kernel_text_mark_rw(unsigned long address, size_t len) { }
+static inline void kernel_text_unmark(unsigned long address, size_t len) { }
 #endif
 
 #endif /* _I386_CACHEFLUSH_H */
-- 
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] Text Edit Lock - x86_64 Fix !CONFIG_DEBUG_RODATA

[Mathieu Desnoyers]

Text Edit Lock - x86_64 Fix !CONFIG_DEBUG_RODATA

Same fix as for i386: declare remplacement empty functions as static inline.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
---
 include/asm-x86_64/cacheflush.h |    6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

Index: linux-2.6-lttng/include/asm-x86_64/cacheflush.h
===================================================================
--- linux-2.6-lttng.orig/include/asm-x86_64/cacheflush.h	2007-07-14 19:26:12.000000000 -0400
+++ linux-2.6-lttng/include/asm-x86_64/cacheflush.h	2007-07-14 19:26:52.000000000 -0400
@@ -31,15 +31,13 @@ void clflush_cache_range(void *addr, int
 
 #ifdef CONFIG_DEBUG_RODATA
 void mark_rodata_ro(void);
-#endif
 
-#if defined(CONFIG_DEBUG_RODATA)
 /* mark kernel text pages writable */
 extern void kernel_text_mark_rw(unsigned long address, size_t len);
 extern void kernel_text_unmark(unsigned long address, size_t len);
 #else
-void kernel_text_mark_rw(unsigned long address, size_t len) { }
-void kernel_text_unmark(unsigned long address, size_t len) { }
+static inline void kernel_text_mark_rw(unsigned long address, size_t len) { }
+static inline void kernel_text_unmark(unsigned long address, size_t len) { }
 #endif
 
 #endif /* _X8664_CACHEFLUSH_H */

-- 
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] Text Edit Lock - Architecture Independent Code - kerneldoc

[Mathieu Desnoyers]

Text Edit Lock - Architecture Independent Code - kerneldoc

Add kerneldoc to text edit lock API.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
CC: hch@infradead.org
---
 include/linux/memory.h |   17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

Index: linux-2.6-lttng/include/linux/memory.h
===================================================================
--- linux-2.6-lttng.orig/include/linux/memory.h	2007-07-14 20:08:50.000000000 -0400
+++ linux-2.6-lttng/include/linux/memory.h	2007-07-14 20:18:13.000000000 -0400
@@ -86,11 +86,24 @@ extern int remove_memory_block(unsigned 
 	register_memory_notifier(&fn##_mem_nb);			\
 }
 
-/*
- * Take and release the kernel text modification lock, used for code patching.
+/**
+ * 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.
  */
+
 extern void kernel_text_lock(void);
+
+/**
+ * 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.
+ */
+
 extern void kernel_text_unlock(void);
 
 #endif /* _LINUX_MEMORY_H_ */
-- 
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] Text Edit Lock - i386 kerneldoc

[Mathieu Desnoyers]

Text Edit Lock - i386 kerneldoc

Add kerneldoc to text edit lock i386 API.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
CC: hch@infradead.org
---
 include/asm-i386/cacheflush.h |   19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

Index: linux-2.6-lttng/include/asm-i386/cacheflush.h
===================================================================
--- linux-2.6-lttng.orig/include/asm-i386/cacheflush.h	2007-07-14 20:23:01.000000000 -0400
+++ linux-2.6-lttng/include/asm-i386/cacheflush.h	2007-07-14 20:24:17.000000000 -0400
@@ -35,8 +35,25 @@ void kernel_map_pages(struct page *page,
 #ifdef CONFIG_DEBUG_RODATA
 void mark_rodata_ro(void);
 
-/* mark kernel text pages writable */
+/**
+ * kernel_text_mark_rw  -   Mark kernel text RW
+ * @address:                location of the code
+ * @len:                    size of code to mark
+ *
+ * Mark the kernel text pages writable so they can safely written to. This is
+ * useful for code patching.
+*/
+
 extern void kernel_text_mark_rw(unsigned long address, size_t len);
+
+/**
+ * kernel_text_unmark  -    Mark kernel text back to its original flags
+ * @address:                location of the code
+ * @len:                    size of code to mark
+ *
+ * Mark the kernel text back to its original flags.
+ */
+
 extern void kernel_text_unmark(unsigned long address, size_t len);
 #else
 static inline void kernel_text_mark_rw(unsigned long address, size_t len) { }
-- 
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] Text Edit Lock - x86_64 kerneldoc

[Mathieu Desnoyers]

Text Edit Lock - x86_64 kerneldoc

Add kerneldoc to text edit lock x86_64 API.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
CC: hch@infradead.org
---
 include/asm-x86_64/cacheflush.h |   19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

Index: linux-2.6-lttng/include/asm-x86_64/cacheflush.h
===================================================================
--- linux-2.6-lttng.orig/include/asm-x86_64/cacheflush.h	2007-07-14 20:25:49.000000000 -0400
+++ linux-2.6-lttng/include/asm-x86_64/cacheflush.h	2007-07-14 20:25:50.000000000 -0400
@@ -32,8 +32,25 @@ void clflush_cache_range(void *addr, int
 #ifdef CONFIG_DEBUG_RODATA
 void mark_rodata_ro(void);
 
-/* mark kernel text pages writable */
+/**
+ * kernel_text_mark_rw  -   Mark kernel text RW
+ * @address:                location of the code
+ * @len:                    size of code to mark
+ *
+ * Mark the kernel text pages writable so they can safely written to. This is
+ * useful for code patching.
+*/
+
 extern void kernel_text_mark_rw(unsigned long address, size_t len);
+
+/**
+ * kernel_text_unmark  -    Mark kernel text back to its original flags
+ * @address:                location of the code
+ * @len:                    size of code to mark
+ *
+ * Mark the kernel text back to its original flags.
+ */
+
 extern void kernel_text_unmark(unsigned long address, size_t len);
 #else
 static inline void kernel_text_mark_rw(unsigned long address, size_t len) { }
-- 
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] Text Edit Lock - Architecture Independent Code - kerneldoc

[Christoph Hellwig]

On Sat, Jul 14, 2007 at 09:28:07PM -0400, Mathieu Desnoyers wrote:
> Text Edit Lock - Architecture Independent Code - kerneldoc
> 
> Add kerneldoc to text edit lock API.

kerneldoc is placed in the implementation file, not the header.

Re: [PATCH] Text Edit Lock - Architecture Independent Code - kerneldoc

[Mathieu Desnoyers]

Please drop. Will be replaced by 
text-edit-lock-architecture-independent-code-kerneldoc-implementation.patch

* Mathieu Desnoyers (mathieu.desnoyers@polymtl.ca) wrote:
> Text Edit Lock - Architecture Independent Code - kerneldoc
> 
> Add kerneldoc to text edit lock API.
> 
> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
> CC: hch@infradead.org
> ---
>  include/linux/memory.h |   17 +++++++++++++++--
>  1 file changed, 15 insertions(+), 2 deletions(-)
> 
> Index: linux-2.6-lttng/include/linux/memory.h
> ===================================================================
> --- linux-2.6-lttng.orig/include/linux/memory.h	2007-07-14 20:08:50.000000000 -0400
> +++ linux-2.6-lttng/include/linux/memory.h	2007-07-14 20:18:13.000000000 -0400
> @@ -86,11 +86,24 @@ extern int remove_memory_block(unsigned 
>  	register_memory_notifier(&fn##_mem_nb);			\
>  }
>  
> -/*
> - * Take and release the kernel text modification lock, used for code patching.
> +/**
> + * 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.
>   */
> +
>  extern void kernel_text_lock(void);
> +
> +/**
> + * 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.
> + */
> +
>  extern void kernel_text_unlock(void);
>  
>  #endif /* _LINUX_MEMORY_H_ */
> -- 
> Mathieu Desnoyers
> Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
> OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68

-- 
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] Text Edit Lock - i386 kerneldoc

[Mathieu Desnoyers]

Please drop, will be replaced by
text-edit-lock-i386-kerneldoc-implementation.patch

* Mathieu Desnoyers (mathieu.desnoyers@polymtl.ca) wrote:
> Text Edit Lock - i386 kerneldoc
> 
> Add kerneldoc to text edit lock i386 API.
> 
> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
> CC: hch@infradead.org
> ---
>  include/asm-i386/cacheflush.h |   19 ++++++++++++++++++-
>  1 file changed, 18 insertions(+), 1 deletion(-)
> 
> Index: linux-2.6-lttng/include/asm-i386/cacheflush.h
> ===================================================================
> --- linux-2.6-lttng.orig/include/asm-i386/cacheflush.h	2007-07-14 20:23:01.000000000 -0400
> +++ linux-2.6-lttng/include/asm-i386/cacheflush.h	2007-07-14 20:24:17.000000000 -0400
> @@ -35,8 +35,25 @@ void kernel_map_pages(struct page *page,
>  #ifdef CONFIG_DEBUG_RODATA
>  void mark_rodata_ro(void);
>  
> -/* mark kernel text pages writable */
> +/**
> + * kernel_text_mark_rw  -   Mark kernel text RW
> + * @address:                location of the code
> + * @len:                    size of code to mark
> + *
> + * Mark the kernel text pages writable so they can safely written to. This is
> + * useful for code patching.
> +*/
> +
>  extern void kernel_text_mark_rw(unsigned long address, size_t len);
> +
> +/**
> + * kernel_text_unmark  -    Mark kernel text back to its original flags
> + * @address:                location of the code
> + * @len:                    size of code to mark
> + *
> + * Mark the kernel text back to its original flags.
> + */
> +
>  extern void kernel_text_unmark(unsigned long address, size_t len);
>  #else
>  static inline void kernel_text_mark_rw(unsigned long address, size_t len) { }
> -- 
> Mathieu Desnoyers
> Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
> OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68

-- 
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] Text Edit Lock - x86_64 kerneldoc

[Mathieu Desnoyers]

Please drop, will be replaced by
text-edit-lock-x86_64-kerneldoc-implementation.patch

* Mathieu Desnoyers (mathieu.desnoyers@polymtl.ca) wrote:
> Text Edit Lock - x86_64 kerneldoc
> 
> Add kerneldoc to text edit lock x86_64 API.
> 
> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
> CC: hch@infradead.org
> ---
>  include/asm-x86_64/cacheflush.h |   19 ++++++++++++++++++-
>  1 file changed, 18 insertions(+), 1 deletion(-)
> 
> Index: linux-2.6-lttng/include/asm-x86_64/cacheflush.h
> ===================================================================
> --- linux-2.6-lttng.orig/include/asm-x86_64/cacheflush.h	2007-07-14 20:25:49.000000000 -0400
> +++ linux-2.6-lttng/include/asm-x86_64/cacheflush.h	2007-07-14 20:25:50.000000000 -0400
> @@ -32,8 +32,25 @@ void clflush_cache_range(void *addr, int
>  #ifdef CONFIG_DEBUG_RODATA
>  void mark_rodata_ro(void);
>  
> -/* mark kernel text pages writable */
> +/**
> + * kernel_text_mark_rw  -   Mark kernel text RW
> + * @address:                location of the code
> + * @len:                    size of code to mark
> + *
> + * Mark the kernel text pages writable so they can safely written to. This is
> + * useful for code patching.
> +*/
> +
>  extern void kernel_text_mark_rw(unsigned long address, size_t len);
> +
> +/**
> + * kernel_text_unmark  -    Mark kernel text back to its original flags
> + * @address:                location of the code
> + * @len:                    size of code to mark
> + *
> + * Mark the kernel text back to its original flags.
> + */
> +
>  extern void kernel_text_unmark(unsigned long address, size_t len);
>  #else
>  static inline void kernel_text_mark_rw(unsigned long address, size_t len) { }
> -- 
> Mathieu Desnoyers
> Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
> OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68

-- 
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] Text Edit Lock - Architecture Independent Code for Implementation

[Mathieu Desnoyers]

Text Edit Lock - Architecture Independent Code - kerneldoc implementation

Add kerneldoc to text edit lock API.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
CC: hch@infradead.org
---
 mm/memory.c |   16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

Index: linux-2.6-lttng/mm/memory.c
===================================================================
--- linux-2.6-lttng.orig/mm/memory.c	2007-07-15 18:56:32.000000000 -0400
+++ linux-2.6-lttng/mm/memory.c	2007-07-15 18:57:18.000000000 -0400
@@ -2820,9 +2820,12 @@ int access_process_vm(struct task_struct
 }
 EXPORT_SYMBOL_GPL(access_process_vm);
 
-/*
- * Lock the kernel text for mutual write exclusion. Used for dynamic code
- * patching.
+/**
+ * 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)
 {
@@ -2830,6 +2833,13 @@ void __kprobes kernel_text_lock(void)
 }
 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);
-- 
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] Text Edit Lock - i386 kerneldoc implementation

[Mathieu Desnoyers]

Text Edit Lock - i386 kerneldoc implementation

Add kerneldoc to text edit lock i386 API.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
CC: hch@infradead.org
---
 arch/i386/mm/init.c |   17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

Index: linux-2.6-lttng/arch/i386/mm/init.c
===================================================================
--- linux-2.6-lttng.orig/arch/i386/mm/init.c	2007-07-15 18:59:31.000000000 -0400
+++ linux-2.6-lttng/arch/i386/mm/init.c	2007-07-15 19:00:05.000000000 -0400
@@ -826,9 +826,13 @@ void mark_rodata_ro(void)
 	global_flush_tlb();
 }
 
-/*
- * Lock the kernel text for mutual write exclusion.
- * Make sure the pages are writable.
+/**
+ * kernel_text_mark_rw  -   Mark kernel text RW
+ * @address:                location of the code
+ * @len:                    size of code to mark
+ *
+ * Mark the kernel text pages writable so they can safely written to. This is
+ * useful for code patching.
  */
 void __kprobes kernel_text_mark_rw(unsigned long address, size_t len)
 {
@@ -846,6 +850,13 @@ void __kprobes kernel_text_mark_rw(unsig
 }
 EXPORT_SYMBOL_GPL(kernel_text_mark_rw);
 
+/**
+ * kernel_text_unmark  -    Mark kernel text back to its original flags
+ * @address:                location of the code
+ * @len:                    size of code to mark
+ *
+ * Mark the kernel text back to its original flags.
+ */
 void __kprobes kernel_text_unmark(unsigned long address, size_t len)
 {
 	if (kernel_text_is_ro && address >= PFN_ALIGN(_text)
-- 
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] Text Edit Lock - x86_64 kerneldoc implementation

[Mathieu Desnoyers]

Text Edit Lock - x86_64 kerneldoc implementation

Add kerneldoc to text edit lock x86_64 API.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
CC: hch@infradead.org
CC: Andi Kleen <andi@firstfloor.org>
---
 arch/x86_64/mm/init.c |   16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

Index: linux-2.6-lttng/arch/x86_64/mm/init.c
===================================================================
--- linux-2.6-lttng.orig/arch/x86_64/mm/init.c	2007-07-15 19:01:50.000000000 -0400
+++ linux-2.6-lttng/arch/x86_64/mm/init.c	2007-07-15 19:02:18.000000000 -0400
@@ -617,8 +617,13 @@ void mark_rodata_ro(void)
 	global_flush_tlb();
 }
 
-/*
- * Mark kernel text pages writable.
+/**
+ * kernel_text_mark_rw  -   Mark kernel text RW
+ * @address:                location of the code
+ * @len:                    size of code to mark
+ *
+ * Mark the kernel text pages writable so they can safely written to. This is
+ * useful for code patching.
  */
 void kernel_text_mark_rw(unsigned long address, size_t len)
 {
@@ -636,6 +641,13 @@ void kernel_text_mark_rw(unsigned long a
 }
 EXPORT_SYMBOL_GPL(kernel_text_mark_rw);
 
+/**
+ * kernel_text_unmark  -    Mark kernel text back to its original flags
+ * @address:                location of the code
+ * @len:                    size of code to mark
+ *
+ * Mark the kernel text back to its original flags.
+ */
 void kernel_text_unmark(unsigned long address, size_t len)
 {
 	if (kernel_text_is_ro && address >= PFN_ALIGN(_text)
-- 
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 1/8] Kprobes - do not use kprobes mutex in arch code

[Ananth N Mavinakayanahalli]

On Sat, Jul 14, 2007 at 03:20:02PM -0400, Mathieu Desnoyers wrote:
> * Christoph Hellwig (hch@infradead.org) wrote:
> > On Fri, Jul 13, 2007 at 09:21:34PM -0400, Mathieu Desnoyers wrote:
> > > Remove 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.
> > 
> > This is not very nice for avr32/sparc64 which have a noop arch_remove_kprobe
> > and now need to take a mutex to do nothing.  Maybe you can find a nice
> > way to avoid that?
> > 
> > Except for this issue making kprobes_mutex static to kprobes.c sounds like
> > a good improvement.
> > 
> 
> Since only unregister_kprobe() calls arch_remove_kprobe(), and only
> after having removed the struct kprobe from the kprobes list (while the
> kprobes mutex is held), I wonder if there is any need to hold the
> kprobes mutex at all when calling arch_remove_kprobe(). It turns out
> that only get_insn_slot()/free_insn_slot() (which is in
> kernel/kprobes.c, but called from arch specific code) seems to really
> use protection of this mutex.

Right.

> Would it make sense to protect the kprobe_insn_pages list with a
> new kprobe_insn_mutex, nestable in the kprobe_mutex ?

Do you think it is required after your change to make kprobe_mutex
static? But yes, for architectures that don't need a arch_remove_kprobe,
the situation is a bit odd... a mutex to do nothing. IIRC, that was the
primary reason why we made the mutex visible outside of kernel/kprobes.c

Ananth

Re: [patch 1/8] Kprobes - do not use kprobes mutex in arch code

[Mathieu Desnoyers]

* Ananth N Mavinakayanahalli (ananth@in.ibm.com) wrote:
> On Sat, Jul 14, 2007 at 03:20:02PM -0400, Mathieu Desnoyers wrote:
> > * Christoph Hellwig (hch@infradead.org) wrote:
> > > On Fri, Jul 13, 2007 at 09:21:34PM -0400, Mathieu Desnoyers wrote:
> > > > Remove 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.
> > > 
> > > This is not very nice for avr32/sparc64 which have a noop arch_remove_kprobe
> > > and now need to take a mutex to do nothing.  Maybe you can find a nice
> > > way to avoid that?
> > > 
> > > Except for this issue making kprobes_mutex static to kprobes.c sounds like
> > > a good improvement.
> > > 
> > 
> > Since only unregister_kprobe() calls arch_remove_kprobe(), and only
> > after having removed the struct kprobe from the kprobes list (while the
> > kprobes mutex is held), I wonder if there is any need to hold the
> > kprobes mutex at all when calling arch_remove_kprobe(). It turns out
> > that only get_insn_slot()/free_insn_slot() (which is in
> > kernel/kprobes.c, but called from arch specific code) seems to really
> > use protection of this mutex.
> 
> Right.
> 
> > Would it make sense to protect the kprobe_insn_pages list with a
> > new kprobe_insn_mutex, nestable in the kprobe_mutex ?
> 
> Do you think it is required after your change to make kprobe_mutex
> static? But yes, for architectures that don't need a arch_remove_kprobe,
> the situation is a bit odd... a mutex to do nothing. IIRC, that was the
> primary reason why we made the mutex visible outside of kernel/kprobes.c
> 

After making the kprobe_mutex static, the only alternative we have is to
protect the arch_remove_kprobe (empty on some architectures) call with
kprobe_mutex, which, as Christoph pointed out, is not so great. Besides,
I think the kprobe_insn_mutex would make more sense than taking a mutex
in the arch-specific arch_remove_kprobe() for a resource that is not
clearly identified.

Mathieu

-- 
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 1/8] Kprobes - do not use kprobes mutex in arch code

[Ananth N Mavinakayanahalli]

On Fri, Jul 13, 2007 at 09:21:34PM -0400, Mathieu Desnoyers wrote:
> Remove 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.
> 
> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>

Acked-by: Ananth N Mavinakayanahalli <ananth@in.ibm.com>

Thanks Mathieu.

> CC: prasanna@in.ibm.com
> CC: ananth@in.ibm.com
> CC: anil.s.keshavamurthy@intel.com
> CC: davem@davemloft.net
> ---
>  arch/i386/kernel/kprobes.c    |    2 --
>  arch/ia64/kernel/kprobes.c    |    2 --
>  arch/powerpc/kernel/kprobes.c |    2 --
>  arch/s390/kernel/kprobes.c    |    2 --
>  arch/x86_64/kernel/kprobes.c  |    2 --
>  include/linux/kprobes.h       |    2 --
>  kernel/kprobes.c              |    2 ++
>  7 files changed, 2 insertions(+), 12 deletions(-)
> 
> Index: linux-2.6-lttng/include/linux/kprobes.h
> ===================================================================
> --- linux-2.6-lttng.orig/include/linux/kprobes.h	2007-07-13 18:42:08.000000000 -0400
> +++ linux-2.6-lttng/include/linux/kprobes.h	2007-07-13 18:42:52.000000000 -0400
> @@ -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>
> @@ -177,7 +176,6 @@
>  }
> 
>  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);
> Index: linux-2.6-lttng/arch/i386/kernel/kprobes.c
> ===================================================================
> --- linux-2.6-lttng.orig/arch/i386/kernel/kprobes.c	2007-07-13 18:42:08.000000000 -0400
> +++ linux-2.6-lttng/arch/i386/kernel/kprobes.c	2007-07-13 18:42:52.000000000 -0400
> @@ -209,9 +209,7 @@
> 
>  void __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)
> Index: linux-2.6-lttng/kernel/kprobes.c
> ===================================================================
> --- linux-2.6-lttng.orig/kernel/kprobes.c	2007-07-13 18:42:10.000000000 -0400
> +++ linux-2.6-lttng/kernel/kprobes.c	2007-07-13 18:43:17.000000000 -0400
> @@ -641,7 +641,9 @@
>  			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)
> Index: linux-2.6-lttng/arch/ia64/kernel/kprobes.c
> ===================================================================
> --- linux-2.6-lttng.orig/arch/ia64/kernel/kprobes.c	2007-07-13 18:43:59.000000000 -0400
> +++ linux-2.6-lttng/arch/ia64/kernel/kprobes.c	2007-07-13 18:44:03.000000000 -0400
> @@ -565,9 +565,7 @@
> 
>  void __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
> Index: linux-2.6-lttng/arch/powerpc/kernel/kprobes.c
> ===================================================================
> --- linux-2.6-lttng.orig/arch/powerpc/kernel/kprobes.c	2007-07-13 18:44:20.000000000 -0400
> +++ linux-2.6-lttng/arch/powerpc/kernel/kprobes.c	2007-07-13 18:44:26.000000000 -0400
> @@ -86,9 +86,7 @@
> 
>  void __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)
> Index: linux-2.6-lttng/arch/s390/kernel/kprobes.c
> ===================================================================
> --- linux-2.6-lttng.orig/arch/s390/kernel/kprobes.c	2007-07-13 18:44:43.000000000 -0400
> +++ linux-2.6-lttng/arch/s390/kernel/kprobes.c	2007-07-13 18:44:46.000000000 -0400
> @@ -218,9 +218,7 @@
> 
>  void __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)
> Index: linux-2.6-lttng/arch/x86_64/kernel/kprobes.c
> ===================================================================
> --- linux-2.6-lttng.orig/arch/x86_64/kernel/kprobes.c	2007-07-13 18:45:08.000000000 -0400
> +++ linux-2.6-lttng/arch/x86_64/kernel/kprobes.c	2007-07-13 18:45:14.000000000 -0400
> @@ -249,9 +249,7 @@
> 
>  void __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)
> 
> -- 
> 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] Kprobes - use a mutex to protect the instruction pages list.

[Ananth N Mavinakayanahalli]

On Sat, Jul 14, 2007 at 03:51:39PM -0400, Mathieu Desnoyers wrote:
> 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.
> 
> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
> CC: hch@infradead.org
> CC: prasanna@in.ibm.com
> CC: ananth@in.ibm.com
> CC: anil.s.keshavamurthy@intel.com
> CC: davem@davemloft.net

Acked-by: Ananth N Mavinakayanahalli <ananth@in.ibm.com>

Thanks Mathieu

> ---
>  kernel/kprobes.c |   27 +++++++++++++++++++++------
>  1 file changed, 21 insertions(+), 6 deletions(-)
> 
> Index: linux-2.6-lttng/kernel/kprobes.c
> ===================================================================
> --- linux-2.6-lttng.orig/kernel/kprobes.c	2007-07-14 15:47:19.000000000 -0400
> +++ linux-2.6-lttng/kernel/kprobes.c	2007-07-14 15:48:51.000000000 -0400
> @@ -101,6 +101,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);
> @@ -137,7 +141,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) {
> @@ -146,7 +152,8 @@ kprobe_opcode_t __kprobes *get_insn_slot
>  				if (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. */
> @@ -160,8 +167,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
> @@ -171,7 +180,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);
> @@ -179,7 +189,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. */
> @@ -213,7 +226,7 @@ static int __kprobes collect_garbage_slo
>  	struct 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;
> 
> @@ -237,6 +250,7 @@ void __kprobes free_insn_slot(kprobe_opc
>  	struct 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)) {
> @@ -253,6 +267,7 @@ void __kprobes free_insn_slot(kprobe_opc
> 
>  	if (dirty && ++kprobe_garbage_slots > INSNS_PER_PAGE)
>  		collect_garbage_slots();
> +	mutex_unlock(&kprobe_insn_mutex);
>  }
>  #endif
> 
> 
> -- 
> 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] Kprobes - Declare kprobe_mutex static

[Ananth N Mavinakayanahalli]

On Sat, Jul 14, 2007 at 03:52:41PM -0400, Mathieu Desnoyers wrote:
> Kprobes - Declare kprobe_mutex static
> 
> Since it will not be used by other kernel objects, it makes sense to declare it
> static.
> 
> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>

Acked-by: Ananth N Mavinakayanahalli <ananth@in.ibm.com>

Thanks Mathieu

> CC: hch@infradead.org
> CC: prasanna@in.ibm.com
> CC: ananth@in.ibm.com
> CC: anil.s.keshavamurthy@intel.com
> CC: davem@davemloft.net
> ---
>  kernel/kprobes.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> Index: linux-2.6-lttng/kernel/kprobes.c
> ===================================================================
> --- linux-2.6-lttng.orig/kernel/kprobes.c	2007-07-14 15:49:17.000000000 -0400
> +++ linux-2.6-lttng/kernel/kprobes.c	2007-07-14 15:49:27.000000000 -0400
> @@ -69,7 +69,7 @@ static atomic_t kprobe_count;
>  /* 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;
> 
> -- 
> Mathieu Desnoyers
> Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
> OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68