From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 17674 invoked by alias); 30 Oct 2006 06:37:19 -0000 Received: (qmail 17659 invoked by uid 22791); 30 Oct 2006 06:37:17 -0000 X-Spam-Status: No, hits=-0.0 required=5.0 tests=AWL,BAYES_50 X-Spam-Check-By: sourceware.org Received: from mga03.intel.com (HELO mga03.intel.com) (143.182.124.21) by sourceware.org (qpsmtpd/0.31) with ESMTP; Mon, 30 Oct 2006 06:37:14 +0000 Received: from azsmga001.ch.intel.com ([10.2.17.19]) by mga03.intel.com with ESMTP; 29 Oct 2006 22:37:12 -0800 Received: from bmao-mobl.ccr.corp.intel.com (HELO [10.239.20.25]) ([10.239.20.25]) by azsmga001.ch.intel.com with ESMTP; 29 Oct 2006 22:37:12 -0800 X-ExtLoop1: 1 X-IronPort-AV: i="4.09,370,1157353200"; d="scan'208"; a="137921619:sNHT21667345" Message-ID: <45459D94.8060905@intel.com> Date: Mon, 30 Oct 2006 06:37:00 -0000 From: "bibo,mao" User-Agent: Thunderbird 1.5.0.7 (Windows/20060909) MIME-Version: 1.0 To: Masami Hiramatsu CC: "Keshavamurthy, Anil S" , Ananth N Mavinakayanahalli , Prasanna S Panchamukhi , Ingo Molnar , SystemTAP , Satoshi Oshima , Hideo Aoki , Yumiko Sugita Subject: Re: [RFC][PATCH][kprobe] enabling booster on the preemptible kernel, take 2 References: <45338593.6090207@hitachi.com> In-Reply-To: <45338593.6090207@hitachi.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit X-IsSubscribed: yes Mailing-List: contact systemtap-help@sourceware.org; run by ezmlm Precedence: bulk List-Subscribe: List-Post: List-Help: , Sender: systemtap-owner@sourceware.org X-SW-Source: 2006-q4/txt/msg00275.txt.bz2 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 > #include > #include > +#include > #include > #include > #include > @@ -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 */ >