Messages in this thread | | | Date | Wed, 19 Apr 2006 21:53:00 -0700 | From | Keshavamurthy Anil S <> | Subject | Re: [(resend)patch 7/7] Kprobes - Register for page fault notify on active probes |
| |
On Thu, Apr 20, 2006 at 09:27:35AM +0530, Ananth N Mavinakayanahalli wrote: > On Wed, Apr 19, 2006 at 03:14:26PM -0700, Anil S Keshavamurthy wrote: > > With this patch Kprobes now registers for page fault notifications only > > when their is an active probe registered. Once all the active probes are > > unregistered their is no need to be notified of page faults and kprobes > > unregisters itself from the page fault notifications. Hence we > > will have ZERO side effects when no probes are active. > > This patch isn't complete yet... comments below. > > > Signed-off-by: Anil S Keshavamurthy <anil.s.keshavamurthy@intel.com> > > --- > > kernel/kprobes.c | 25 +++++++++++++++++-------- > > 1 file changed, 17 insertions(+), 8 deletions(-) > > > > Index: linux-2.6.17-rc1-mm3/kernel/kprobes.c > > =================================================================== > > --- linux-2.6.17-rc1-mm3.orig/kernel/kprobes.c > > +++ linux-2.6.17-rc1-mm3/kernel/kprobes.c > > @@ -47,11 +47,17 @@ > > > > static struct hlist_head kprobe_table[KPROBE_TABLE_SIZE]; > > static struct hlist_head kretprobe_inst_table[KPROBE_TABLE_SIZE]; > > +static atomic_t kprobe_count; > > > > DEFINE_MUTEX(kprobe_mutex); /* Protects kprobe_table */ > > DEFINE_SPINLOCK(kretprobe_lock); /* Protects kretprobe_inst_table */ > > static DEFINE_PER_CPU(struct kprobe *, kprobe_instance) = NULL; > > > > +static struct notifier_block kprobe_page_fault_nb = { > > + .notifier_call = kprobe_exceptions_notify, > > + .priority = 0x7fffffff /* we need to notified first */ > > +}; > > + > > #ifdef __ARCH_WANT_KPROBES_INSN_SLOT > > /* > > * kprobe->ainsn.insn points to the copy of the instruction to be > > @@ -464,6 +470,8 @@ static int __kprobes __register_kprobe(s > > old_p = get_kprobe(p->addr); > > if (old_p) { > > ret = register_aggr_kprobe(old_p, p); > > + if (!ret) > > + atomic_inc(&kprobe_count); > > goto out; > > } > > > > @@ -474,6 +482,9 @@ static int __kprobes __register_kprobe(s > > hlist_add_head_rcu(&p->hlist, > > &kprobe_table[hash_ptr(p->addr, KPROBE_HASH_BITS)]); > > > > + if(atomic_add_return(1, &kprobe_count) == 2) > ^^^ > if (..) please, here and elsewhere > > This will not work as desired for i386 (i386 no longer has a kprobe on the > trampoline) and sparc64 (no retprobe support). Instead of hardcoding value 2, #define KPROBE_ARCH_INITIAL_COUNT x should do the trick. > > > + register_page_fault_notifier(&kprobe_page_fault_nb); > > + > > arch_arm_kprobe(p); > > > > out: > > @@ -523,9 +534,13 @@ valid_p: > > cleanup_p = 0; > > } > > > > + if(atomic_add_return(-1, &kprobe_count) == 1) > > + unregister_page_fault_notifier(&kprobe_page_fault_nb); > > Same here - i386 and sparc64 need different checks. Yup, will take care in my next version.
> > > + else > > + synchronize_rcu(); > > As of now, synchronize_sched() is aliased to synchronize_rcu() but I am > told its scheduled to change in the future. > > Please revert this back to synchronize_sched(). > I will revert it back in my take2 :)
Thanks again for your valuable feedback.
-Anil Keshavamurthy - 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/
| |