public inbox for systemtap@sourceware.org
 help / color / mirror / Atom feed
* RE: [Patch 2/3][Djprobe] Djprobe update for linux-2.6.14-mm1
@ 2005-11-14  2:03 Zhang, Yanmin
  2005-11-22 13:04 ` Masami Hiramatsu
  0 siblings, 1 reply; 5+ messages in thread
From: Zhang, Yanmin @ 2005-11-14  2:03 UTC (permalink / raw)
  To: Masami Hiramatsu
  Cc: systemtap, Satoshi Oshima, Yumiko Sugita, Hideo Aoki,
	Keshavamurthy, Anil S

>>-----Original Message-----
>>From: Masami Hiramatsu [mailto:hiramatu@sdl.hitachi.co.jp]
>>Sent: 2005年11月12日 3:20
>>To: Zhang, Yanmin
>>Cc: systemtap@sources.redhat.com; Satoshi Oshima; Yumiko Sugita; Hideo Aoki;
>>Keshavamurthy, Anil S
>>Subject: Re: [Patch 2/3][Djprobe] Djprobe update for linux-2.6.14-mm1
>>
>>Hi,
>>
>>Thank you for your review!
>>
>>Zhang, Yanmin wrote:
>>>>>-----Original Message-----
>>>>>From: systemtap-owner@sourceware.org
>>[mailto:systemtap-owner@sourceware.org]
>>>>>On Behalf Of Masami Hiramatsu
>>>>>Sent: 2005/11/8 21:26
>>>>>To: systemtap@sources.redhat.com
>>>>>Cc: Satoshi Oshima; Yumiko Sugita; Hideo Aoki
>>>>>Subject: [Patch 2/3][Djprobe] Djprobe update for linux-2.6.14-mm1
>>>>>
>>>>>Hi,
>>>>>
>>>>>This patch is the architecture independant part of djprobe.
>>>>>+static inline
>>>>>+    struct djprobe_instance *__create_djprobe_instance(struct djprobe
>>*djp,
>>>>>+						       void *addr, int size)
>>>>>+{
>>>>>+	struct djprobe_instance *djpi;
>>>>>+	/* allocate a new instance */
>>>>>+	djpi = kcalloc(1, sizeof(struct djprobe_instance), GFP_ATOMIC);
>>>>>+	if (djpi == NULL) {
>>>>>+		goto out;
>>>>>+	}
>>>>>+	/* allocate stub */
>>>>>+	djpi->stub.insn = __get_insn_slot(&djprobe_insn_pages);
>>>>>+	if (djpi->stub.insn == NULL) {
>>>
>>> [YM] If coming here, djpi->plist is not initiated.
>>> So __free_djprobe_instance=>hlist_del will cause panic.
>>> How about to move the INIT_LIST_HEAD(&djpi->plist) just after kcalloc?
>>
>>Thanks for finding that. I will fix it so.
>>
>>>>>+int __kprobes register_djprobe(struct djprobe *djp, void *addr, int size)
>>>>>+{
>>>>>+	struct djprobe_instance *djpi;
>>>>>+	struct kprobe *kp;
>>>>>+	int ret = 0, i;
>>>>>+
>>>>>+	BUG_ON(in_interrupt());
>>>>>+
>>>>>+	if (size > ARCH_STUB_INSN_MAX || size < ARCH_STUB_INSN_MIN)
>>>>>+		return -EINVAL;
>>>>>+
>>>>>+	if ((ret = in_kprobes_functions((unsigned long)addr)) != 0)
>>>>>+		return ret;
>>>>>+
>>>>>+	down(&djprobe_mutex);
>>>>>+	INIT_LIST_HEAD(&djp->plist);
>>>>>+	/* check confliction with other djprobes */
>>>>>+	djpi = __get_djprobe_instance(addr, size);
>>>>>+	if (djpi) {
>>>>>+		if (djpi->kp.addr == addr) {
>>>>>+			djp->inst = djpi;	/* add to another instance */
>>>>>+			list_add_rcu(&djp->plist, &djpi->plist);
>>>>>+		} else {
>>>>>+			ret = -EEXIST;	/* other djprobes were inserted */
>>>>>+		}
>>>>>+		goto out;
>>>>>+	}
>>>>>+	djpi = __create_djprobe_instance(djp, addr, size);
>>>>>+	if (djpi == NULL) {
>>>>>+		ret = -ENOMEM;
>>>>>+		goto out;
>>>>>+	}
>>>>>+
>>>>>+	/* check confliction with kprobes */
>>>>>+	for (i = 0; i < size; i++) {
>>>>>+		kp = get_kprobe((void *)((long)addr + i));
>>>
>>> [YM] There is a race between get_kprobe and register_kprobe without
>>> locking kprobe_lock. Could register_kprobe to check if the address is
>>> in a JTPR of registered djprobe? I think djprobe and kprobe could
>>> share the same spin_lock, namely kprobe_lock.
>>
>>hmm, but __check_safety() may sleep. So spin-lock will cause dead-lock.
>>I think it can avoid race condition by following two changes.
>>
>>1) delay checking confliction like below.
>>
>>       /* first, register as a kprobe.
>>	if there is another competitor, this waits until it registered */
>>        ret = register_kprobe(&djpi->kp);
>>        if (ret < 0) {
>>       fail:
>>                djpi->kp.addr = NULL;
>>                djp->inst = NULL;
>>                list_del_rcu(&djp->plist);
>>                __free_djprobe_instance(djpi);
>>        } else {
>>                /* next, check confliction with kprobes */
>>                for (i = 0; i < size; i++) {
>>                        kp = get_kprobe((void *)((long)addr + i));
>>                        if (kp != NULL && kp != &djpi->kp) {
>>                                ret = -EEXIST;  /* other kprobes were
>>inserted */
>>                                goto fail;
>>                        }
>>                }
>>                __check_safety();
>>                arch_install_djprobe_instance(djpi);
>>        }
>>
>>
>>2) share the mutex of djprobe with kprobes like below.
>>
>>int __kprobes register_kprobe(struct kprobe *p)
>>{
>>        int ret = 0;
>>        unsigned long flags = 0;
>>        struct kprobe *old_p;
>>
>>        if ((ret = in_kprobes_functions((unsigned long) p->addr)) != 0)
>>                return ret;
>>#ifdef CONFIG_DJPROBE
>>        down(&djprobe_mutex);
>>        if (p->pre_handler != djprobe_pre_handler &&
>>            get_djprobe_instance(p->addr, 1) != NULL)
>>                return -EEXIST;
>>#endif /* CONFIG_DJPROBE */
>>        if ((ret = arch_prepare_kprobe(p)) != 0)
>>                goto rm_kprobe;
>>
>>        p->nmissed = 0;
>>        spin_lock_irqsave(&kprobe_lock, flags);
>>        old_p = get_kprobe(p->addr);
>>        if (old_p) {
>>                ret = register_aggr_kprobe(old_p, p);
>>                goto out;
>>        }
>>
>>        arch_copy_kprobe(p);
>>        INIT_HLIST_NODE(&p->hlist);
>>        hlist_add_head_rcu(&p->hlist,
>>                       &kprobe_table[hash_ptr(p->addr,
>>KPROBE_HASH_BITS)]);
>>
>>        arch_arm_kprobe(p);
>>
>>out:
>>        spin_unlock_irqrestore(&kprobe_lock, flags);
>>rm_kprobe:
>>#ifdef CONFIG_DJPROBE
>>        up(&djprobe_mutex);
>>#endif /* CONFIG_DJPROBE */
>>        if (ret == -EEXIST)
>>                arch_remove_kprobe(p);
>>        return ret;
>>}
[YM] It's reasonable. In function register_kprobe, 
1) get_djprobe_instance should be __get_djprobe_instance if djprobe_mutex is used.
2) Release djprobe_mutex before " return -EEXIST".
3) Parameter size of call to get_djprobe_instance is always 1 here. How about to change it to ARCH_STUB_INSN_MAX?

One more comment on your 3rd patch, how about to change:
+#define ARCH_STUB_SIZE ((long)&arch_tmpl_stub_end - (long)&arch_tmpl_stub_entry)
to
+#define ARCH_STUB_SIZE (((long)&arch_tmpl_stub_end - (long)&arch_tmpl_stub_entry)/sizeof(kprobe_opcode_t))

On ia32, sizeof(kprobe_opcode_t) is equal to 1, but on other platform, it might not be. Just to make it clearer.


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Patch 2/3][Djprobe] Djprobe update for linux-2.6.14-mm1
  2005-11-14  2:03 [Patch 2/3][Djprobe] Djprobe update for linux-2.6.14-mm1 Zhang, Yanmin
@ 2005-11-22 13:04 ` Masami Hiramatsu
  0 siblings, 0 replies; 5+ messages in thread
