Date: Tue, 16 Sep 2014 11:05:09 +0100 From: "Steven Hartland" <smh@freebsd.org> To: "Mark Johnston" <markj@FreeBSD.org>, "Matthew Ahrens" <mahrens@delphix.com> Cc: hackers@freebsd.org, freebsd-dtrace@freebsd.org Subject: Re: ZFS SET_ERROR dtrace probe possible under FreeBSD? Message-ID: <C5055A802D714A6F9BA38405ABC1B2D8@multiplay.co.uk> References: <AEC968EBE6DE4E56A76DD7578DC25483@multiplay.co.uk> <CAJjvXiF4kPFW--SioqAvR%2BF1kwMgYUkfGqtLd4ZHh0jWhrNN5Q@mail.gmail.com> <20140916031318.GB26720@charmander.picturesperfect.net>
next in thread | previous in thread | raw e-mail | index | archive | help
----- 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? Also is kern_dtrace.c the correct place for the probe define given its zfs specific? Regards Steve
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?C5055A802D714A6F9BA38405ABC1B2D8>