hello,
I have some questions for your patches.
> Paravirt and alternatives are always done when SMP is
> inactive, so there is no
> need to use locks.
> -#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);
> +
Why "mark_rodata_ro" doesn't consider smp instance? Maybe it will be appied in
future.
> ===================================================================
> --- linux-2.6-lttng.orig/kernel/kprobes.c 2007-12-12
> 18:10:32.000000000 -0500
> +++ linux-2.6-lttng/kernel/kprobes.c 2007-12-12
> 18:10:34.000000000 -0500
> @@ -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.
> -----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;
> linux-kernel@vger.kernel.org
> Cc: Mathieu Desnoyers; Andi Kleen
> Subject: [patch 05/24] Text Edit Lock - Architecture Independent Code
>
> 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 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.h 2007-11-07
> 11:13:48.000000000 -0500
> @@ -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_ */
> Index: linux-2.6-lttng/mm/memory.c
> ===================================================================
> --- linux-2.6-lttng.orig/mm/memory.c 2007-11-07
> 11:12:33.000000000 -0500
> +++ linux-2.6-lttng/mm/memory.c 2007-11-07
> 11:14:25.000000000 -0500
> @@ -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_struct
>
> return 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);
>
> --
> 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/
>
>
--
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/
