From mboxrd@z Thu Jan  1 00:00:00 1970
From: Frederic Weisbecker <fweisbec@gmail.com>
Subject: Re: [PATCH -tip v5 4/7] tracing: add kprobe-based event tracer
Date: Mon, 11 May 2009 23:26:04 +0200
Message-ID: <20090511212602.GA5965@nowhere>
References: <20090509004829.5505.38720.stgit@localhost.localdomain> <20090509004859.5505.18729.stgit@localhost.localdomain> <c62985530905090936v22f6ab1s867d8f8d6a74b95a@mail.gmail.com> <4A05BE81.8070306@redhat.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: Masami Hiramatsu <mhiramat@redhat.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mail-ew0-f224.google.com ([209.85.219.224]:62072 "EHLO
	mail-ew0-f224.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1755510AbZEKV0H (ORCPT <rfc822;kvm@vger.kernel.org>);
	Mon, 11 May 2009 17:26:07 -0400
Content-Disposition: inline
In-Reply-To: <4A05BE81.8070306@redhat.com>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

On Sat, May 09, 2009 at 01:33:53PM -0400, Masami Hiramatsu wrote:
> Fr=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_pr=
obe *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=
_probe();
> >      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.
>=20
> 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().
>=20
> So you don't need any locks there :-)
>=20
> Thank you,
>=20
>=20


Aah, ok :)
So this patch looks sane.

Thanks.