From mboxrd@z Thu Jan  1 00:00:00 1970
From: Masami Hiramatsu <mhiramat@redhat.com>
Subject: Re: [PATCH -tip v5 4/7] tracing: add kprobe-based event tracer
Date: Sat, 09 May 2009 13:33:53 -0400
Message-ID: <4A05BE81.8070306@redhat.com>
References: <20090509004829.5505.38720.stgit@localhost.localdomain>	 <20090509004859.5505.18729.stgit@localhost.localdomain> <c62985530905090936v22f6ab1s867d8f8d6a74b95a@mail.gmail.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: QUOTED-PRINTABLE
Cc: Ingo Molnar <mingo@elte.hu>, Steven Rostedt <rostedt@goodmis.org>,
	lkml <linux-kernel@vger.kernel.org>,
	systemtap <systemtap@sources.redhat.com>,
	kvm <kvm@vger.kernel.org>,
	Ananth N Mavinakayanahalli <ananth@in.ibm.com>
To: =?ISO-8859-1?Q?Fr=E9d=E9ric_Weisbecker?= <fweisbec@gmail.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mx2.redhat.com ([66.187.237.31]:57358 "EHLO mx2.redhat.com"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1752121AbZEIRdG (ORCPT <rfc822;kvm@vger.kernel.org>);
	Sat, 9 May 2009 13:33:06 -0400
In-Reply-To: <c62985530905090936v22f6ab1s867d8f8d6a74b95a@mail.gmail.com>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

=46r=E9d=E9ric Weisbecker wrote:
> Hi,
>=20
> 2009/5/9 Masami Hiramatsu <mhiramat@redhat.com>:
[...]
>> +
>> +/* event recording functions */
>> +static void kprobe_trace_record(unsigned long ip, struct trace_prob=
e *tp,
>> +                               struct pt_regs *regs)
>> +{
>> +       __trace_bprintk(ip, "%s%s%+ld\n",
>> +                       probe_is_return(tp) ? "<-" : "@",
>> +                       probe_symbol(tp), probe_offset(tp));
>> +}
>=20
>=20
>=20
> What happens here if you have:
>=20
> kprobe_trace_record() {
>       probe_symbol() {
>             ....                         probes_open() {
>                                               cleanup_all_probes() {
>                                                          free_trace_p=
robe();
>      return tp->symbol ? ....; //crack!
>
> I wonder if you shouldn't use a per_cpu list of probes,
> spinlocked/irqsaved  accessed
> and also a kind of prevention against nmi.

Sure, cleanup_all_probes() invokes unregister_kprobe() via
unregister_trace_probe(), which waits running probe-handlers by
using synchronize_sched()(because kprobes disables preemption
around its handlers), before free_trace_probe().

So you don't need any locks there :-)

Thank you,


--=20
Masami Hiramatsu

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

e-mail: mhiramat@redhat.com