Skip site navigation (1)Skip section navigation (2)
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>