From: Masami Hiramatsu @ 2005-11-22 13:04 UTC (permalink / raw)
  To: Zhang, Yanmin
  Cc: systemtap, Satoshi Oshima, Yumiko Sugita, Hideo Aoki,
	Keshavamurthy, Anil S

Hi, Zhang

I am sorry to reply so late.

Zhang, Yanmin wrote:
> [YM] It's reasonable. In function register_kprobe,

Thanks.

> 1) get_djprobe_instance should be __get_djprobe_instance if djprobe_mutex is used.

Exactly. I missed it.

> 2) Release djprobe_mutex before " return -EEXIST".

Thanks to find that!

> 3) Parameter size of call to get_djprobe_instance is always 1 here. How about to change it to ARCH_STUB_INSN_MAX?
> One more comment on your 3rd patch, how about to change:
> +#define ARCH_STUB_SIZE ((long)&arch_tmpl_stub_end - (long)&arch_tmpl_stub_entry)
> to
> +#define ARCH_STUB_SIZE (((long)&arch_tmpl_stub_end - (long)&arch_tmpl_stub_entry)/sizeof(kprobe_opcode_t))
>
> On ia32, sizeof(kprobe_opcode_t) is equal to 1, but on other platform, it might not be. Just to make it clearer.
>

OK, I will change it as like that.

Best Regards,

-- 
Masami HIRAMATSU
2nd Research Dept.
Hitachi, Ltd., Systems Development Laboratory
E-mail: hiramatu@sdl.hitachi.co.jp

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Patch 2/3][Djprobe] Djprobe update for linux-2.6.14-mm1
  2005-11-11  2:33 Zhang, Yanmin
