lkml.org 
[lkml]   [2006]   [Apr]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [(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/

    \
     
     \ /
      Last update: 2006-04-20 06:58    [W:3.402 / U:0.760 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site