From owner-freebsd-dtrace@FreeBSD.ORG Thu May 30 13:53:06 2013 Return-Path: Delivered-To: freebsd-dtrace@freebsd.org Received: from mx1.freebsd.org (mx1.FreeBSD.org [8.8.178.115]) by hub.freebsd.org (Postfix) with ESMTP id 64B5F41C; Thu, 30 May 2013 13:53:06 +0000 (UTC) (envelope-from markjdb@gmail.com) Received: from mail-ie0-x22f.google.com (mail-ie0-x22f.google.com [IPv6:2607:f8b0:4001:c03::22f]) by mx1.freebsd.org (Postfix) with ESMTP id 2EE3D344; Thu, 30 May 2013 13:53:06 +0000 (UTC) Received: by mail-ie0-f175.google.com with SMTP id tp5so623803ieb.6 for ; Thu, 30 May 2013 06:53:05 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=sender:date:from:to:cc:subject:message-id:references:mime-version :content-type:content-disposition:in-reply-to:user-agent; bh=xYumpEGiYuEo3aIfBV0dAH1WeEUuRUvXxDN/nFSMQL4=; b=hmecbRN2SFvCmpN/325ulVs1Dew2ZSQGOBrxBrDeAiWPMIL+QIubALSxUbTV6OnxM1 VoZKQlPDD0gPcHthRK4Pd76BY+xH7fmJG15RfK83hPWNMExHf+W++3KBregw8FsO771r uHGjr4rnzk/dM8z4sUw2vgZnaQk6ECDPd+NE81qDGc6SUn723K0MgMiSbn0FBT88qjoF VixQ/tY0N/UjWZSWm3oHW1X5WCtlgx2Tqyy7rV1RyVcGhWxbyp6fVP3xYP+gFTjJeK6L 8mtgGCSBU1RU1dvm9nyHHaR1DMHcnuQc/AdMMnwlYU9vumgPwc0n5ttmIr/C/+TJOo7D 8qHg== X-Received: by 10.50.130.78 with SMTP id oc14mr3763600igb.48.1369921985831; Thu, 30 May 2013 06:53:05 -0700 (PDT) Received: from gloom.sandvine.com ([64.7.137.182]) by mx.google.com with ESMTPSA id d9sm27474724igr.4.2013.05.30.06.53.04 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Thu, 30 May 2013 06:53:05 -0700 (PDT) Sender: Mark Johnston Date: Thu, 30 May 2013 09:52:43 -0400 From: Mark Johnston To: Brendan Gregg Subject: Re: adding SDT probe functions with 6+ DTrace arguments Message-ID: <20130530135243.GA1406@gloom.sandvine.com> References: <20130530040705.GA40320@gloom> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) Cc: rstone@freebsd.org, freebsd-dtrace@freebsd.org X-BeenThere: freebsd-dtrace@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: "A discussion list for developers working on DTrace in FreeBSD." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 30 May 2013 13:53:06 -0000 On Wed, May 29, 2013 at 11:14:32PM -0700, Brendan Gregg wrote: > G'Day Mark, > > As this might be interesting: to check that DTRACE_PROBE7 worked, in > Solaris I had added an sdt:::test probe, which could be invoked by the > DTrace test suite (sdt/tst.sdtargs.c) by calling uadmin() with A_SDTTEST: > > case A_SDTTEST: > { > DTRACE_PROBE7(test, int, 1, int, 2, int, 3, int, 4, int, 5, > int, 6, int, 7); > break; > } Yep, this is the test case that I mentioned below. I had to do a bit of work to port it - uadmin doesn't exist on FreeBSD (and before yesterday, I had never heard of it :). I fixed the test in the patch below: http://people.freebsd.org/~markj/patches/sdt-many-args.diff As mentioned below, there are a couple of off-by-one bugs in the way dtrace_getarg() gets extra arguments off the stack. One of them is FreeBSD-specific (our SDT implementation doesn't set the aframes parameter properly), and the other is an amd64-specific bug which I think also exists in illumos; this is fixed in the change to dtrace_isa.c in the above patch. Here's a little diagram of how the local variables are set up at the end of dtrace_getarg() when fetching the sixth argument of an SDT probe on FreeBSD: ... --------------- | arg 1 | --------------- <----- stack | arg 0 | stack[-1] --------------- | ret addr | --------------- | frame ptr | --------------- <----- fp This is the stack frame of dtrace_probe(). To get at arg 0 - which is the sixth argument of a DTrace probe, the local variable 'arg' should be -1; in the current code, it ends up being 0. Specifically, in this case dtrace_getarg() is called with arg == 5. Then we increment it once a bit later, and then we put arg -= (inreg + 1); so arg ends up being 0 when it should be -1. Then the seventh argument is at stack[0], so 'arg' should be 0 but is 1, and so on. I think this bug comes from looking at the i386 code; the difference there is that all arguments to the probe are on the stack, and the first argument is the probe ID. So we _want_ to skip it. Then 'stack' is basically a pointer to the array of probe arguments. -Mark > > Getting the tcp and ip providers working will be great. > > Brendan > > On Wed, May 29, 2013 at 11:02 PM, Adam Leventhal wrote: > > > Hey Mark, > > > > I'm just now familiarizing myself with the way that FreeBSD DTrace > > handles SDT probes, but certainly having more arguments (as needed) > > makes sense, and the approach you've taken seems in keeping with the > > existing methodology. > > > > Adam > > > > On Wed, May 29, 2013 at 9:07 PM, Mark Johnston wrote: > > > Hello, > > > > > > Does anyone have objections or comments on the patch below? It simply > > > makes it possible to use the SDT_* macros to implement DTrace probes > > > with more than 5 arguments. dtrace_probe() only takes 5 arguments, so > > > some ugly casting is needed, but it works properly in my testing on > > > amd64. In fact, there are a couple of bugs in the way that DTrace > > > fetches the 6th argument (or the 7th, and so on) from the probe site, > > > but I'll be fixing that soon as well, along with the test case that's > > > supposed to detect this problem in the first place. > > > > > > I don't know of any existing SDT providers in FreeBSD that would use > > > these macros, but the ip, iscsi and tcp providers will - hence my > > > interest in making sure that this functionality works properly. > > > > > > Thanks! > > > -Mark > > > > > > diff --git a/sys/sys/sdt.h b/sys/sys/sdt.h > > > index 522e1f2..21edd53 100644 > > > --- a/sys/sys/sdt.h > > > +++ b/sys/sys/sdt.h > > > @@ -91,6 +91,10 @@ > > > #define SDT_PROBE_DEFINE3(prov, mod, func, name, sname, arg0, > > arg1, arg2) > > > #define SDT_PROBE_DEFINE4(prov, mod, func, name, sname, arg0, > > arg1, arg2, arg3) > > > #define SDT_PROBE_DEFINE5(prov, mod, func, name, sname, arg0, > > arg1, arg2, arg3, arg4) > > > +#define SDT_PROBE_DEFINE6(prov, mod, func, name, snamp, arg0, > > arg1, arg2, \ > > > + arg3, arg4, arg5) > > > +#define SDT_PROBE_DEFINE7(prov, mod, func, name, snamp, arg0, > > arg1, arg2, \ > > > + arg3, arg4, arg5, arg6) > > > > > > #define SDT_PROBE0(prov, mod, func, name) > > > #define SDT_PROBE1(prov, mod, func, name, arg0) > > > @@ -98,6 +102,9 @@ > > > #define SDT_PROBE3(prov, mod, func, name, arg0, arg1, arg2) > > > #define SDT_PROBE4(prov, mod, func, name, arg0, arg1, arg2, arg3) > > > #define SDT_PROBE5(prov, mod, func, name, arg0, arg1, arg2, > > arg3, arg4) > > > +#define SDT_PROBE6(prov, mod, func, name, arg0, arg1, arg2, > > arg3, arg4, arg5) > > > +#define SDT_PROBE7(prov, mod, func, name, arg0, arg1, arg2, > > arg3, arg4, arg5, \ > > > + arg6) > > > > > > #else > > > > > > @@ -233,6 +240,27 @@ struct sdt_provider { > > > SDT_PROBE_ARGTYPE(prov, mod, func, name, 3, arg3); \ > > > SDT_PROBE_ARGTYPE(prov, mod, func, name, 4, arg4) > > > > > > +#define SDT_PROBE_DEFINE6(prov, mod, func, name, sname, arg0, > > arg1, arg2, arg3,\ > > > + arg4, arg5) \ > > > + SDT_PROBE_DEFINE(prov, mod, func, name, sname); \ > > > + SDT_PROBE_ARGTYPE(prov, mod, func, name, 0, arg0); \ > > > + SDT_PROBE_ARGTYPE(prov, mod, func, name, 1, arg1); \ > > > + SDT_PROBE_ARGTYPE(prov, mod, func, name, 2, arg2); \ > > > + SDT_PROBE_ARGTYPE(prov, mod, func, name, 3, arg3); \ > > > + SDT_PROBE_ARGTYPE(prov, mod, func, name, 4, arg4); \ > > > + SDT_PROBE_ARGTYPE(prov, mod, func, name, 5, arg5); > > > + > > > +#define SDT_PROBE_DEFINE7(prov, mod, func, name, sname, arg0, > > arg1, arg2, arg3,\ > > > + arg4, arg5, arg6) \ > > > + SDT_PROBE_DEFINE(prov, mod, func, name, sname); \ > > > + SDT_PROBE_ARGTYPE(prov, mod, func, name, 0, arg0); \ > > > + SDT_PROBE_ARGTYPE(prov, mod, func, name, 1, arg1); \ > > > + SDT_PROBE_ARGTYPE(prov, mod, func, name, 2, arg2); \ > > > + SDT_PROBE_ARGTYPE(prov, mod, func, name, 3, arg3); \ > > > + SDT_PROBE_ARGTYPE(prov, mod, func, name, 4, arg4); \ > > > + SDT_PROBE_ARGTYPE(prov, mod, func, name, 5, arg5); \ > > > + SDT_PROBE_ARGTYPE(prov, mod, func, name, 6, arg6); > > > + > > > #define SDT_PROBE0(prov, mod, func, name) > > \ > > > SDT_PROBE(prov, mod, func, name, 0, 0, 0, 0, 0) > > > #define SDT_PROBE1(prov, mod, func, name, arg0) > > \ > > > @@ -245,6 +273,27 @@ struct sdt_provider { > > > SDT_PROBE(prov, mod, func, name, arg0, arg1, arg2, arg3, 0) > > > #define SDT_PROBE5(prov, mod, func, name, arg0, arg1, arg2, > > arg3, arg4) \ > > > SDT_PROBE(prov, mod, func, name, arg0, arg1, arg2, arg3, arg4) > > > +#define SDT_PROBE6(prov, mod, func, name, arg0, arg1, arg2, > > arg3, arg4, arg5) \ > > > + do { > > \ > > > + if (sdt_##prov##_##mod##_##func##_##name->id) > > \ > > > + (*(void (*)(uint32_t, uintptr_t, uintptr_t, > > uintptr_t, \ > > > + uintptr_t, uintptr_t, > > uintptr_t))sdt_probe_func)( \ > > > + sdt_##prov##_##mod##_##func##_##name->id, > > \ > > > + (uintptr_t)arg0, (uintptr_t)arg1, > > (uintptr_t)arg2, \ > > > + (uintptr_t)arg3, (uintptr_t)arg4, > > (uintptr_t)arg5);\ > > > + } while (0) > > > +#define SDT_PROBE7(prov, mod, func, name, arg0, arg1, arg2, > > arg3, arg4, arg5, \ > > > + arg6) > > \ > > > + do { > > \ > > > + if (sdt_##prov##_##mod##_##func##_##name->id) > > \ > > > + (*(void (*)(uint32_t, uintptr_t, uintptr_t, > > uintptr_t, \ > > > + uintptr_t, uintptr_t, uintptr_t, uintptr_t)) > > \ > > > + sdt_probe_func)( > > \ > > > + sdt_##prov##_##mod##_##func##_##name->id, > > \ > > > + (uintptr_t)arg0, (uintptr_t)arg1, > > (uintptr_t)arg2, \ > > > + (uintptr_t)arg3, (uintptr_t)arg4, > > (uintptr_t)arg5, \ > > > + (uintptr_t)arg6); > > \ > > > + } while (0) > > > > > > typedef int (*sdt_argtype_listall_func_t)(struct sdt_argtype *, void *); > > > typedef int (*sdt_probe_listall_func_t)(struct sdt_probe *, void *); > > > _______________________________________________ > > > freebsd-dtrace@freebsd.org mailing list > > > https://lists.freebsd.org/mailman/listinfo/freebsd-dtrace > > > To unsubscribe, send any mail to "freebsd-dtrace-unsubscribe@freebsd.org > > " > > > > > > > > -- > > Adam Leventhal > > CTO, Delphix > > http://blog.delphix.com/ahl > > _______________________________________________ > > freebsd-dtrace@freebsd.org mailing list > > https://lists.freebsd.org/mailman/listinfo/freebsd-dtrace > > To unsubscribe, send any mail to "freebsd-dtrace-unsubscribe@freebsd.org" > > > > > > -- > Brendan Gregg, Joyent http://dtrace.org/blogs/brendan