* Re: [RFC][PATCH][kprobe] enabling booster on the preemptible kernel, take 2
@ 2006-11-01 17:01 bibo mao
2006-11-02 18:51 ` Masami Hiramatsu
0 siblings, 1 reply; 12+ messages in thread
From: bibo mao @ 2006-11-01 17:01 UTC (permalink / raw)
To: mingo
Cc: anil.s.keshavamurthy, ananth, prasanna, systemtap, soshima,
haoki, yumiko.sugita.yf
I am not familiar with freeze_processes(), I only view code.
And I write simple program(though buggy) to test:
---------------------------------------
struct task_struct *g, *p;
if (freeze_processes()) {
goto Thaw;
}
do_each_thread(g, p) {
if (frozen(p))
continue;
printk("%s not stopped\n", p->comm );
} while_each_thread(g, p);
Thaw:
thaw_processes();
------------------------------------
the output is this(except for current thread):
ksoftirqd/0 not stopped
watchdog/0 not stopped
events/0 not stopped
khelper not stopped
kthread not stopped
kblockd/0 not stopped
kacpid not stopped
aio/0 not stopped
xfslogd/0 not stopped
xfsdatad/0 not stopped
kpsmoused not stopped
ipw2100/0 not stopped
it seems that many threads are not frozen even freeze_processes
return 0.
thanks
bibo,mao
"Ingo Molnar" <mingo@redhat.com> wrote:
> On Tue, 2006-10-31 at 22:17 +0900, Masami Hiramatsu wrote:
>> OK, I see.
>> It seems problematic because the softirqd is PF_NOFREEZE and it
>> can execute most of functions...
>> I think we need to find a new way to solve this problem.
>
> could you outline the problem to me? freeze_processes() should be a
> generic facility to move all kernel processing into a 'known' context of
> execution. All the PF_NOFREEZE kernel threads are supposed to do
> periodic calls to try_to_freeze(). They should not (and most of the time
> they do not) prevent freezing of all state on the system.
>
> am i misunderstanding the problem?
>
> Ingo
>
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RFC][PATCH][kprobe] enabling booster on the preemptible kernel, take 2
2006-11-01 17:01 [RFC][PATCH][kprobe] enabling booster on the preemptible kernel, take 2 bibo mao
@ 2006-11-02 18:51 ` Masami Hiramatsu
2006-11-03 9:16 ` bibo,mao
0 siblings, 1 reply; 12+ messages in thread
From: Masami Hiramatsu @ 2006-11-02 18:51 UTC (permalink / raw)
To: bibo mao
Cc: mingo, anil.s.keshavamurthy, ananth, prasanna, systemtap,
soshima, haoki, yumiko.sugita.yf
Hi Bibo,
bibo mao wrote:
> I am not familiar with freeze_processes(), I only view code.
> And I write simple program(though buggy) to test:
[...]
> it seems that many threads are not frozen even freeze_processes
> return 0.
I think the most important issue is whether those threads are
preempted (suddenly scheduled out) or not.
Those preempted threads might be preempted on the instruction
buffer or on the middle of the target instructions. In that
case, we cannot free the buffer or overwrite a jump code.
But, if those threads are sleeping at the known places, we can
ensure safety of freeing/overwriting.
Therefore, I think it is necessary to check whether all
non-frozen threads are sleeping or not, like as below;
---------------------------------------
int check_safety(void)
{
int ret = 0;
struct task_struct *g, *p;
if (freeze_processes()) {
goto Thaw;
}
do_each_thread(g, p) {
if (frozen(p))
continue;
if (p->state == TASK_RUNNING) {
ret = -EAGAIN;
break;
}
} while_each_thread(g, p);
Thaw:
thaw_processes();
return ret;
}
Thanks,
--
Masami HIRAMATSU
Linux Technology Center
Hitachi, Ltd., Systems Development Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RFC][PATCH][kprobe] enabling booster on the preemptible kernel, take 2
2006-11-02 18:51 ` Masami Hiramatsu
@ 2006-11-03 9:16 ` bibo,mao
2006-11-06 18:52 ` [RFC][PATCH][kprobe] enabling booster on the preemptible kernel, take 3 Masami Hiramatsu
0 siblings, 1 reply; 12+ messages in thread
From: bibo,mao @ 2006-11-03 9:16 UTC (permalink / raw)
To: Masami Hiramatsu
Cc: bibo mao, mingo, Keshavamurthy, Anil S, ananth, prasanna,
systemtap, soshima, haoki, yumiko.sugita.yf
Masami Hiramatsu wrote:
> Hi Bibo,
>
> bibo mao wrote:
> > I am not familiar with freeze_processes(), I only view code.
> > And I write simple program(though buggy) to test:
> [...]
> > it seems that many threads are not frozen even freeze_processes
> > return 0.
>
> I think the most important issue is whether those threads are
> preempted (suddenly scheduled out) or not.
> Those preempted threads might be preempted on the instruction
> buffer or on the middle of the target instructions. In that
> case, we cannot free the buffer or overwrite a jump code.
> But, if those threads are sleeping at the known places, we can
> ensure safety of freeing/overwriting.
> Therefore, I think it is necessary to check whether all
> non-frozen threads are sleeping or not, like as below;
I think that will be ok, it will be safe to free jump buffer
at that time.
thanks
bibo,mao
> ---------------------------------------
> int check_safety(void)
> {
> int ret = 0;
> struct task_struct *g, *p;
> if (freeze_processes()) {
> goto Thaw;
> }
> do_each_thread(g, p) {
> if (frozen(p))
> continue;
> if (p->state == TASK_RUNNING) {
> ret = -EAGAIN;
> break;
> }
> } while_each_thread(g, p);
> Thaw:
> thaw_processes();
> return ret;
> }
>
>
> Thanks,
>
>
> --
> Masami HIRAMATSU
> Linux Technology Center
> Hitachi, Ltd., Systems Development Laboratory
> E-mail: masami.hiramatsu.pt@hitachi.com
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* [RFC][PATCH][kprobe] enabling booster on the preemptible kernel, take 3
2006-11-03 9:16 ` bibo,mao
@ 2006-11-06 18:52 ` Masami Hiramatsu
0 siblings, 0 replies; 12+ messages in thread
From: Masami Hiramatsu @ 2006-11-06 18:52 UTC (permalink / raw)
To: bibo,mao, mingo, Keshavamurthy, Anil S, ananth, prasanna
Cc: bibo mao, systemtap, soshima, haoki, yumiko.sugita.yf
Hi,
bibo,mao wrote:
> Masami Hiramatsu wrote:
>> I think the most important issue is whether those threads are
>> preempted (suddenly scheduled out) or not.
>> Those preempted threads might be preempted on the instruction
>> buffer or on the middle of the target instructions. In that
>> case, we cannot free the buffer or overwrite a jump code.
>> But, if those threads are sleeping at the known places, we can
>> ensure safety of freeing/overwriting.
>> Therefore, I think it is necessary to check whether all
>> non-frozen threads are sleeping or not, like as below;
> I think that will be ok, it will be safe to free jump buffer at that time.
Thanks,
I implemented new safety check routine following this idea.
--
Masami HIRAMATSU
2nd Research Dept.
Hitachi, Ltd., Systems Development Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com
When we are unregistering a kprobe-booster, we can't release
its buffer immediately on the preemptive kernel, because
some processes might be preempted on the buffer.
The freeze_processes() and thaw_processes() functions can
clean most of processes up from the buffer. There are still
some non-frozen threads who have the PF_NOFREEZE flag. If
those threads are sleeping at the known place outside the
buffer, we can ensure safety of freeing.
However, the processing of this check routine takes a long time.
So, this patch introduces the garbage collection mechanism
of insn_slot. It also introduces the "dirty" flag to
free_insn_slot because of efficiency.
The "clean" instruction slots (dirty flag is cleared) are
released immediately. But the "dirty" slots which are used
by boosted kprobes, are marked as garbages.
collect_garbage_slots() will be invoked to release "dirty"
slots if 1) there are more than INSNS_PER_PAGE garbage slots
or 2) there are no unused slots.
arch/i386/kernel/kprobes.c | 4 -
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 | 117 ++++++++++++++++++++++++++++++++++--------
7 files changed, 103 insertions(+), 28 deletions(-)
Index: linux-2.6.19-rc4-mm2/kernel/kprobes.c
===================================================================
--- linux-2.6.19-rc4-mm2.orig/kernel/kprobes.c
+++ linux-2.6.19-rc4-mm2/kernel/kprobes.c
@@ -38,6 +38,7 @@
#include <linux/module.h>
#include <linux/moduleloader.h>
#include <linux/kallsyms.h>
+#include <linux/freezer.h>
#include <asm-generic/sections.h>
#include <asm/cacheflush.h>
#include <asm/errno.h>
@@ -83,9 +84,36 @@ struct kprobe_insn_page {
kprobe_opcode_t *insns; /* Page of instruction slots */
char slot_used[INSNS_PER_PAGE];
int nused;
+ int ngarbage;
};
static struct hlist_head kprobe_insn_pages;
+static int kprobe_garbage_slots;
+static int collect_garbage_slots(void);
+
+static int __kprobes check_safety(void)
+{
+ int ret = 0;
+#if defined(CONFIG_PREEMPT) && defined(CONFIG_PM)
+ ret = freeze_processes();
+ if (ret == 0) {
+ struct task_struct *p, *q;
+ do_each_thread(p, q) {
+ if (p != current && p->state == TASK_RUNNING &&
+ p->pid != 0) {
+ printk("Check failed: %s is running\n", p->comm);
+ ret = -1;
+ goto loop_end;
+ }
+ } while_each_thread(p, q);
+ }
+loop_end:
+ thaw_processes();
+#else
+ synchronize_sched();
+#endif
+ return ret;
+}
/**
* get_insn_slot() - Find a slot on an executable page for an instruction.
@@ -96,6 +124,7 @@ kprobe_opcode_t __kprobes *get_insn_slot
struct kprobe_insn_page *kip;
struct hlist_node *pos;
+ retry:
hlist_for_each(pos, &kprobe_insn_pages) {
kip = hlist_entry(pos, struct kprobe_insn_page, hlist);
if (kip->nused < INSNS_PER_PAGE) {
@@ -112,7 +141,11 @@ kprobe_opcode_t __kprobes *get_insn_slot
}
}
- /* All out of space. Need to allocate a new page. Use slot 0.*/
+ /* If there are any garbage slots, collect it and try again. */
+ if (kprobe_garbage_slots && collect_garbage_slots() == 0) {
+ goto retry;
+ }
+ /* 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;
@@ -133,10 +166,62 @@ kprobe_opcode_t __kprobes *get_insn_slot
memset(kip->slot_used, 0, INSNS_PER_PAGE);
kip->slot_used[0] = 1;
kip->nused = 1;
+ kip->ngarbage = 0;
return kip->insns;
}
-void __kprobes free_insn_slot(kprobe_opcode_t *slot)
+/* Return 1 if all garbages are collected, otherwise 0. */
+static int __kprobes collect_one_slot(struct kprobe_insn_page *kip, int idx)
+{
+ kip->slot_used[idx] = 0;
+ kip->nused--;
+ if (kip->nused == 0) {
+ /*
+ * Page is no longer in use. Free it unless
+ * it's the last one. We keep the last one
+ * so as not to have to set it up again the
+ * next time somebody inserts a probe.
+ */
+ hlist_del(&kip->hlist);
+ if (hlist_empty(&kprobe_insn_pages)) {
+ INIT_HLIST_NODE(&kip->hlist);
+ hlist_add_head(&kip->hlist,
+ &kprobe_insn_pages);
+ } else {
+ module_free(NULL, kip->insns);
+ kfree(kip);
+ }
+ return 1;
+ }
+ return 0;
+}
+
+static int __kprobes collect_garbage_slots(void)
+{
+ struct kprobe_insn_page *kip;
+ struct hlist_node *pos, *next;
+
+ /* Ensure no-one is preepmted on the garbages */
+ if (check_safety() != 0)
+ return -EAGAIN;
+
+ hlist_for_each_safe(pos, next, &kprobe_insn_pages) {
+ int i;
+ kip = hlist_entry(pos, struct kprobe_insn_page, hlist);
+ if (kip->ngarbage == 0)
+ continue;
+ kip->ngarbage = 0; /* we will collect all garbages */
+ for (i = 0; i < INSNS_PER_PAGE; i++) {
+ if (kip->slot_used[i] == -1 &&
+ collect_one_slot(kip, i))
+ break;
+ }
+ }
+ kprobe_garbage_slots = 0;
+ return 0;
+}
+
+void __kprobes free_insn_slot(kprobe_opcode_t * slot, int dirty)
{
struct kprobe_insn_page *kip;
struct hlist_node *pos;
@@ -146,28 +231,18 @@ void __kprobes free_insn_slot(kprobe_opc
if (kip->insns <= slot &&
slot < kip->insns + (INSNS_PER_PAGE * MAX_INSN_SIZE)) {
int i = (slot - kip->insns) / MAX_INSN_SIZE;
- kip->slot_used[i] = 0;
- kip->nused--;
- if (kip->nused == 0) {
- /*
- * Page is no longer in use. Free it unless
- * it's the last one. We keep the last one
- * so as not to have to set it up again the
- * next time somebody inserts a probe.
- */
- hlist_del(&kip->hlist);
- if (hlist_empty(&kprobe_insn_pages)) {
- INIT_HLIST_NODE(&kip->hlist);
- hlist_add_head(&kip->hlist,
- &kprobe_insn_pages);
- } else {
- module_free(NULL, kip->insns);
- kfree(kip);
- }
+ if (dirty) {
+ kip->slot_used[i] = -1;
+ kip->ngarbage++;
+ } else {
+ collect_one_slot(kip, i);
}
- return;
+ break;
}
}
+ if (dirty && (++kprobe_garbage_slots > INSNS_PER_PAGE)) {
+ collect_garbage_slots();
+ }
}
#endif
Index: linux-2.6.19-rc4-mm2/arch/i386/kernel/kprobes.c
===================================================================
--- linux-2.6.19-rc4-mm2.orig/arch/i386/kernel/kprobes.c
+++ linux-2.6.19-rc4-mm2/arch/i386/kernel/kprobes.c
@@ -184,7 +184,7 @@ void __kprobes arch_disarm_kprobe(struct
void __kprobes arch_remove_kprobe(struct kprobe *p)
{
mutex_lock(&kprobe_mutex);
- free_insn_slot(p->ainsn.insn);
+ free_insn_slot(p->ainsn.insn, (p->ainsn.boostable == 1));
mutex_unlock(&kprobe_mutex);
}
@@ -333,7 +333,7 @@ static int __kprobes kprobe_handler(stru
return 1;
ss_probe:
-#ifndef CONFIG_PREEMPT
+#if !defined(CONFIG_PREEMPT) || defined(CONFIG_PM)
if (p->ainsn.boostable == 1 && !p->post_handler){
/* Boost up -- we can execute copied instructions directly */
reset_current_kprobe();
Index: linux-2.6.19-rc4-mm2/arch/ia64/kernel/kprobes.c
===================================================================
--- linux-2.6.19-rc4-mm2.orig/arch/ia64/kernel/kprobes.c
+++ linux-2.6.19-rc4-mm2/arch/ia64/kernel/kprobes.c
@@ -481,7 +481,7 @@ void __kprobes arch_disarm_kprobe(struct
void __kprobes arch_remove_kprobe(struct kprobe *p)
{
mutex_lock(&kprobe_mutex);
- free_insn_slot(p->ainsn.insn);
+ free_insn_slot(p->ainsn.insn, 0);
mutex_unlock(&kprobe_mutex);
}
/*
Index: linux-2.6.19-rc4-mm2/arch/powerpc/kernel/kprobes.c
===================================================================
--- linux-2.6.19-rc4-mm2.orig/arch/powerpc/kernel/kprobes.c
+++ linux-2.6.19-rc4-mm2/arch/powerpc/kernel/kprobes.c
@@ -85,7 +85,7 @@ void __kprobes arch_disarm_kprobe(struct
void __kprobes arch_remove_kprobe(struct kprobe *p)
{
mutex_lock(&kprobe_mutex);
- free_insn_slot(p->ainsn.insn);
+ free_insn_slot(p->ainsn.insn, 0);
mutex_unlock(&kprobe_mutex);
}
Index: linux-2.6.19-rc4-mm2/arch/s390/kernel/kprobes.c
===================================================================
--- linux-2.6.19-rc4-mm2.orig/arch/s390/kernel/kprobes.c
+++ linux-2.6.19-rc4-mm2/arch/s390/kernel/kprobes.c
@@ -200,7 +200,7 @@ void __kprobes arch_disarm_kprobe(struct
void __kprobes arch_remove_kprobe(struct kprobe *p)
{
mutex_lock(&kprobe_mutex);
- free_insn_slot(p->ainsn.insn);
+ free_insn_slot(p->ainsn.insn, 0);
mutex_unlock(&kprobe_mutex);
}
Index: linux-2.6.19-rc4-mm2/arch/x86_64/kernel/kprobes.c
===================================================================
--- linux-2.6.19-rc4-mm2.orig/arch/x86_64/kernel/kprobes.c
+++ linux-2.6.19-rc4-mm2/arch/x86_64/kernel/kprobes.c
@@ -224,7 +224,7 @@ void __kprobes arch_disarm_kprobe(struct
void __kprobes arch_remove_kprobe(struct kprobe *p)
{
mutex_lock(&kprobe_mutex);
- free_insn_slot(p->ainsn.insn);
+ free_insn_slot(p->ainsn.insn, 0);
mutex_unlock(&kprobe_mutex);
}
Index: linux-2.6.19-rc4-mm2/include/linux/kprobes.h
===================================================================
--- linux-2.6.19-rc4-mm2.orig/include/linux/kprobes.h
+++ linux-2.6.19-rc4-mm2/include/linux/kprobes.h
@@ -165,7 +165,7 @@ extern void arch_disarm_kprobe(struct kp
extern int arch_init_kprobes(void);
extern void show_registers(struct pt_regs *regs);
extern kprobe_opcode_t *get_insn_slot(void);
-extern void free_insn_slot(kprobe_opcode_t *slot);
+extern void free_insn_slot(kprobe_opcode_t *slot, int dirty);
extern void kprobes_inc_nmissed_count(struct kprobe *p);
/* Get the kprobe at this addr (if any) - called with preemption disabled */
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RFC][PATCH][kprobe] enabling booster on the preemptible kernel, take 2
2006-10-31 14:13 ` Ingo Molnar
@ 2006-10-31 16:39 ` Masami Hiramatsu
0 siblings, 0 replies; 12+ messages in thread
From: Masami Hiramatsu @ 2006-10-31 16:39 UTC (permalink / raw)
To: Ingo Molnar
Cc: bibo,mao, bibo,mao, Keshavamurthy, Anil S,
Ananth N Mavinakayanahalli, Prasanna S Panchamukhi, SystemTAP,
Satoshi Oshima, Hideo Aoki, Yumiko Sugita
Hi Ingo,
Ingo Molnar wrote:
> another thought:
>
> would it help if we extended the 'soft lockup watchdog' with periodic
> 'try whether the whole system can be frozen' checks? That would make
> testing coverage alot wider, and it would also give kernel developers an
> easy way to make sure the kernel is still freezable. (instead of having
> to try something like sw-suspend explicitly)
It seems useful to me, I'll check that.
Thank you!
--
Masami HIRAMATSU
Linux Technology Center
Hitachi, Ltd., Systems Development Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RFC][PATCH][kprobe] enabling booster on the preemptible kernel, take 2
2006-10-31 13:47 ` Masami Hiramatsu
2006-10-31 13:49 ` Ingo Molnar
@ 2006-10-31 14:13 ` Ingo Molnar
2006-10-31 16:39 ` Masami Hiramatsu
1 sibling, 1 reply; 12+ messages in thread
From: Ingo Molnar @ 2006-10-31 14:13 UTC (permalink / raw)
To: Masami Hiramatsu
Cc: bibo,mao, bibo,mao, Keshavamurthy, Anil S,
Ananth N Mavinakayanahalli, Prasanna S Panchamukhi, SystemTAP,
Satoshi Oshima, Hideo Aoki, Yumiko Sugita
another thought:
would it help if we extended the 'soft lockup watchdog' with periodic
'try whether the whole system can be frozen' checks? That would make
testing coverage alot wider, and it would also give kernel developers an
easy way to make sure the kernel is still freezable. (instead of having
to try something like sw-suspend explicitly)
Ingo
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RFC][PATCH][kprobe] enabling booster on the preemptible kernel, take 2
2006-10-31 13:47 ` Masami Hiramatsu
@ 2006-10-31 13:49 ` Ingo Molnar
2006-10-31 14:13 ` Ingo Molnar
1 sibling, 0 replies; 12+ messages in thread
From: Ingo Molnar @ 2006-10-31 13:49 UTC (permalink / raw)
To: Masami Hiramatsu
Cc: bibo,mao, bibo,mao, Keshavamurthy, Anil S,
Ananth N Mavinakayanahalli, Prasanna S Panchamukhi, SystemTAP,
Satoshi Oshima, Hideo Aoki, Yumiko Sugita
On Tue, 2006-10-31 at 22:17 +0900, Masami Hiramatsu wrote:
> OK, I see.
> It seems problematic because the softirqd is PF_NOFREEZE and it
> can execute most of functions...
> I think we need to find a new way to solve this problem.
could you outline the problem to me? freeze_processes() should be a
generic facility to move all kernel processing into a 'known' context of
execution. All the PF_NOFREEZE kernel threads are supposed to do
periodic calls to try_to_freeze(). They should not (and most of the time
they do not) prevent freezing of all state on the system.
am i misunderstanding the problem?
Ingo
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RFC][PATCH][kprobe] enabling booster on the preemptible kernel, take 2
2006-10-31 9:14 ` bibo,mao
@ 2006-10-31 13:47 ` Masami Hiramatsu
2006-10-31 13:49 ` Ingo Molnar
2006-10-31 14:13 ` Ingo Molnar
0 siblings, 2 replies; 12+ messages in thread
From: Masami Hiramatsu @ 2006-10-31 13:47 UTC (permalink / raw)
To: bibo,mao
Cc: bibo,mao, Keshavamurthy, Anil S, Ananth N Mavinakayanahalli,
Prasanna S Panchamukhi, Ingo Molnar, SystemTAP, Satoshi Oshima,
Hideo Aoki, Yumiko Sugita
Hi bibo,
bibo,mao wrote:
>>> I do not know whether there exists non-freezeable and preemptive kernel
>>> thread, if there exist then this thread will not be frozen.
>>
>> In that case, freeze_processes() returns the positive value which
>> means how many processes are not frozen. If freeze_processes()
>> returns non-zero, this function aborts the garbage collection.
>>
> But from the code, return value of freeze_processes() represents how many
> processes can be frozen but are not frozen. I grep the kernel code, there
> still exists many processes which flag is PF_NOFREEZE.
> I think if current probed thread is PF_NOFREEZE, then kprobe_handler
> need skip the bootser.
OK, I see.
It seems problematic because the softirqd is PF_NOFREEZE and it
can execute most of functions...
I think we need to find a new way to solve this problem.
Thank you for your advice.
--
Masami HIRAMATSU
Linux Technology Center
Hitachi, Ltd., Systems Development Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RFC][PATCH][kprobe] enabling booster on the preemptible kernel, take 2
2006-10-30 14:07 ` Masami Hiramatsu
@ 2006-10-31 9:14 ` bibo,mao
2006-10-31 13:47 ` Masami Hiramatsu
0 siblings, 1 reply; 12+ messages in thread
From: bibo,mao @ 2006-10-31 9:14 UTC (permalink / raw)
To: Masami Hiramatsu
Cc: bibo,mao, Keshavamurthy, Anil S, Ananth N Mavinakayanahalli,
Prasanna S Panchamukhi, Ingo Molnar, SystemTAP, Satoshi Oshima,
Hideo Aoki, Yumiko Sugita
Masami Hiramatsu wrote:
> Hi bibo,
>
> Thank you for your review!
>
> bibo,mao wrote:
>> This patch will boost kprobe on preemptible kernel, I think
>> it is deserved to waster some memory for better performance
>> by deferring memory free after freeze_processes.
>
> I think it doesn't waste memory so much, because it tries
> to reuse garbage memories before the kernel allocates an
> additional page.
>
> [...]
>>> +static int __kprobes collect_garbage_slots(void)
>>> +{
>>> + struct kprobe_insn_page *kip;
>>> + struct hlist_node *pos, *next;
>>> + int ret = -1;
>>> +
>>> +#if defined(CONFIG_PREEMPT) && defined(CONFIG_PM)
>>> + /* Ensure no-one is preepmted on the garbages */
>>> + if (freeze_processes() != 0)
>> I do not know whether there exists non-freezeable and preemptive kernel
>> thread, if there exist then this thread will not be frozen.
>
> In that case, freeze_processes() returns the positive value which
> means how many processes are not frozen. If freeze_processes()
> returns non-zero, this function aborts the garbage collection.
>
But from the code, return value of freeze_processes() represents how many
processes can be frozen but are not frozen. I grep the kernel code, there
still exists many processes which flag is PF_NOFREEZE.
I think if current probed thread is PF_NOFREEZE, then kprobe_handler need
skip the bootser.
thanks
bibo,mao
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RFC][PATCH][kprobe] enabling booster on the preemptible kernel, take 2
2006-10-30 6:37 ` bibo,mao
@ 2006-10-30 14:07 ` Masami Hiramatsu
2006-10-31 9:14 ` bibo,mao
0 siblings, 1 reply; 12+ messages in thread
From: Masami Hiramatsu @ 2006-10-30 14:07 UTC (permalink / raw)
To: bibo,mao
Cc: Keshavamurthy, Anil S, Ananth N Mavinakayanahalli,
Prasanna S Panchamukhi, Ingo Molnar, SystemTAP, Satoshi Oshima,
Hideo Aoki, Yumiko Sugita
Hi bibo,
Thank you for your review!
bibo,mao wrote:
> This patch will boost kprobe on preemptible kernel, I think
> it is deserved to waster some memory for better performance
> by deferring memory free after freeze_processes.
I think it doesn't waste memory so much, because it tries
to reuse garbage memories before the kernel allocates an
additional page.
[...]
>> +static int __kprobes collect_garbage_slots(void)
>> +{
>> + struct kprobe_insn_page *kip;
>> + struct hlist_node *pos, *next;
>> + int ret = -1;
>> +
>> +#if defined(CONFIG_PREEMPT) && defined(CONFIG_PM)
>> + /* Ensure no-one is preepmted on the garbages */
>> + if (freeze_processes() != 0)
> I do not know whether there exists non-freezeable and preemptive kernel
> thread, if there exist then this thread will not be frozen.
In that case, freeze_processes() returns the positive value which
means how many processes are not frozen. If freeze_processes()
returns non-zero, this function aborts the garbage collection.
>> + goto thaw_all;
>> +#endif
>> + hlist_for_each_safe(pos, next, &kprobe_insn_pages) {
>> + int i;
>> + kip = hlist_entry(pos, struct kprobe_insn_page, hlist);
>> + if (kip->ngarbage == 0)
>> + continue;
>> + kip->ngarbage = 0; /* we will collect all
>> garbages */
>> + for (i = 0; i < INSNS_PER_PAGE; i++) {
>> + if (kip->slot_used[i] == -1 &&
>> + collect_one_slot(kip, i))
> if collect_one_slot executes kfree(kip) and return 0, then kernel will
> continue
> execute the for () loop sentence and access freed kip point by
> kip->slot_used.
Exactly, it's a bug.
Thank you. I'll fix that.
>> @@ -146,28 +215,18 @@
>> if (kip->insns <= slot &&
>> slot < kip->insns + (INSNS_PER_PAGE *
>> MAX_INSN_SIZE)) {
>> int i = (slot - kip->insns) / MAX_INSN_SIZE;
>> - kip->slot_used[i] = 0;
>> - kip->nused--;
>> - if (kip->nused == 0) {
>> - /*
>> - * Page is no longer in use. Free it
>> unless
>> - * it's the last one. We keep the
>> last one
>> - * so as not to have to set it up
>> again the
>> - * next time somebody inserts a probe.
>> - */
>> - hlist_del(&kip->hlist);
>> - if (hlist_empty(&kprobe_insn_pages)) {
>> - INIT_HLIST_NODE(&kip->hlist);
>> - hlist_add_head(&kip->hlist,
>> - &kprobe_insn_pages);
>> - } else {
>> - module_free(NULL, kip->insns);
>> - kfree(kip);
>> - }
>> + if (dirty) {
>> + kip->slot_used[i] = -1;
>> + kip->ngarbage++;
> it seems that break sentence is missing.
Oh, it's my mistake. Thanks.
>> + } else {
>> + collect_one_slot(kip, i);
>> + break;
>> }
>> - return;
So, I will add a break here.
Best regards,
--
Masami HIRAMATSU
Linux Technology Center
Hitachi, Ltd., Systems Development Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RFC][PATCH][kprobe] enabling booster on the preemptible kernel, take 2
2006-10-16 13:14 [RFC][PATCH][kprobe] enabling booster on the preemptible kernel, take 2 Masami Hiramatsu
@ 2006-10-30 6:37 ` bibo,mao
2006-10-30 14:07 ` Masami Hiramatsu
0 siblings, 1 reply; 12+ messages in thread
From: bibo,mao @ 2006-10-30 6:37 UTC (permalink / raw)
To: Masami Hiramatsu
Cc: Keshavamurthy, Anil S, Ananth N Mavinakayanahalli,
Prasanna S Panchamukhi, Ingo Molnar, SystemTAP, Satoshi Oshima,
Hideo Aoki, Yumiko Sugita
This patch will boost kprobe on preemptible kernel, I think
it is deserved to waster some memory for better performance
by deferring memory free after freeze_processes.
thanks
bibo,mao
Masami Hiramatsu wrote:
> Hi,
>
> Here is the patch which enables kprobe-booster on
> the preemptive kernel.
>
> When we are unregistering a kprobe-booster, we can't release
> its buffer immediately on the preemptive kernel, because
> some processes might be preempted on the buffer.
> The freeze_processes() and thaw_processes() functions can
> clean those processes up from the buffer. However, the
> processing of those functions takes a long time.
> So, this patch introduces the garbage collection mechanism
> of insn_slot. It also introduces the "dirty" flag to
> free_insn_slot because of efficiency.
>
> The "clean" instruction slots (dirty flag is cleared) are
> released immediately. But the "dirty" slots which are used
> by boosted kprobes, are marked as garbages.
> collect_garbage_slots() will be invoked to release "dirty"
> slots if 1) there are more than INSNS_PER_PAGE garbage slots
> or 2) there are no unused slots.
>
> Thanks,
> --
> Masami HIRAMATSU
> 2nd Research Dept.
> Hitachi, Ltd., Systems Development Laboratory
> E-mail: masami.hiramatsu.pt@hitachi.com
>
>
> ---
> arch/i386/kernel/kprobes.c | 4 -
> 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 | 101 +++++++++++++++++++++++++++++++++---------
> 7 files changed, 87 insertions(+), 28 deletions(-)
>
> Index: linux-2.6.19-rc1-mm1/kernel/kprobes.c
> ===================================================================
> --- linux-2.6.19-rc1-mm1.orig/kernel/kprobes.c 2006-10-16 10:40:02.000000000 +0900
> +++ linux-2.6.19-rc1-mm1/kernel/kprobes.c 2006-10-16 21:50:44.000000000 +0900
> @@ -38,6 +38,7 @@
> #include <linux/module.h>
> #include <linux/moduleloader.h>
> #include <linux/kallsyms.h>
> +#include <linux/sched.h>
> #include <asm-generic/sections.h>
> #include <asm/cacheflush.h>
> #include <asm/errno.h>
> @@ -83,9 +84,12 @@
> kprobe_opcode_t *insns; /* Page of instruction slots */
> char slot_used[INSNS_PER_PAGE];
> int nused;
> + int ngarbage;
> };
>
> static struct hlist_head kprobe_insn_pages;
> +static int kprobe_garbage_slots;
> +static int collect_garbage_slots(void);
>
> /**
> * get_insn_slot() - Find a slot on an executable page for an instruction.
> @@ -96,6 +100,7 @@
> struct kprobe_insn_page *kip;
> struct hlist_node *pos;
>
> + retry:
> hlist_for_each(pos, &kprobe_insn_pages) {
> kip = hlist_entry(pos, struct kprobe_insn_page, hlist);
> if (kip->nused < INSNS_PER_PAGE) {
> @@ -112,7 +117,11 @@
> }
> }
>
> - /* All out of space. Need to allocate a new page. Use slot 0.*/
> + /* If there are any garbage slots, collect it and try again. */
> + if (kprobe_garbage_slots && collect_garbage_slots() == 0) {
I am not familiar with freeze_processes/thaw_process, but I only think that
it will bring performance downgrade greatly for the moment.I think kmalloc
method is better than collect_garbage_slots.
> + goto retry;
> + }
> + /* 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;
> @@ -133,10 +142,70 @@
> memset(kip->slot_used, 0, INSNS_PER_PAGE);
> kip->slot_used[0] = 1;
> kip->nused = 1;
> + kip->ngarbage = 0;
> return kip->insns;
> }
>
> -void __kprobes free_insn_slot(kprobe_opcode_t *slot)
> +/* Return 1 if all garbages are collected, otherwise 0. */
> +static int __kprobes collect_one_slot(struct kprobe_insn_page *kip, int idx)
> +{
> + kip->slot_used[idx] = 0;
> + kip->nused--;
> + if (kip->nused == 0) {
> + /*
> + * Page is no longer in use. Free it unless
> + * it's the last one. We keep the last one
> + * so as not to have to set it up again the
> + * next time somebody inserts a probe.
> + */
> + hlist_del(&kip->hlist);
> + if (hlist_empty(&kprobe_insn_pages)) {
> + INIT_HLIST_NODE(&kip->hlist);
> + hlist_add_head(&kip->hlist,
> + &kprobe_insn_pages);
> + return 1;
> + } else {
> + module_free(NULL, kip->insns);
> + kfree(kip);
> + }
> + }
> + return 0;
> +}
> +
> +static int __kprobes collect_garbage_slots(void)
> +{
> + struct kprobe_insn_page *kip;
> + struct hlist_node *pos, *next;
> + int ret = -1;
> +
> +#if defined(CONFIG_PREEMPT) && defined(CONFIG_PM)
> + /* Ensure no-one is preepmted on the garbages */
> + if (freeze_processes() != 0)
I do not know whether there exists non-freezeable and preemptive
kernel thread, if there exist then this thread will not be frozen.
> + goto thaw_all;
> +#endif
> + hlist_for_each_safe(pos, next, &kprobe_insn_pages) {
> + int i;
> + kip = hlist_entry(pos, struct kprobe_insn_page, hlist);
> + if (kip->ngarbage == 0)
> + continue;
> + kip->ngarbage = 0; /* we will collect all garbages */
> + for (i = 0; i < INSNS_PER_PAGE; i++) {
> + if (kip->slot_used[i] == -1 &&
> + collect_one_slot(kip, i))
if collect_one_slot executes kfree(kip) and return 0, then kernel will continue
execute the for () loop sentence and access freed kip point by kip->slot_used.
> + goto collected;
> + }
> + }
> + collected:
> + kprobe_garbage_slots = 0;
> + ret = 0;
> +#if defined(CONFIG_PREEMPT) && defined(CONFIG_PM)
> + thaw_all:
> + thaw_processes();
> +#endif
> + return ret;
> +}
> +
> +void __kprobes free_insn_slot(kprobe_opcode_t * slot, int dirty)
> {
> struct kprobe_insn_page *kip;
> struct hlist_node *pos;
> @@ -146,28 +215,18 @@
> if (kip->insns <= slot &&
> slot < kip->insns + (INSNS_PER_PAGE * MAX_INSN_SIZE)) {
> int i = (slot - kip->insns) / MAX_INSN_SIZE;
> - kip->slot_used[i] = 0;
> - kip->nused--;
> - if (kip->nused == 0) {
> - /*
> - * Page is no longer in use. Free it unless
> - * it's the last one. We keep the last one
> - * so as not to have to set it up again the
> - * next time somebody inserts a probe.
> - */
> - hlist_del(&kip->hlist);
> - if (hlist_empty(&kprobe_insn_pages)) {
> - INIT_HLIST_NODE(&kip->hlist);
> - hlist_add_head(&kip->hlist,
> - &kprobe_insn_pages);
> - } else {
> - module_free(NULL, kip->insns);
> - kfree(kip);
> - }
> + if (dirty) {
> + kip->slot_used[i] = -1;
> + kip->ngarbage++;
it seems that break sentence is missing.
> + } else {
> + collect_one_slot(kip, i);
> + break;
> }
> - return;
> }
> }
> + if (dirty && (++kprobe_garbage_slots > INSNS_PER_PAGE)) {
> + collect_garbage_slots();
> + }
> }
> #endif
>
> Index: linux-2.6.19-rc1-mm1/arch/i386/kernel/kprobes.c
> ===================================================================
> --- linux-2.6.19-rc1-mm1.orig/arch/i386/kernel/kprobes.c 2006-10-16
> 10:40:00.000000000 +0900
> +++ linux-2.6.19-rc1-mm1/arch/i386/kernel/kprobes.c 2006-10-16
> 21:43:03.000000000 +0900
> @@ -184,7 +184,7 @@
> void __kprobes arch_remove_kprobe(struct kprobe *p)
> {
> mutex_lock(&kprobe_mutex);
> - free_insn_slot(p->ainsn.insn);
> + free_insn_slot(p->ainsn.insn, (p->ainsn.boostable == 1));
> mutex_unlock(&kprobe_mutex);
> }
>
> @@ -333,7 +333,7 @@
> return 1;
>
> ss_probe:
> -#ifndef CONFIG_PREEMPT
> +#if !defined(CONFIG_PREEMPT) || defined(CONFIG_PM)
> if (p->ainsn.boostable == 1 && !p->post_handler){
> /* Boost up -- we can execute copied instructions directly */
> reset_current_kprobe();
> Index: linux-2.6.19-rc1-mm1/arch/ia64/kernel/kprobes.c
> ===================================================================
> --- linux-2.6.19-rc1-mm1.orig/arch/ia64/kernel/kprobes.c 2006-10-16
> 10:40:00.000000000 +0900
> +++ linux-2.6.19-rc1-mm1/arch/ia64/kernel/kprobes.c 2006-10-16
> 10:54:09.000000000 +0900
> @@ -481,7 +481,7 @@
> void __kprobes arch_remove_kprobe(struct kprobe *p)
> {
> mutex_lock(&kprobe_mutex);
> - free_insn_slot(p->ainsn.insn);
> + free_insn_slot(p->ainsn.insn, 0);
> mutex_unlock(&kprobe_mutex);
> }
> /*
> Index: linux-2.6.19-rc1-mm1/arch/powerpc/kernel/kprobes.c
> ===================================================================
> --- linux-2.6.19-rc1-mm1.orig/arch/powerpc/kernel/kprobes.c 2006-10-16
> 10:40:00.000000000 +0900
> +++ linux-2.6.19-rc1-mm1/arch/powerpc/kernel/kprobes.c 2006-10-16
> 10:54:09.000000000 +0900
> @@ -85,7 +85,7 @@
> void __kprobes arch_remove_kprobe(struct kprobe *p)
> {
> mutex_lock(&kprobe_mutex);
> - free_insn_slot(p->ainsn.insn);
> + free_insn_slot(p->ainsn.insn, 0);
> mutex_unlock(&kprobe_mutex);
> }
>
> Index: linux-2.6.19-rc1-mm1/arch/s390/kernel/kprobes.c
> ===================================================================
> --- linux-2.6.19-rc1-mm1.orig/arch/s390/kernel/kprobes.c 2006-10-16
> 10:40:00.000000000 +0900
> +++ linux-2.6.19-rc1-mm1/arch/s390/kernel/kprobes.c 2006-10-16
> 10:54:09.000000000 +0900
> @@ -200,7 +200,7 @@
> void __kprobes arch_remove_kprobe(struct kprobe *p)
> {
> mutex_lock(&kprobe_mutex);
> - free_insn_slot(p->ainsn.insn);
> + free_insn_slot(p->ainsn.insn, 0);
> mutex_unlock(&kprobe_mutex);
> }
>
> Index: linux-2.6.19-rc1-mm1/arch/x86_64/kernel/kprobes.c
> ===================================================================
> --- linux-2.6.19-rc1-mm1.orig/arch/x86_64/kernel/kprobes.c 2006-10-16
> 10:40:00.000000000 +0900
> +++ linux-2.6.19-rc1-mm1/arch/x86_64/kernel/kprobes.c 2006-10-16
> 10:54:09.000000000 +0900
> @@ -224,7 +224,7 @@
> void __kprobes arch_remove_kprobe(struct kprobe *p)
> {
> mutex_lock(&kprobe_mutex);
> - free_insn_slot(p->ainsn.insn);
> + free_insn_slot(p->ainsn.insn, 0);
> mutex_unlock(&kprobe_mutex);
> }
>
> Index: linux-2.6.19-rc1-mm1/include/linux/kprobes.h
> ===================================================================
> --- linux-2.6.19-rc1-mm1.orig/include/linux/kprobes.h 2006-10-16
> 10:40:02.000000000 +0900
> +++ linux-2.6.19-rc1-mm1/include/linux/kprobes.h 2006-10-16
> 21:43:07.000000000 +0900
> @@ -165,7 +165,7 @@
> extern int arch_init_kprobes(void);
> extern void show_registers(struct pt_regs *regs);
> extern kprobe_opcode_t *get_insn_slot(void);
> -extern void free_insn_slot(kprobe_opcode_t *slot);
> +extern void free_insn_slot(kprobe_opcode_t *slot, int dirty);
> extern void kprobes_inc_nmissed_count(struct kprobe *p);
>
> /* Get the kprobe at this addr (if any) - called with preemption disabled */
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* [RFC][PATCH][kprobe] enabling booster on the preemptible kernel, take 2
@ 2006-10-16 13:14 Masami Hiramatsu
2006-10-30 6:37 ` bibo,mao
0 siblings, 1 reply; 12+ messages in thread
From: Masami Hiramatsu @ 2006-10-16 13:14 UTC (permalink / raw)
To: Keshavamurthy, Anil S, Ananth N Mavinakayanahalli,
Prasanna S Panchamukhi, Ingo Molnar
Cc: SystemTAP, Satoshi Oshima, Hideo Aoki, Yumiko Sugita
Hi,
Here is the patch which enables kprobe-booster on
the preemptive kernel.
When we are unregistering a kprobe-booster, we can't release
its buffer immediately on the preemptive kernel, because
some processes might be preempted on the buffer.
The freeze_processes() and thaw_processes() functions can
clean those processes up from the buffer. However, the
processing of those functions takes a long time.
So, this patch introduces the garbage collection mechanism
of insn_slot. It also introduces the "dirty" flag to
free_insn_slot because of efficiency.
The "clean" instruction slots (dirty flag is cleared) are
released immediately. But the "dirty" slots which are used
by boosted kprobes, are marked as garbages.
collect_garbage_slots() will be invoked to release "dirty"
slots if 1) there are more than INSNS_PER_PAGE garbage slots
or 2) there are no unused slots.
Thanks,
--
Masami HIRAMATSU
2nd Research Dept.
Hitachi, Ltd., Systems Development Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com
---
arch/i386/kernel/kprobes.c | 4 -
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 | 101 +++++++++++++++++++++++++++++++++---------
7 files changed, 87 insertions(+), 28 deletions(-)
Index: linux-2.6.19-rc1-mm1/kernel/kprobes.c
===================================================================
--- linux-2.6.19-rc1-mm1.orig/kernel/kprobes.c 2006-10-16 10:40:02.000000000 +0900
+++ linux-2.6.19-rc1-mm1/kernel/kprobes.c 2006-10-16 21:50:44.000000000 +0900
@@ -38,6 +38,7 @@
#include <linux/module.h>
#include <linux/moduleloader.h>
#include <linux/kallsyms.h>
+#include <linux/sched.h>
#include <asm-generic/sections.h>
#include <asm/cacheflush.h>
#include <asm/errno.h>
@@ -83,9 +84,12 @@
kprobe_opcode_t *insns; /* Page of instruction slots */
char slot_used[INSNS_PER_PAGE];
int nused;
+ int ngarbage;
};
static struct hlist_head kprobe_insn_pages;
+static int kprobe_garbage_slots;
+static int collect_garbage_slots(void);
/**
* get_insn_slot() - Find a slot on an executable page for an instruction.
@@ -96,6 +100,7 @@
struct kprobe_insn_page *kip;
struct hlist_node *pos;
+ retry:
hlist_for_each(pos, &kprobe_insn_pages) {
kip = hlist_entry(pos, struct kprobe_insn_page, hlist);
if (kip->nused < INSNS_PER_PAGE) {
@@ -112,7 +117,11 @@
}
}
- /* All out of space. Need to allocate a new page. Use slot 0.*/
+ /* If there are any garbage slots, collect it and try again. */
+ if (kprobe_garbage_slots && collect_garbage_slots() == 0) {
+ goto retry;
+ }
+ /* 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;
@@ -133,10 +142,70 @@
memset(kip->slot_used, 0, INSNS_PER_PAGE);
kip->slot_used[0] = 1;
kip->nused = 1;
+ kip->ngarbage = 0;
return kip->insns;
}
-void __kprobes free_insn_slot(kprobe_opcode_t *slot)
+/* Return 1 if all garbages are collected, otherwise 0. */
+static int __kprobes collect_one_slot(struct kprobe_insn_page *kip, int idx)
+{
+ kip->slot_used[idx] = 0;
+ kip->nused--;
+ if (kip->nused == 0) {
+ /*
+ * Page is no longer in use. Free it unless
+ * it's the last one. We keep the last one
+ * so as not to have to set it up again the
+ * next time somebody inserts a probe.
+ */
+ hlist_del(&kip->hlist);
+ if (hlist_empty(&kprobe_insn_pages)) {
+ INIT_HLIST_NODE(&kip->hlist);
+ hlist_add_head(&kip->hlist,
+ &kprobe_insn_pages);
+ return 1;
+ } else {
+ module_free(NULL, kip->insns);
+ kfree(kip);
+ }
+ }
+ return 0;
+}
+
+static int __kprobes collect_garbage_slots(void)
+{
+ struct kprobe_insn_page *kip;
+ struct hlist_node *pos, *next;
+ int ret = -1;
+
+#if defined(CONFIG_PREEMPT) && defined(CONFIG_PM)
+ /* Ensure no-one is preepmted on the garbages */
+ if (freeze_processes() != 0)
+ goto thaw_all;
+#endif
+ hlist_for_each_safe(pos, next, &kprobe_insn_pages) {
+ int i;
+ kip = hlist_entry(pos, struct kprobe_insn_page, hlist);
+ if (kip->ngarbage == 0)
+ continue;
+ kip->ngarbage = 0; /* we will collect all garbages */
+ for (i = 0; i < INSNS_PER_PAGE; i++) {
+ if (kip->slot_used[i] == -1 &&
+ collect_one_slot(kip, i))
+ goto collected;
+ }
+ }
+ collected:
+ kprobe_garbage_slots = 0;
+ ret = 0;
+#if defined(CONFIG_PREEMPT) && defined(CONFIG_PM)
+ thaw_all:
+ thaw_processes();
+#endif
+ return ret;
+}
+
+void __kprobes free_insn_slot(kprobe_opcode_t * slot, int dirty)
{
struct kprobe_insn_page *kip;
struct hlist_node *pos;
@@ -146,28 +215,18 @@
if (kip->insns <= slot &&
slot < kip->insns + (INSNS_PER_PAGE * MAX_INSN_SIZE)) {
int i = (slot - kip->insns) / MAX_INSN_SIZE;
- kip->slot_used[i] = 0;
- kip->nused--;
- if (kip->nused == 0) {
- /*
- * Page is no longer in use. Free it unless
- * it's the last one. We keep the last one
- * so as not to have to set it up again the
- * next time somebody inserts a probe.
- */
- hlist_del(&kip->hlist);
- if (hlist_empty(&kprobe_insn_pages)) {
- INIT_HLIST_NODE(&kip->hlist);
- hlist_add_head(&kip->hlist,
- &kprobe_insn_pages);
- } else {
- module_free(NULL, kip->insns);
- kfree(kip);
- }
+ if (dirty) {
+ kip->slot_used[i] = -1;
+ kip->ngarbage++;
+ } else {
+ collect_one_slot(kip, i);
+ break;
}
- return;
}
}
+ if (dirty && (++kprobe_garbage_slots > INSNS_PER_PAGE)) {
+ collect_garbage_slots();
+ }
}
#endif
Index: linux-2.6.19-rc1-mm1/arch/i386/kernel/kprobes.c
===================================================================
--- linux-2.6.19-rc1-mm1.orig/arch/i386/kernel/kprobes.c 2006-10-16 10:40:00.000000000 +0900
+++ linux-2.6.19-rc1-mm1/arch/i386/kernel/kprobes.c 2006-10-16 21:43:03.000000000 +0900
@@ -184,7 +184,7 @@
void __kprobes arch_remove_kprobe(struct kprobe *p)
{
mutex_lock(&kprobe_mutex);
- free_insn_slot(p->ainsn.insn);
+ free_insn_slot(p->ainsn.insn, (p->ainsn.boostable == 1));
mutex_unlock(&kprobe_mutex);
}
@@ -333,7 +333,7 @@
return 1;
ss_probe:
-#ifndef CONFIG_PREEMPT
+#if !defined(CONFIG_PREEMPT) || defined(CONFIG_PM)
if (p->ainsn.boostable == 1 && !p->post_handler){
/* Boost up -- we can execute copied instructions directly */
reset_current_kprobe();
Index: linux-2.6.19-rc1-mm1/arch/ia64/kernel/kprobes.c
===================================================================
--- linux-2.6.19-rc1-mm1.orig/arch/ia64/kernel/kprobes.c 2006-10-16 10:40:00.000000000 +0900
+++ linux-2.6.19-rc1-mm1/arch/ia64/kernel/kprobes.c 2006-10-16 10:54:09.000000000 +0900
@@ -481,7 +481,7 @@
void __kprobes arch_remove_kprobe(struct kprobe *p)
{
mutex_lock(&kprobe_mutex);
- free_insn_slot(p->ainsn.insn);
+ free_insn_slot(p->ainsn.insn, 0);
mutex_unlock(&kprobe_mutex);
}
/*
Index: linux-2.6.19-rc1-mm1/arch/powerpc/kernel/kprobes.c
===================================================================
--- linux-2.6.19-rc1-mm1.orig/arch/powerpc/kernel/kprobes.c 2006-10-16 10:40:00.000000000 +0900
+++ linux-2.6.19-rc1-mm1/arch/powerpc/kernel/kprobes.c 2006-10-16 10:54:09.000000000 +0900
@@ -85,7 +85,7 @@
void __kprobes arch_remove_kprobe(struct kprobe *p)
{
mutex_lock(&kprobe_mutex);
- free_insn_slot(p->ainsn.insn);
+ free_insn_slot(p->ainsn.insn, 0);
mutex_unlock(&kprobe_mutex);
}
Index: linux-2.6.19-rc1-mm1/arch/s390/kernel/kprobes.c
===================================================================
--- linux-2.6.19-rc1-mm1.orig/arch/s390/kernel/kprobes.c 2006-10-16 10:40:00.000000000 +0900
+++ linux-2.6.19-rc1-mm1/arch/s390/kernel/kprobes.c 2006-10-16 10:54:09.000000000 +0900
@@ -200,7 +200,7 @@
void __kprobes arch_remove_kprobe(struct kprobe *p)
{
mutex_lock(&kprobe_mutex);
- free_insn_slot(p->ainsn.insn);
+ free_insn_slot(p->ainsn.insn, 0);
mutex_unlock(&kprobe_mutex);
}
Index: linux-2.6.19-rc1-mm1/arch/x86_64/kernel/kprobes.c
===================================================================
--- linux-2.6.19-rc1-mm1.orig/arch/x86_64/kernel/kprobes.c 2006-10-16 10:40:00.000000000 +0900
+++ linux-2.6.19-rc1-mm1/arch/x86_64/kernel/kprobes.c 2006-10-16 10:54:09.000000000 +0900
@@ -224,7 +224,7 @@
void __kprobes arch_remove_kprobe(struct kprobe *p)
{
mutex_lock(&kprobe_mutex);
- free_insn_slot(p->ainsn.insn);
+ free_insn_slot(p->ainsn.insn, 0);
mutex_unlock(&kprobe_mutex);
}
Index: linux-2.6.19-rc1-mm1/include/linux/kprobes.h
===================================================================
--- linux-2.6.19-rc1-mm1.orig/include/linux/kprobes.h 2006-10-16 10:40:02.000000000 +0900
+++ linux-2.6.19-rc1-mm1/include/linux/kprobes.h 2006-10-16 21:43:07.000000000 +0900
@@ -165,7 +165,7 @@
extern int arch_init_kprobes(void);
extern void show_registers(struct pt_regs *regs);
extern kprobe_opcode_t *get_insn_slot(void);
-extern void free_insn_slot(kprobe_opcode_t *slot);
+extern void free_insn_slot(kprobe_opcode_t *slot, int dirty);
extern void kprobes_inc_nmissed_count(struct kprobe *p);
/* Get the kprobe at this addr (if any) - called with preemption disabled */
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2006-11-06 14:18 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2006-11-01 17:01 [RFC][PATCH][kprobe] enabling booster on the preemptible kernel, take 2 bibo mao
2006-11-02 18:51 ` Masami Hiramatsu
2006-11-03 9:16 ` bibo,mao
2006-11-06 18:52 ` [RFC][PATCH][kprobe] enabling booster on the preemptible kernel, take 3 Masami Hiramatsu
-- strict thread matches above, loose matches on Subject: below --
2006-10-16 13:14 [RFC][PATCH][kprobe] enabling booster on the preemptible kernel, take 2 Masami Hiramatsu
2006-10-30 6:37 ` bibo,mao
2006-10-30 14:07 ` Masami Hiramatsu
2006-10-31 9:14 ` bibo,mao
2006-10-31 13:47 ` Masami Hiramatsu
2006-10-31 13:49 ` Ingo Molnar
2006-10-31 14:13 ` Ingo Molnar
2006-10-31 16:39 ` Masami Hiramatsu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).