@ 2005-11-11 19:20 ` Masami Hiramatsu
  0 siblings, 0 replies; 5+ messages in thread
From: Masami Hiramatsu @ 2005-11-11 19:20 UTC (permalink / raw)
  To: Zhang, Yanmin
  Cc: systemtap, Satoshi Oshima, Yumiko Sugita, Hideo Aoki,
	Keshavamurthy, Anil S

Hi,

Thank you for your review!

Zhang, Yanmin wrote:
>>>-----Original Message-----
>>>From: systemtap-owner@sourceware.org [mailto:systemtap-owner@sourceware.org]
>>>On Behalf Of Masami Hiramatsu
>>>Sent: 2005/11/8 21:26
>>>To: systemtap@sources.redhat.com
>>>Cc: Satoshi Oshima; Yumiko Sugita; Hideo Aoki
>>>Subject: [Patch 2/3][Djprobe] Djprobe update for linux-2.6.14-mm1
>>>
>>>Hi,
>>>
>>>This patch is the architecture independant part of djprobe.
>>>+static inline
>>>+    struct djprobe_instance *__create_djprobe_instance(struct djprobe *djp,
>>>+						       void *addr, int size)
>>>+{
>>>+	struct djprobe_instance *djpi;
>>>+	/* allocate a new instance */
>>>+	djpi = kcalloc(1, sizeof(struct djprobe_instance), GFP_ATOMIC);
>>>+	if (djpi == NULL) {
>>>+		goto out;
>>>+	}
>>>+	/* allocate stub */
>>>+	djpi->stub.insn = __get_insn_slot(&djprobe_insn_pages);
>>>+	if (djpi->stub.insn == NULL) {
>
> [YM] If coming here, djpi->plist is not initiated.
> So __free_djprobe_instance=>hlist_del will cause panic.
> How about to move the INIT_LIST_HEAD(&djpi->plist) just after kcalloc?

Thanks for finding that. I will fix it so.

>>>+int __kprobes register_djprobe(struct djprobe *djp, void *addr, int size)
>>>+{
>>>+	struct djprobe_instance *djpi;
>>>+	struct kprobe *kp;
>>>+	int ret = 0, i;
>>>+
>>>+	BUG_ON(in_interrupt());
>>>+
>>>+	if (size > ARCH_STUB_INSN_MAX || size < ARCH_STUB_INSN_MIN)
>>>+		return -EINVAL;
>>>+
>>>+	if ((ret = in_kprobes_functions((unsigned long)addr)) != 0)
>>>+		return ret;
>>>+
>>>+	down(&djprobe_mutex);
>>>+	INIT_LIST_HEAD(&djp->plist);
>>>+	/* check confliction with other djprobes */
>>>+	djpi = __get_djprobe_instance(addr, size);
>>>+	if (djpi) {
>>>+		if (djpi->kp.addr == addr) {
>>>+			djp->inst = djpi;	/* add to another instance */
>>>+			list_add_rcu(&djp->plist, &djpi->plist);
>>>+		} else {
>>>+			ret = -EEXIST;	/* other djprobes were inserted */
>>>+		}
>>>+		goto out;
>>>+	}
>>>+	djpi = __create_djprobe_instance(djp, addr, size);
>>>+	if (djpi == NULL) {
>>>+		ret = -ENOMEM;
>>>+		goto out;
>>>+	}
>>>+
>>>+	/* check confliction with kprobes */
>>>+	for (i = 0; i < size; i++) {
>>>+		kp = get_kprobe((void *)((long)addr + i));
>
> [YM] There is a race between get_kprobe and register_kprobe without
> locking kprobe_lock. Could register_kprobe to check if the address is
> in a JTPR of registered djprobe? I think djprobe and kprobe could
> share the same spin_lock, namely kprobe_lock.

hmm, but __check_safety() may sleep. So spin-lock will cause dead-lock.
I think it can avoid race condition by following two changes.

1) delay checking confliction like below.

       /* first, register as a kprobe.
	if there is another competitor, this waits until it registered */
        ret = register_kprobe(&djpi->kp);
        if (ret < 0) {
       fail:
                djpi->kp.addr = NULL;
                djp->inst = NULL;
                list_del_rcu(&djp->plist);
                __free_djprobe_instance(djpi);
        } else {
                /* next, check confliction with kprobes */
                for (i = 0; i < size; i++) {
                        kp = get_kprobe((void *)((long)addr + i));
                        if (kp != NULL && kp != &djpi->kp) {
                                ret = -EEXIST;  /* other kprobes were inserted */
                                goto fail;
                        }
                }
                __check_safety();
                arch_install_djprobe_instance(djpi);
        }


2) share the mutex of djprobe with kprobes like below.

int __kprobes register_kprobe(struct kprobe *p)
{
        int ret = 0;
        unsigned long flags = 0;
        struct kprobe *old_p;

        if ((ret = in_kprobes_functions((unsigned long) p->addr)) != 0)
                return ret;
#ifdef CONFIG_DJPROBE
        down(&djprobe_mutex);
        if (p->pre_handler != djprobe_pre_handler &&
            get_djprobe_instance(p->addr, 1) != NULL)
                return -EEXIST;
#endif /* CONFIG_DJPROBE */
        if ((ret = arch_prepare_kprobe(p)) != 0)
                goto rm_kprobe;

        p->nmissed = 0;
        spin_lock_irqsave(&kprobe_lock, flags);
        old_p = get_kprobe(p->addr);
        if (old_p) {
                ret = register_aggr_kprobe(old_p, p);
                goto out;
        }

        arch_copy_kprobe(p);
        INIT_HLIST_NODE(&p->hlist);
        hlist_add_head_rcu(&p->hlist,
                       &kprobe_table[hash_ptr(p->addr, KPROBE_HASH_BITS)]);

        arch_arm_kprobe(p);

out:
        spin_unlock_irqrestore(&kprobe_lock, flags);
rm_kprobe:
#ifdef CONFIG_DJPROBE
        up(&djprobe_mutex);
#endif /* CONFIG_DJPROBE */
        if (ret == -EEXIST)
                arch_remove_kprobe(p);
        return ret;
}

What would you think about this?

Best Regards,

-- 
Masami HIRAMATSU
2nd Research Dept.
Hitachi, Ltd., Systems Development Laboratory
E-mail: hiramatu@sdl.hitachi.co.jp


^ permalink raw reply	[flat|nested] 5+ messages in thread

* RE: [Patch 2/3][Djprobe] Djprobe update for linux-2.6.14-mm1
@ 2005-11-11  2:33 Zhang, Yanmin
  2005-11-11 19:20 ` Masami Hiramatsu
  0 siblings, 1 reply; 5+ messages in thread
