Date: Tue, 16 Sep 2014 09:39:27 -0700 From: Mark Johnston <markj@FreeBSD.org> To: Steven Hartland <smh@freebsd.org> Cc: Matthew Ahrens <mahrens@delphix.com>, hackers@freebsd.org, freebsd-dtrace@freebsd.org Subject: Re: ZFS SET_ERROR dtrace probe possible under FreeBSD? Message-ID: <20140916163927.GA36108@charmander.picturesperfect.net> In-Reply-To: <C5055A802D714A6F9BA38405ABC1B2D8@multiplay.co.uk> References: <AEC968EBE6DE4E56A76DD7578DC25483@multiplay.co.uk> <CAJjvXiF4kPFW--SioqAvR%2BF1kwMgYUkfGqtLd4ZHh0jWhrNN5Q@mail.gmail.com> <20140916031318.GB26720@charmander.picturesperfect.net> <C5055A802D714A6F9BA38405ABC1B2D8@multiplay.co.uk>
next in thread | previous in thread | raw e-mail | index | archive | help
On Tue, Sep 16, 2014 at 11:05:09AM +0100, Steven Hartland wrote: > > ----- Original Message ----- > From: "Mark Johnston" <markj@FreeBSD.org> > > > > On Mon, Sep 15, 2014 at 07:59:50PM -0700, Matthew Ahrens wrote: > >> Disclaimer: I'm not an expert in FreeBSD dtrace. > >> > >> It looks like the FreeBSD kernel uses these declaration for kernel SDT > >> probes, in sdt.h: > >> > >> 333 <http://src.illumos.org/source/xref/freebsd-head/sys/sys/sdt.h#333>#*define* > >> DTRACE_PROBE1 <http://src.illumos.org/source/s?refs=DTRACE_PROBE1&project=freebsd-head>(name > >> <http://src.illumos.org/source/s?defs=name&project=freebsd-head>, > >> type0 <http://src.illumos.org/source/s?defs=type0&project=freebsd-head>, > >> arg0 <http://src.illumos.org/source/s?defs=arg0&project=freebsd-head>) \334 > >> <http://src.illumos.org/source/xref/freebsd-head/sys/sys/sdt.h#334>; DTRACE_PROBE_IMPL_START > >> <http://src.illumos.org/source/xref/freebsd-head/sys/sys/sdt.h#DTRACE_PROBE_IMPL_START>(name > >> <http://src.illumos.org/source/s?defs=name&project=freebsd-head>, arg0 > >> <http://src.illumos.org/source/s?defs=arg0&project=freebsd-head>, 0, > >> 0, 0, 0) \335 > >> <http://src.illumos.org/source/xref/freebsd-head/sys/sys/sdt.h#335>; SDT_PROBE_ARGTYPE > >> <http://src.illumos.org/source/s?defs=SDT_PROBE_ARGTYPE&project=freebsd-head>(sdt > >> <http://src.illumos.org/source/xref/freebsd-head/sys/sys/sdt.h#sdt>, , > >> , name <http://src.illumos.org/source/s?defs=name&project=freebsd-head>, > >> 0, #type0 <http://src.illumos.org/source/s?defs=type0&project=freebsd-head>, > >> NULL <http://src.illumos.org/source/s?defs=NULL&project=freebsd-head>); \336 > >> <http://src.illumos.org/source/xref/freebsd-head/sys/sys/sdt.h#336>; DTRACE_PROBE_IMPL_END > >> <http://src.illumos.org/source/xref/freebsd-head/sys/sys/sdt.h#DTRACE_PROBE_IMPL_END>; > >> > >> > >> 324 <http://src.illumos.org/source/xref/freebsd-head/sys/sys/sdt.h#324>#*define* DTRACE_PROBE_IMPL_START > >> <http://src.illumos.org/source/s?refs=DTRACE_PROBE_IMPL_START&project=freebsd-head>(name > >> <http://src.illumos.org/source/s?defs=name&project=freebsd-head>, arg0 > >> <http://src.illumos.org/source/s?defs=arg0&project=freebsd-head>, arg1 > >> <http://src.illumos.org/source/s?defs=arg1&project=freebsd-head>, arg2 > >> <http://src.illumos.org/source/s?defs=arg2&project=freebsd-head>, arg3 > >> <http://src.illumos.org/source/s?defs=arg3&project=freebsd-head>, arg4 > >> <http://src.illumos.org/source/s?defs=arg4&project=freebsd-head>) *do* > >> { \325 <http://src.illumos.org/source/xref/freebsd-head/sys/sys/sdt.h#325>; *static* > >> SDT_PROBE_DEFINE > >> <http://src.illumos.org/source/s?defs=SDT_PROBE_DEFINE&project=freebsd-head>(sdt > >> <http://src.illumos.org/source/xref/freebsd-head/sys/sys/sdt.h#sdt>, , > >> , name <http://src.illumos.org/source/s?defs=name&project=freebsd-head>); > >> \326 <http://src.illumos.org/source/xref/freebsd-head/sys/sys/sdt.h#326>; SDT_PROBE > >> <http://src.illumos.org/source/s?defs=SDT_PROBE&project=freebsd-head>(sdt > >> <http://src.illumos.org/source/xref/freebsd-head/sys/sys/sdt.h#sdt>, , > >> , name <http://src.illumos.org/source/s?defs=name&project=freebsd-head>, > >> arg0 <http://src.illumos.org/source/s?defs=arg0&project=freebsd-head>, > >> arg1 <http://src.illumos.org/source/s?defs=arg1&project=freebsd-head>, > >> arg2 <http://src.illumos.org/source/s?defs=arg2&project=freebsd-head>, > >> arg3 <http://src.illumos.org/source/s?defs=arg3&project=freebsd-head>, > >> arg4 <http://src.illumos.org/source/s?defs=arg4&project=freebsd-head>);327 > >> <http://src.illumos.org/source/xref/freebsd-head/sys/sys/sdt.h#327>#*define* > >> DTRACE_PROBE_IMPL_END > >> <http://src.illumos.org/source/s?refs=DTRACE_PROBE_IMPL_END&project=freebsd-head>; } > >> *while* (0) > >> > >> > >> 163 <http://src.illumos.org/source/xref/freebsd-head/sys/sys/sdt.h#163>#*define* > >> SDT_PROBE <http://src.illumos.org/source/s?refs=SDT_PROBE&project=freebsd-head>(prov > >> <http://src.illumos.org/source/xref/freebsd-head/sys/sys/sdt.h#prov>, > >> mod <http://src.illumos.org/source/xref/freebsd-head/sys/sys/sdt.h#mod>, > >> func <http://src.illumos.org/source/xref/freebsd-head/sys/sys/sdt.h#func>, > >> name <http://src.illumos.org/source/s?defs=name&project=freebsd-head>, > >> arg0 <http://src.illumos.org/source/s?defs=arg0&project=freebsd-head>, > >> arg1 <http://src.illumos.org/source/s?defs=arg1&project=freebsd-head>, > >> arg2 <http://src.illumos.org/source/s?defs=arg2&project=freebsd-head>, > >> arg3 <http://src.illumos.org/source/s?defs=arg3&project=freebsd-head>, > >> arg4 <http://src.illumos.org/source/s?defs=arg4&project=freebsd-head>) *do* > >> { \164 <http://src.illumos.org/source/xref/freebsd-head/sys/sys/sdt.h#164>; *if* > >> (sdt_ <http://src.illumos.org/source/s?defs=sdt_&project=freebsd-head>##prov > >> <http://src.illumos.org/source/xref/freebsd-head/sys/sys/sdt.h#prov>##_##mod > >> <http://src.illumos.org/source/xref/freebsd-head/sys/sys/sdt.h#mod>##_##func > >> <http://src.illumos.org/source/xref/freebsd-head/sys/sys/sdt.h#func>##_##name > >> <http://src.illumos.org/source/s?defs=name&project=freebsd-head>->id > >> <http://src.illumos.org/source/s?defs=id&project=freebsd-head>) \165 > >> <http://src.illumos.org/source/xref/freebsd-head/sys/sys/sdt.h#165>; (*sdt_probe_func > >> <http://src.illumos.org/source/s?defs=sdt_probe_func&project=freebsd-head>)(sdt_ > >> <http://src.illumos.org/source/s?defs=sdt_&project=freebsd-head>##prov > >> <http://src.illumos.org/source/xref/freebsd-head/sys/sys/sdt.h#prov>##_##mod > >> <http://src.illumos.org/source/xref/freebsd-head/sys/sys/sdt.h#mod>##_##func > >> <http://src.illumos.org/source/xref/freebsd-head/sys/sys/sdt.h#func>##_##name > >> <http://src.illumos.org/source/s?defs=name&project=freebsd-head>->id > >> <http://src.illumos.org/source/s?defs=id&project=freebsd-head>, \166 > >> <http://src.illumos.org/source/xref/freebsd-head/sys/sys/sdt.h#166>; > >> (uintptr_t <http://src.illumos.org/source/s?defs=uintptr_t&project=freebsd-head>) > >> arg0 <http://src.illumos.org/source/s?defs=arg0&project=freebsd-head>, > >> (uintptr_t <http://src.illumos.org/source/s?defs=uintptr_t&project=freebsd-head>) > >> arg1 <http://src.illumos.org/source/s?defs=arg1&project=freebsd-head>, > >> (uintptr_t <http://src.illumos.org/source/s?defs=uintptr_t&project=freebsd-head>) > >> arg2 <http://src.illumos.org/source/s?defs=arg2&project=freebsd-head>, \167 > >> <http://src.illumos.org/source/xref/freebsd-head/sys/sys/sdt.h#167>; > >> (uintptr_t <http://src.illumos.org/source/s?defs=uintptr_t&project=freebsd-head>) > >> arg3 <http://src.illumos.org/source/s?defs=arg3&project=freebsd-head>, > >> (uintptr_t <http://src.illumos.org/source/s?defs=uintptr_t&project=freebsd-head>) > >> arg4 <http://src.illumos.org/source/s?defs=arg4&project=freebsd-head>); \168 > >> <http://src.illumos.org/source/xref/freebsd-head/sys/sys/sdt.h#168>} > >> *while* (0) > >> > >> > >> To do the equivalent "extra static" magic, you will need to expand out the > >> DTRACE_PROBE1 macro. I think it should look something like: > >> > >> SDT_PROBE_DEFINE1(sdt, zfs, , set__error, "int"); > >> > >> #define SET_ERROR(err) \ > >> ((sdt_sdt_zfs__set__error->id && \ > >> (*sdt_probe_func)(sdt_sdt_zfs__set__error->id, (uintptr_t)err, 0, 0, 0, > >> 0)), \ > >> err) > > > > I think it would need to be > > > > SDT_PROBE_DECLARE(sdt, , , set__error); > > > > #define SET_ERROR(err) ... > > > > in the compat sdt.h, and then kern_dtrace.c or so would contain > > > > SDT_PROBE_DEFINE1(sdt, , , set__error, "int"); > > > > Note that the module shouldn't be hard-coded - it'll be filled in when > > the probes are created by the SDT code. > > Thanks guys for the pointers, I tried the following slightly ammended > from Matts original due to sdt_probe_func being a void return: > > Index: sys/cddl/compat/opensolaris/sys/sdt.h > =================================================================== > --- sys/cddl/compat/opensolaris/sys/sdt.h (revision 271518) > +++ sys/cddl/compat/opensolaris/sys/sdt.h (working copy) > @@ -34,6 +34,11 @@ > #endif > #include_next <sys/sdt.h> > > -#define SET_ERROR(err) (err) > +SDT_PROBE_DECLARE(sdt, , , set__error); > > +#define SET_ERROR(err) \ > + ((sdt_sdt___set__error->id ? \ > + (*sdt_probe_func)(sdt_sdt___set__error->id, \ > + (uintptr_t)err, 0, 0, 0, 0) : 0), err) > + > #endif /* _OPENSOLARIS_SYS_SDT_H_ */ > Index: sys/kern/kern_dtrace.c > =================================================================== > --- sys/kern/kern_dtrace.c (revision 271518) > +++ sys/kern/kern_dtrace.c (working copy) > @@ -48,6 +48,8 @@ FEATURE(kdtrace_hooks, > > static MALLOC_DEFINE(M_KDTRACE, "kdtrace", "DTrace hooks"); > > +SDT_PROBE_DEFINE1(sdt, , , set__error, "int"); > + > /* Hooks used in the machine-dependent trap handlers. */ > dtrace_trap_func_t dtrace_trap_func; > dtrace_doubletrap_func_t dtrace_doubletrap_func; > > But it fails with:- > /usr/src/sys/kern/kern_dtrace.c:51:24: error: expected identifier > SDT_PROBE_DEFINE1(sdt, , , set__error, "int"); > ^ > /usr/src/sys/kern/kern_dtrace.c:51:1: error: type specifier missing, defaults to 'int' [-Werror,-Wimplicit-int] > SDT_PROBE_DEFINE1(sdt, , , set__error, "int"); > > All other calls to SDT_PROBE_DEFINE1 seem to define all args e.g. > sys/kern/kern_exit.c:SDT_PROBE_DEFINE1(proc, kernel, , exit, "int"); > > Are you sure they should be ommited? You'll need to #include <sys/sdt.h> in kern_dtrace.c. > > Also is kern_dtrace.c the correct place for the probe define given > its zfs specific? I was just thinking that it might be useful elsewhere too. For now, perhaps the best place is in a new file under sys/cddl/compat/opensolaris/kern. Then it'll be part of opensolaris.ko and SET_ERROR will work for any illumos code in the tree, not just ZFS. -Mark
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20140916163927.GA36108>