Re: [patch 05/24] Text Edit Lock - Architecture Independent Code
* zhangxiliang ([EMAIL PROTECTED]) wrote:
> hello,
>I have some questions for your patches.
>
Hi,
> > 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.
>
In its previous state, mark_rodata_ro was disabled in these situations :
- System supports CPU_HOTPLUG (alternatives will need to be applied when
we pass from 1->2 and 2->1 cpu.
- System supports KPROBES, it need to put breakpoints in the code.
The main effect of the change I introduce is that I allow the kernel
code to be marked RO even in these situations. Alternatives and kprobes
uses the text_poke to modify kernel code, which temporarily disabled the
Write Protection bit on the local CPU so the memory write to RO pages
can be done.
So I guess the answer to your question is : previously, mark_rodata_ro
did not support CPU HOTPLUG nor KPROBES, and now it does, which is
much cleaner.
>
> > ===
> > --- linux-2.6-lttng.orig/kernel/kprobes.c 2007-12-12
> > 18:10:32.0 -0500
> > +++ linux-2.6-lttng/kernel/kprobes.c2007-12-12
> > 18:10:34.0 -0500
> > @@ -644,7 +644,9 @@ valid_p:
> > list_del_rcu(>list);
> > kfree(old_p);
> > }
> > + mutex_lock(_mutex);
> > arch_remove_kprobe(p);
> > + mutex_unlock(_mutex);
> > } else {
> > mutex_lock(_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.
>
Which patch is this coming from ?
My Text Edit Lock - kprobes architecture independent support
patch _removes_ the kprobe_mutex taken around arch_remove_kprobes
because it is useless, I don't see how this patch snippet applies to my
patchset at all.
If you suggest to change the way locking is currently done in
kprobes, please do this in a separate thread, as a RFC ?
Mathieu
>
> > -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.0 -0500
> > +++ linux-2.6-lttng/include/linux/memory.h 2007-11-07
> > 11:13:48.0 -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
> >
Re: [patch 05/24] Text Edit Lock - Architecture Independent Code
* zhangxiliang ([EMAIL PROTECTED]) wrote:
hello,
I have some questions for your patches.
Hi,
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.
In its previous state, mark_rodata_ro was disabled in these situations :
- System supports CPU_HOTPLUG (alternatives will need to be applied when
we pass from 1-2 and 2-1 cpu.
- System supports KPROBES, it need to put breakpoints in the code.
The main effect of the change I introduce is that I allow the kernel
code to be marked RO even in these situations. Alternatives and kprobes
uses the text_poke to modify kernel code, which temporarily disabled the
Write Protection bit on the local CPU so the memory write to RO pages
can be done.
So I guess the answer to your question is : previously, mark_rodata_ro
did not support CPU HOTPLUG nor KPROBES, and now it does, which is
much cleaner.
===
--- linux-2.6-lttng.orig/kernel/kprobes.c 2007-12-12
18:10:32.0 -0500
+++ linux-2.6-lttng/kernel/kprobes.c2007-12-12
18:10:34.0 -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.
Which patch is this coming from ?
My Text Edit Lock - kprobes architecture independent support
patch _removes_ the kprobe_mutex taken around arch_remove_kprobes
because it is useless, I don't see how this patch snippet applies to my
patchset at all.
If you suggest to change the way locking is currently done in
kprobes, please do this in a separate thread, as a RFC ?
Mathieu
-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.0 -0500
+++ linux-2.6-lttng/include/linux/memory.h 2007-11-07
11:13:48.0 -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.c2007-11-07
11:12:33.0 -0500
+++ linux-2.6-lttng/mm/memory.c 2007-11-07
11:14:25.0 -0500
@@ -50,6 +50,8 @@
RE: [patch 05/24] Text Edit Lock - Architecture Independent Code
> > ===
> > --- linux-2.6-lttng.orig/kernel/kprobes.c 2007-12-12
> > 18:10:32.0 -0500
> > +++ linux-2.6-lttng/kernel/kprobes.c2007-12-12
> > 18:10:34.0 -0500
> > @@ -644,7 +644,9 @@ valid_p:
> > list_del_rcu(>list);
> > kfree(old_p);
> > }
> > + mutex_lock(_mutex);
> > arch_remove_kprobe(p);
> > + mutex_unlock(_mutex);
> > } else {
> > mutex_lock(_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.
Could we insert the "mutex_lock" and "mutex_unlock" into "free_insn_slot"
instead of architecture code?
modify as follows:
void __kprobes free_insn_slot(kprobe_opcode_t * slot, int dirty)
{
struct kprobe_insn_page *kip;
struct hlist_node *pos;
+ mutex_lock(_mutex);
hlist_for_each_entry(kip, pos, _insn_pages, hlist) {
if (kip->insns <= slot &&
slot < kip->insns + (INSNS_PER_PAGE * MAX_INSN_SIZE)) {
int i = (slot - kip->insns) / MAX_INSN_SIZE;
if (dirty) {
kip->slot_used[i] = SLOT_DIRTY;
kip->ngarbage++;
} else {
collect_one_slot(kip, i);
}
break;
}
}
if (dirty && ++kprobe_garbage_slots > INSNS_PER_PAGE)
collect_garbage_slots();
+ mutex_unlock(_mutex);
}
> -Original Message-----
> From: [EMAIL PROTECTED]
> [mailto:[EMAIL PROTECTED] On Behalf Of zhangxiliang
> Sent: Friday, December 21, 2007 1:19 PM
> To: 'Mathieu Desnoyers'; [EMAIL PROTECTED]; 'Ingo
> Molnar'; linux-kernel@vger.kernel.org
> Cc: 'Andi Kleen'
> Subject: RE: [patch 05/24] Text Edit Lock - Architecture
> Independent Code
>
> 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.0 -0500
> > +++ linux-2.6-lttng/kernel/kprobes.c2007-12-12
> > 18:10:34.0 -0500
> > @@ -644,7 +644,9 @@ valid_p:
> > list_del_rcu(>list);
> > kfree(old_p);
> > }
> > + mutex_lock(_mutex);
> > arch_remove_kprobe(p);
> > + mutex_unlock(_mutex);
> > } else {
> > mutex_lock(_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.
&
RE: [patch 05/24] Text Edit Lock - Architecture Independent Code
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.0 -0500
> +++ linux-2.6-lttng/kernel/kprobes.c 2007-12-12
> 18:10:34.0 -0500
> @@ -644,7 +644,9 @@ valid_p:
> list_del_rcu(>list);
> kfree(old_p);
> }
> + mutex_lock(_mutex);
> arch_remove_kprobe(p);
> + mutex_unlock(_mutex);
> } else {
> mutex_lock(_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.0 -0500
> +++ linux-2.6-lttng/include/linux/memory.h2007-11-07
> 11:13:48.0 -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.0 -0500
> +++ linux-2.6-lttng/mm/memory.c 2007-11-07
> 11:14:25.0 -0500
> @@ -50,6 +50,8 @@
> #include
> #include
> #include
> +#include
> +#include
>
> #include
> #include
> @@ -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(_mutex);
> +}
> +EXPORT_SYMBOL_GPL(kernel_text_lock);
> +
> +/**
> + * kernel_text_unlock - Release the kernel text modification lock
> + *
> + * Insures mutual write exclusion of kernel and
RE: [patch 05/24] Text Edit Lock - Architecture Independent Code
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.0 -0500
+++ linux-2.6-lttng/kernel/kprobes.c 2007-12-12
18:10:34.0 -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.0 -0500
+++ linux-2.6-lttng/include/linux/memory.h2007-11-07
11:13:48.0 -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.0 -0500
+++ linux-2.6-lttng/mm/memory.c 2007-11-07
11:14:25.0 -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
RE: [patch 05/24] Text Edit Lock - Architecture Independent Code
===
--- linux-2.6-lttng.orig/kernel/kprobes.c 2007-12-12
18:10:32.0 -0500
+++ linux-2.6-lttng/kernel/kprobes.c2007-12-12
18:10:34.0 -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.
Could we insert the mutex_lock and mutex_unlock into free_insn_slot
instead of architecture code?
modify as follows:
void __kprobes free_insn_slot(kprobe_opcode_t * slot, int dirty)
{
struct kprobe_insn_page *kip;
struct hlist_node *pos;
+ mutex_lock(kprobe_mutex);
hlist_for_each_entry(kip, pos, kprobe_insn_pages, hlist) {
if (kip-insns = slot
slot kip-insns + (INSNS_PER_PAGE * MAX_INSN_SIZE)) {
int i = (slot - kip-insns) / MAX_INSN_SIZE;
if (dirty) {
kip-slot_used[i] = SLOT_DIRTY;
kip-ngarbage++;
} else {
collect_one_slot(kip, i);
}
break;
}
}
if (dirty ++kprobe_garbage_slots INSNS_PER_PAGE)
collect_garbage_slots();
+ mutex_unlock(kprobe_mutex);
}
-Original Message-
From: [EMAIL PROTECTED]
[mailto:[EMAIL PROTECTED] On Behalf Of zhangxiliang
Sent: Friday, December 21, 2007 1:19 PM
To: 'Mathieu Desnoyers'; [EMAIL PROTECTED]; 'Ingo
Molnar'; linux-kernel@vger.kernel.org
Cc: 'Andi Kleen'
Subject: RE: [patch 05/24] Text Edit Lock - Architecture
Independent Code
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.0 -0500
+++ linux-2.6-lttng/kernel/kprobes.c2007-12-12
18:10:34.0 -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
