Message-ID: <47384D34.6070000@redhat.com>
Date: Mon, 12 Nov 2007 07:55:16 -0500
From: Masami Hiramatsu <mhiramat@redhat.com>
User-Agent: Thunderbird 2.0.0.5 (X11/20070727)
MIME-Version: 1.0
To: Arjan van de Ven <arjan@infradead.org>
CC: linux-kernel@vger.kernel.org, tglx@tglx.de, mingo@elte.hu, hpa@zytor.com,
       ananth@in.ibm.com
Subject: Re: [patch] prepare kprobes code for x86 unification
References: <20071024150047.4144fda0@laptopd505.fenrus.org>
In-Reply-To: <20071024150047.4144fda0@laptopd505.fenrus.org>
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 2129
Lines: 59

Hi Arjan,

Arjan van de Ven wrote:
> From: Arjan van de Ven <arjan@linux.intel.com>
> Subject: prepare kprobes code for x86 unification
> 
> This patch is a first step towards unification of the kprobes
> infrastructure between 32 and 64 bit x86; the patch is mostly
> about removing spurious whitespace changes and about adding
> harmless includes and the like to make the 32/64 files more
> identical.
> 
> Signed-off-by: Arjan van de Ven

I got a bug in this patch. In kprobes_64.c, Since this added __kprobes
to kretprobe_trampoline_holder(), the arch_init_kprobes() failed to
insert a kprobe into that function, and the initialization was failed.
This means that kprobe_exceptions_notify() was not registered to
die_notifier_chain. So, it caused a kernel crash.

> @@ -385,7 +428,7 @@ no_kprobe:
>   * here. When a retprobed function returns, this probe is hit and
>   * trampoline_probe_handler() runs, calling the kretprobe's handler.
>   */
> - void kretprobe_trampoline_holder(void)
> + void __kprobes kretprobe_trampoline_holder(void)
>   {
>   	asm volatile (  ".global kretprobe_trampoline\n"
>   			"kretprobe_trampoline: \n"

On i386, I introduced kretprobe-booster, which did not need to insert
a kprobe. Thus, I also added __kprobes to kretprobe_trampoline_holder()
on i386.

I think we can remove __kprobes from kretprobe_trampoline_holder()
in kprobes_32.c if you'd like to make the both same. However, it
is not safe. So we need a comment on top explaining why it does not
have that annotation and users must take care of it.

By the way, I'm porting boosters to x86-64 (and ia64). Since some parts
of that will be covered by this patch, I'd like to know your unification
plan.

Best regards,

-- 
Masami Hiramatsu

Software Engineer
Hitachi Computer Products (America) Inc.
Software Solutions Division

e-mail: mhiramat@redhat.com, masami.hiramatsu.pt@hitachi.com

-
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/