Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 15 Sep 2014 20:13:19 -0700
From:      Mark Johnston <markj@FreeBSD.org>
To:        Matthew Ahrens <mahrens@delphix.com>
Cc:        Steven Hartland <smh@freebsd.org>, hackers@freebsd.org, freebsd-dtrace@freebsd.org
Subject:   Re: ZFS SET_ERROR dtrace probe possible under FreeBSD?
Message-ID:  <20140916031318.GB26720@charmander.picturesperfect.net>
In-Reply-To: <CAJjvXiF4kPFW--SioqAvR%2BF1kwMgYUkfGqtLd4ZHh0jWhrNN5Q@mail.gmail.com>
References:  <AEC968EBE6DE4E56A76DD7578DC25483@multiplay.co.uk> <CAJjvXiF4kPFW--SioqAvR%2BF1kwMgYUkfGqtLd4ZHh0jWhrNN5Q@mail.gmail.com>

next in thread | previous in thread | raw e-mail | index | archive | help
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.

-Mark

> 
> Note that SET_ERROR is an expansion of SDT_PROBE, but rewritten to be a
> single statement, so that we can do the same trick with the comma operator.
> 
> --matt
> 
> On Mon, Sep 15, 2014 at 7:03 PM, Steven Hartland <smh@freebsd.org> wrote:
> 
> > The following commit added SET_ERROR dtrace probes to illumos
> > https://github.com/illumos/illumos-gate/commit/be6fd75
> >
> > Now we have all the SET_ERROR calls but the FreeBSD's ZFS implementation
> > but our SET_ERROR in sys/cddl/compat/opensolaris/sys/sdt.h is simply:
> > #define SET_ERROR(err)  (err)
> >
> > I tried using the illumos version but that resulted being unable
> > to mount ZFS root, so clearly not right.
> >
> > /*
> > * the set-error SDT probe is extra static, in that we declare its fake
> > * function literally, rather than with the DTRACE_PROBE1() macro. This is
> > * necessary so that SET_ERROR() can evaluate to a value, which wouldn't
> > * be possible if it required multiple statements (to declare the function
> > * and then call it).
> > *
> > * SET_ERROR() uses the comma operator so that it can be used without much
> > * additional code. For example, "return (EINVAL);" becomes
> > * "return (SET_ERROR(EINVAL));". Note that the argument will be evaluated
> > * twice, so it should not have side effects (e.g. something like:
> > * "return (SET_ERROR(log_error(EINVAL, info)));" would log the error
> > twice).
> > */
> > extern void __dtrace_probe_set__error(uintptr_t);
> > #define SET_ERROR(err) (__dtrace_probe_set__error(err), err)
> >
> >
> > For those that know the the ins and outs of our dtrace is it
> > possible for us to add support for this trace point?
> >
> >    Regards
> >    Steve



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20140916031318.GB26720>