Messages in this thread | | | From | "zhangxiliang" <> | Subject | RE: [patch 05/24] Text Edit Lock - Architecture Independent Code | Date | Fri, 21 Dec 2007 13:18:46 +0800 |
| |
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: linux-kernel-owner@vger.kernel.org > [mailto:linux-kernel-owner@vger.kernel.org] On Behalf Of > Mathieu Desnoyers > Sent: Friday, December 21, 2007 9:55 AM > To: akpm@linux-foundation.org; 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 <mathieu.desnoyers@polymtl.ca> > CC: Andi Kleen <andi@firstfloor.org> > --- > 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 majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ > >
| |