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 <[email protected]>
> > ---
> > 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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