From: Zhang, Yanmin @ 2005-11-11  2:33 UTC (permalink / raw)
  To: Masami Hiramatsu, systemtap
  Cc: Satoshi Oshima, Yumiko Sugita, Hideo Aoki, Keshavamurthy, Anil S

>>-----Original Message-----
>>From: systemtap-owner@sourceware.org [mailto:systemtap-owner@sourceware.org]
>>On Behalf Of Masami Hiramatsu
>>Sent: 2005年11月8日 21:26
>>To: systemtap@sources.redhat.com
>>Cc: Satoshi Oshima; Yumiko Sugita; Hideo Aoki
>>Subject: [Patch 2/3][Djprobe] Djprobe update for linux-2.6.14-mm1
>>
>>Hi,
>>
>>This patch is the architecture independant part of djprobe.
>>+static inline
>>+    struct djprobe_instance *__create_djprobe_instance(struct djprobe *djp,
>>+						       void *addr, int size)
>>+{
>>+	struct djprobe_instance *djpi;
>>+	/* allocate a new instance */
>>+	djpi = kcalloc(1, sizeof(struct djprobe_instance), GFP_ATOMIC);
>>+	if (djpi == NULL) {
>>+		goto out;
>>+	}
>>+	/* allocate stub */
>>+	djpi->stub.insn = __get_insn_slot(&djprobe_insn_pages);
>>+	if (djpi->stub.insn == NULL) {
[YM] If coming here, djpi->plist is not initiated. So __free_djprobe_instance=>hlist_del will cause panic. How about to move the INIT_LIST_HEAD(&djpi->plist) just after kcalloc?




>>+		__free_djprobe_instance(djpi);
>>+		djpi = NULL;
>>+		goto out;
>>+	}
>>+
>>+	/* attach */
>>+	djp->inst = djpi;
>>+	INIT_LIST_HEAD(&djpi->plist);
>>+	list_add_rcu(&djp->plist, &djpi->plist);
>>+	djpi->kp.addr = addr;
>>+	djpi->kp.pre_handler = djprobe_pre_handler;
>>+	arch_prepare_djprobe_instance(djpi, size);
>>+
>>+	INIT_HLIST_NODE(&djpi->hlist);
>>+	hlist_add_head(&djpi->hlist,
>>&djprobe_inst_table[hash_djprobe(addr)]);
>>+      out:
>>+	return djpi;
>>+}
>>+
>>+static struct djprobe_instance *__kprobes __get_djprobe_instance(void
>>*addr,
>>+								 int size)
>>+{
>>+	struct djprobe_instance *djpi;
>>+	struct hlist_node *node;
>>+	unsigned long idx, eidx;
>>+
>>+	idx = hash_djprobe(addr - ARCH_STUB_INSN_MAX);
>>+	eidx = ((hash_djprobe(addr + size) + 1) & DJPROBE_TABLE_MASK);
>>+	do {
>>+		hlist_for_each_entry(djpi, node, &djprobe_inst_table[idx],
>>+				     hlist) {
>>+			if (((long)addr <
>>+			     (long)djpi->kp.addr + DJPI_ARCH_SIZE(djpi))
>>+			    && ((long)djpi->kp.addr < (long)addr + size)) {
>>+				return djpi;
>>+			}
>>+		}
>>+		idx = ((idx + 1) & DJPROBE_TABLE_MASK);
>>+	}while (idx != eidx);
>>+
>>+	return NULL;
>>+}
>>+
>>+struct djprobe_instance *__kprobes get_djprobe_instance(void *addr, int
>>size)
>>+{
>>+	struct djprobe_instance *djpi;
>>+	down(&djprobe_mutex);
>>+	djpi = __get_djprobe_instance(addr, size);
>>+	up(&djprobe_mutex);
>>+	return djpi;
>>+}
>>+
>>+/* This work function invoked while djprobe_mutex is locked. */
>>+static void __kprobes __work_check_safety(void *data)
>>+{
>>+	if (atomic_dec_and_test(&djprobe_count)) {
>>+		wake_up_all(&djprobe_wqh);
>>+	}
>>+}
>>+
>>+static void __kprobes __check_safety(void)
>>+{
>>+	int cpu;
>>+	struct work_struct *wk;
>>+	lock_cpu_hotplug();
>>+	atomic_set(&djprobe_count, num_online_cpus() - 1);
>>+	for_each_online_cpu(cpu) {
>>+		if (cpu == smp_processor_id())
>>+			continue;
>>+		wk = &per_cpu(djprobe_works, cpu);
>>+		INIT_WORK(wk, __work_check_safety, NULL);
>>+		schedule_delayed_work_on(cpu, wk, 0);
>>+	}
>>+	wait_event(djprobe_wqh, (atomic_read(&djprobe_count) == 0));
>>+	unlock_cpu_hotplug();
>>+}
>>+
>>+int __kprobes register_djprobe(struct djprobe *djp, void *addr, int size)
>>+{
>>+	struct djprobe_instance *djpi;
>>+	struct kprobe *kp;
>>+	int ret = 0, i;
>>+
>>+	BUG_ON(in_interrupt());
>>+
>>+	if (size > ARCH_STUB_INSN_MAX || size < ARCH_STUB_INSN_MIN)
>>+		return -EINVAL;
>>+
>>+	if ((ret = in_kprobes_functions((unsigned long)addr)) != 0)
>>+		return ret;
>>+
>>+	down(&djprobe_mutex);
>>+	INIT_LIST_HEAD(&djp->plist);
>>+	/* check confliction with other djprobes */
>>+	djpi = __get_djprobe_instance(addr, size);
>>+	if (djpi) {
>>+		if (djpi->kp.addr == addr) {
>>+			djp->inst = djpi;	/* add to another instance */
>>+			list_add_rcu(&djp->plist, &djpi->plist);
>>+		} else {
>>+			ret = -EEXIST;	/* other djprobes were inserted */
>>+		}
>>+		goto out;
>>+	}
>>+	djpi = __create_djprobe_instance(djp, addr, size);
>>+	if (djpi == NULL) {
>>+		ret = -ENOMEM;
>>+		goto out;
>>+	}
>>+
>>+	/* check confliction with kprobes */
>>+	for (i = 0; i < size; i++) {
>>+		kp = get_kprobe((void *)((long)addr + i));
[YM] There is a race between get_kprobe and register_kprobe without locking kprobe_lock. Could register_kprobe to check if the address is in a JTPR of registered djprobe? I think djprobe and kprobe could share the same spin_lock, namely kprobe_lock.



>>+		if (kp != NULL) {
>>+			ret = -EEXIST;	/* a kprobes were inserted */
>>+			goto fail;
>>+		}
>>+	}
>>+	ret = register_kprobe(&djpi->kp);
>>+	if (ret < 0) {
>>+       fail:
>>+		djpi->kp.addr = NULL;
>>+		djp->inst = NULL;
>>+		list_del_rcu(&djp->plist);
>>+		__free_djprobe_instance(djpi);
>>+	} else {
>>+		__check_safety();
>>+		arch_install_djprobe_instance(djpi);
>>+	}
>>+       out:
>>+	up(&djprobe_mutex);
>>+	return ret;

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [Patch 2/3][Djprobe] Djprobe update for linux-2.6.14-mm1
  2005-11-04 13:29       ` Masami Hiramatsu
@ 2005-11-08 13:26         ` Masami Hiramatsu
  0 siblings, 0 replies; 5+ messages in thread
From: Masami Hiramatsu @ 2005-11-08 13:26 UTC (permalink / raw)
  To: systemtap; +Cc: Satoshi Oshima, Yumiko Sugita, Hideo Aoki

Hi,

This patch is the architecture independant part of djprobe.
I removed djprobe_post_handler.

-- 
Masami HIRAMATSU
2nd Research Dept.
Hitachi, Ltd., Systems Development Laboratory
E-mail: hiramatu@sdl.hitachi.co.jp

 include/linux/djprobe.h |   78 ++++++++++++++
 include/linux/kprobes.h |    4
 kernel/Makefile         |    1
 kernel/djprobe.c        |  252 ++++++++++++++++++++++++++++++++++++++++++++++++
 kernel/kprobes.c        |    8 +
 5 files changed, 342 insertions(+), 1 deletion(-)
diff -Narup linux-2.6.14-mm1.djp.1/include/linux/djprobe.h linux-2.6.14-mm1.djp.2/include/linux/djprobe.h
--- linux-2.6.14-mm1.djp.1/include/linux/djprobe.h	1970-01-01 09:00:00.000000000 +0900
+++ linux-2.6.14-mm1.djp.2/include/linux/djprobe.h	2005-11-08 11:56:43.000000000 +0900
@@ -0,0 +1,78 @@
+#ifndef _LINUX_DJPROBE_H
+#define _LINUX_DJPROBE_H
+/*
+ *  Kernel Direct Jump Probe (Djprobe)
+ *  include/linux/djprobe.h
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
+ *
+ * Copyright (C) Hitachi, Ltd. 2005
+ *
+ * 2005-Aug	Created by Masami HIRAMATSU <hiramatu@sdl.hitachi.co.jp>
+ * 		Initial implementation of Direct jump probe (djprobe)
+ *              to reduce overhead.
+ */
+#include <linux/config.h>
+#include <linux/list.h>
+#include <linux/smp.h>
+#include <linux/kprobes.h>
+#include <asm/djprobe.h>
+
+struct djprobe;
+/* djprobe's instance (internal use)*/
+struct djprobe_instance {
+	struct list_head plist; /* list of djprobes for multiprobe support */
+	struct arch_djprobe_stub stub;
+	struct kprobe kp;
+	struct hlist_node hlist; /* list of djprobe_instances */
+};
+#define DJPI_EMPTY(djpi)  (list_empty(&djpi->plist))
+
+struct djprobe;
+typedef void (*djprobe_handler_t) (struct djprobe *, struct pt_regs *);
+/*
+ * Direct Jump probe interface structure
+ */
+struct djprobe {
+	/* list of djprobes */
+	struct list_head plist;
+
+	/* probing handler (pre-executed) */
+	djprobe_handler_t handler;
+	
+	/* pointer for instance */
+	struct djprobe_instance *inst;
+};
+
+#ifdef CONFIG_DJPROBE
+extern int arch_prepare_djprobe_instance(struct djprobe_instance *djpi,
+					 unsigned long size);
+extern int djprobe_pre_handler(struct kprobe *, struct pt_regs *);
+extern void arch_install_djprobe_instance(struct djprobe_instance *djpi);
+extern void arch_uninstall_djprobe_instance(struct djprobe_instance *djpi);
+struct djprobe_instance *__kprobes get_djprobe_instance(void *addr, int size);
+
+int register_djprobe(struct djprobe *p, void *addr, int size);
+void unregister_djprobe(struct djprobe *p);
+#else /* CONFIG_DJPROBE */
+static inline int register_djprobe(struct djprobe *p)
+{
+	return -ENOSYS;
+}
+static inline void unregister_djprobe(struct djprobe *p)
+{
+}
+#endif				/* CONFIG_DJPROBE */
+#endif				/* _LINUX_DJPROBE_H */
diff -Narup linux-2.6.14-mm1.djp.1/include/linux/kprobes.h linux-2.6.14-mm1.djp.2/include/linux/kprobes.h
--- linux-2.6.14-mm1.djp.1/include/linux/kprobes.h	2005-11-08 11:52:46.000000000 +0900
+++ linux-2.6.14-mm1.djp.2/include/linux/kprobes.h	2005-11-08 11:58:50.000000000 +0900
@@ -163,6 +163,10 @@ 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 kprobe_opcode_t *__get_insn_slot(struct kprobe_insn_page_list *pages);
+extern void __free_insn_slot(struct kprobe_insn_page_list *pages,
+			     kprobe_opcode_t * slot);
+extern int in_kprobes_functions(unsigned long addr);

 /* Get the kprobe at this addr (if any) - called with preemption disabled */
 struct kprobe *get_kprobe(void *addr);
diff -Narup linux-2.6.14-mm1.djp.1/kernel/Makefile linux-2.6.14-mm1.djp.2/kernel/Makefile
--- linux-2.6.14-mm1.djp.1/kernel/Makefile	2005-11-08 11:51:04.000000000 +0900
+++ linux-2.6.14-mm1.djp.2/kernel/Makefile	2005-11-08 11:56:43.000000000 +0900
@@ -27,6 +27,7 @@ obj-$(CONFIG_STOP_MACHINE) += stop_machi
 obj-$(CONFIG_AUDIT) += audit.o
 obj-$(CONFIG_AUDITSYSCALL) += auditsc.o
 obj-$(CONFIG_KPROBES) += kprobes.o
+obj-$(CONFIG_DJPROBE) += djprobe.o
 obj-$(CONFIG_SYSFS) += ksysfs.o
 obj-$(CONFIG_DETECT_SOFTLOCKUP) += softlockup.o
 obj-$(CONFIG_GENERIC_HARDIRQS) += irq/
diff -Narup linux-2.6.14-mm1.djp.1/kernel/djprobe.c linux-2.6.14-mm1.djp.2/kernel/djprobe.c
--- linux-2.6.14-mm1.djp.1/kernel/djprobe.c	1970-01-01 09:00:00.000000000 +0900
+++ linux-2.6.14-mm1.djp.2/kernel/djprobe.c	2005-11-08 11:56:43.000000000 +0900
@@ -0,0 +1,252 @@
+/*
+ *  Kernel Direct Jump Probe (Djprobe)
+ *  kernel/djprobes.c
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
+ *
+ * Copyright (C) Hitachi, Ltd. 2005
+ *
+ * 2005-Aug	Created by Masami HIRAMATSU <hiramatu@sdl.hitachi.co.jp>
+ * 		Initial implementation of Direct jump probe (djprobe)
+ *              to reduce overhead.
+ */
+#include <linux/djprobe.h>
+#include <linux/hash.h>
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/moduleloader.h>
+#include <asm-generic/sections.h>
+#include <asm/cacheflush.h>
+#include <asm/errno.h>
+
+#include <linux/cpu.h>
+#include <linux/percpu.h>
+#include <asm/semaphore.h>
+
+/*
+ * The djprobe do not refer instances list when probe function called.
+ * This list is operated on registering and unregistering djprobe.
+ */
+#define DJPROBE_BLOCK_BITS 6
+#define DJPROBE_BLOCK_SIZE (1 << DJPROBE_BLOCK_BITS)
+#define DJPROBE_HASH_BITS 8
+#define DJPROBE_TABLE_SIZE (1 << DJPROBE_HASH_BITS)
+#define DJPROBE_TABLE_MASK (DJPROBE_TABLE_SIZE - 1)
+
+/* djprobe instance hash table */
+static struct hlist_head djprobe_inst_table[DJPROBE_TABLE_SIZE];
+
+#define hash_djprobe(key) \
+	(((unsigned long)(key) >> DJPROBE_BLOCK_BITS) & DJPROBE_TABLE_MASK)
+
+static DECLARE_MUTEX(djprobe_mutex);
+static DEFINE_PER_CPU(struct work_struct, djprobe_works);
+static DECLARE_WAIT_QUEUE_HEAD(djprobe_wqh);
+static atomic_t djprobe_count = ATOMIC_INIT(0);
+
+/* Instruction pages for djprobe's stub code */
+static struct kprobe_insn_page_list djprobe_insn_pages = {
+	HLIST_HEAD_INIT, 0
+};
+
+static inline void __free_djprobe_instance(struct djprobe_instance *djpi)
+{
+	hlist_del(&djpi->hlist);
+	if (djpi->kp.addr) {
+		unregister_kprobe(&(djpi->kp));
+	}
+	if (djpi->stub.insn)
+		__free_insn_slot(&djprobe_insn_pages, djpi->stub.insn);
+	kfree(djpi);
+}
+
+static inline
+    struct djprobe_instance *__create_djprobe_instance(struct djprobe *djp,
+						       void *addr, int size)
+{
+	struct djprobe_instance *djpi;
+	/* allocate a new instance */
+	djpi = kcalloc(1, sizeof(struct djprobe_instance), GFP_ATOMIC);
+	if (djpi == NULL) {
+		goto out;
+	}
+	/* allocate stub */
+	djpi->stub.insn = __get_insn_slot(&djprobe_insn_pages);
+	if (djpi->stub.insn == NULL) {
+		__free_djprobe_instance(djpi);
+		djpi = NULL;
+		goto out;
+	}
+
+	/* attach */
+	djp->inst = djpi;
+	INIT_LIST_HEAD(&djpi->plist);
+	list_add_rcu(&djp->plist, &djpi->plist);
+	djpi->kp.addr = addr;
+	djpi->kp.pre_handler = djprobe_pre_handler;
+	arch_prepare_djprobe_instance(djpi, size);
+
+	INIT_HLIST_NODE(&djpi->hlist);
+	hlist_add_head(&djpi->hlist, &djprobe_inst_table[hash_djprobe(addr)]);
+      out:
+	return djpi;
+}
+
+static struct djprobe_instance *__kprobes __get_djprobe_instance(void *addr,
+								 int size)
+{
+	struct djprobe_instance *djpi;
+	struct hlist_node *node;
+	unsigned long idx, eidx;
+
+	idx = hash_djprobe(addr - ARCH_STUB_INSN_MAX);
+	eidx = ((hash_djprobe(addr + size) + 1) & DJPROBE_TABLE_MASK);
+	do {
+		hlist_for_each_entry(djpi, node, &djprobe_inst_table[idx],
+				     hlist) {
+			if (((long)addr <
+			     (long)djpi->kp.addr + DJPI_ARCH_SIZE(djpi))
+			    && ((long)djpi->kp.addr < (long)addr + size)) {
+				return djpi;
+			}
+		}
+		idx = ((idx + 1) & DJPROBE_TABLE_MASK);
+	}while (idx != eidx);
+
+	return NULL;
+}
+
+struct djprobe_instance *__kprobes get_djprobe_instance(void *addr, int size)
+{
+	struct djprobe_instance *djpi;
+	down(&djprobe_mutex);
+	djpi = __get_djprobe_instance(addr, size);
+	up(&djprobe_mutex);
+	return djpi;
+}
+
+/* This work function invoked while djprobe_mutex is locked. */
+static void __kprobes __work_check_safety(void *data)
+{
+	if (atomic_dec_and_test(&djprobe_count)) {
+		wake_up_all(&djprobe_wqh);
+	}
+}
+
+static void __kprobes __check_safety(void)
+{
+	int cpu;
+	struct work_struct *wk;
+	lock_cpu_hotplug();
+	atomic_set(&djprobe_count, num_online_cpus() - 1);
+	for_each_online_cpu(cpu) {
+		if (cpu == smp_processor_id())
+			continue;
+		wk = &per_cpu(djprobe_works, cpu);
+		INIT_WORK(wk, __work_check_safety, NULL);
+		schedule_delayed_work_on(cpu, wk, 0);
+	}
+	wait_event(djprobe_wqh, (atomic_read(&djprobe_count) == 0));
+	unlock_cpu_hotplug();
+}
+
+int __kprobes register_djprobe(struct djprobe *djp, void *addr, int size)
+{
+	struct djprobe_instance *djpi;
+	struct kprobe *kp;
+	int ret = 0, i;
+
+	BUG_ON(in_interrupt());
+
+	if (size > ARCH_STUB_INSN_MAX || size < ARCH_STUB_INSN_MIN)
+		return -EINVAL;
+
+	if ((ret = in_kprobes_functions((unsigned long)addr)) != 0)
+		return ret;
+
+	down(&djprobe_mutex);
+	INIT_LIST_HEAD(&djp->plist);
+	/* check confliction with other djprobes */
+	djpi = __get_djprobe_instance(addr, size);
+	if (djpi) {
+		if (djpi->kp.addr == addr) {
+			djp->inst = djpi;	/* add to another instance */
+			list_add_rcu(&djp->plist, &djpi->plist);
+		} else {
+			ret = -EEXIST;	/* other djprobes were inserted */
+		}
+		goto out;
+	}
+	djpi = __create_djprobe_instance(djp, addr, size);
+	if (djpi == NULL) {
+		ret = -ENOMEM;
+		goto out;
+	}
+
+	/* check confliction with kprobes */
+	for (i = 0; i < size; i++) {
+		kp = get_kprobe((void *)((long)addr + i));
+		if (kp != NULL) {
+			ret = -EEXIST;	/* a kprobes were inserted */
+			goto fail;
+		}
+	}
+	ret = register_kprobe(&djpi->kp);
+	if (ret < 0) {
+       fail:
+		djpi->kp.addr = NULL;
+		djp->inst = NULL;
+		list_del_rcu(&djp->plist);
+		__free_djprobe_instance(djpi);
+	} else {
+		__check_safety();
+		arch_install_djprobe_instance(djpi);
+	}
+       out:
+	up(&djprobe_mutex);
+	return ret;
+}
+
+void __kprobes unregister_djprobe(struct djprobe *djp)
+{
+	struct djprobe_instance *djpi;
+
+	BUG_ON(in_interrupt());
+
+	down(&djprobe_mutex);
+	djpi = djp->inst;
+	if (djp->plist.next == djp->plist.prev) {
+		arch_uninstall_djprobe_instance(djpi);	/* this requires irq enabled */
+		list_del_rcu(&djp->plist);
+		djp->inst = NULL;
+		__check_safety();
+		__free_djprobe_instance(djpi);
+	} else {
+		list_del_rcu(&djp->plist);
+		djp->inst = NULL;
+	}
+	up(&djprobe_mutex);
+}
+
+static int __init init_djprobe(void)
+{
+	djprobe_insn_pages.insn_size = ARCH_STUB_SIZE;
+	return 0;
+}
+
+__initcall(init_djprobe);
+
+EXPORT_SYMBOL_GPL(register_djprobe);
+EXPORT_SYMBOL_GPL(unregister_djprobe);
diff -Narup linux-2.6.14-mm1.djp.1/kernel/kprobes.c linux-2.6.14-mm1.djp.2/kernel/kprobes.c
--- linux-2.6.14-mm1.djp.1/kernel/kprobes.c	2005-11-08 11:52:46.000000000 +0900
+++ linux-2.6.14-mm1.djp.2/kernel/kprobes.c	2005-11-08 11:56:43.000000000 +0900
@@ -37,6 +37,7 @@
 #include <linux/slab.h>
 #include <linux/module.h>
 #include <linux/moduleloader.h>
+#include <linux/djprobe.h>
 #include <asm-generic/sections.h>
 #include <asm/cacheflush.h>
 #include <asm/errno.h>
@@ -467,7 +468,7 @@ static inline void cleanup_aggr_kprobe(s
 		spin_unlock_irqrestore(&kprobe_lock, flags);
 }

-static int __kprobes in_kprobes_functions(unsigned long addr)
+int __kprobes in_kprobes_functions(unsigned long addr)
 {
 	if (addr >= (unsigned long)__kprobes_text_start
 		&& addr < (unsigned long)__kprobes_text_end)
@@ -483,6 +484,11 @@ int __kprobes register_kprobe(struct kpr

 	if ((ret = in_kprobes_functions((unsigned long) p->addr)) != 0)
 		return ret;
+#ifdef CONFIG_DJPROBE
+	if (p->pre_handler != djprobe_pre_handler &&
+	    get_djprobe_instance(p->addr, 1) != NULL)
+		return -EEXIST;
+#endif /* CONFIG_DJPROBE */
 	if ((ret = arch_prepare_kprobe(p)) != 0)
 		goto rm_kprobe;


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2005-11-22 13:04 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2005-11-14  2:03 [Patch 2/3][Djprobe] Djprobe update for linux-2.6.14-mm1 Zhang, Yanmin
2005-11-22 13:04 ` Masami Hiramatsu
  -- strict thread matches above, loose matches on Subject: below --
2005-11-11  2:33 Zhang, Yanmin
2005-11-11 19:20 ` Masami Hiramatsu
2005-11-02  1:14 [Fwd: [RFC][PATCH 0/3]Djprobe (Direct Jump Probe) for 2.6.14-rc5-mm1] Masami Hiramatsu
2005-11-02  1:51 ` [Fwd: [RFC][PATCH 1/3]Djprobe " Masami Hiramatsu
2005-11-02  1:52   ` [Fwd: [RFC][PATCH 2/3]Djprobe " Masami Hiramatsu
2005-11-02  1:54     ` [Fwd: [RFC][PATCH 3/3]Djprobe " Masami Hiramatsu
2005-11-04 13:29       ` Masami Hiramatsu
2005-11-08 13:26         ` [Patch 2/3][Djprobe] Djprobe update for linux-2.6.14-mm1 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).