From owner-freebsd-hackers@FreeBSD.ORG Tue Sep 16 16:39:50 2014 Return-Path: Delivered-To: hackers@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 591C2F93; Tue, 16 Sep 2014 16:39:50 +0000 (UTC) Received: from mail-pa0-x22d.google.com (mail-pa0-x22d.google.com [IPv6:2607:f8b0:400e:c03::22d]) (using TLSv1 with cipher ECDHE-RSA-RC4-SHA (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 1B22D94D; Tue, 16 Sep 2014 16:39:50 +0000 (UTC) Received: by mail-pa0-f45.google.com with SMTP id rd3so130451pab.32 for ; Tue, 16 Sep 2014 09:39:49 -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=+jQasUqFR6AdLMLCMImEIhRZBrhWURC3D1kiUehdSIw=; b=p1rzfvOyztlyn+xeaTqhWolEd8o+h0ia+F8HnRovtZVok7zKgGZGMDa6Vwwd8/SwRb X+ovhjFeWbrTf47gpoGwz5khj0CftrYxKxI2zbkPl40mYoJMOYGzy8TIWwzr/hT1RH0/ wRQmrgf6eWrTAYKghIDepbiOWwqIuspu/LO6rQ8rqqZpD1RomEQBFmTq6+/uTPxWgcZw yjf94rzTuE06crhJtlLKlN3cGJ48DmyGsM/iUuegz28tCSUNNmkfb81/elK6NG/kvZhv NK8RL5V5qEDmh3d4pEYWbUVnSy3nh2ukzO9FIlja/gu/XCDNyBSDCq7n8ZPWVD+5HffD glRQ== X-Received: by 10.66.139.16 with SMTP id qu16mr17170212pab.153.1410885589467; Tue, 16 Sep 2014 09:39:49 -0700 (PDT) Received: from charmander.picturesperfect.net (c-67-182-131-225.hsd1.wa.comcast.net. [67.182.131.225]) by mx.google.com with ESMTPSA id ca4sm7120449pad.0.2014.09.16.09.39.47 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 16 Sep 2014 09:39:48 -0700 (PDT) Sender: Mark Johnston Date: Tue, 16 Sep 2014 09:39:27 -0700 From: Mark Johnston To: Steven Hartland Subject: Re: ZFS SET_ERROR dtrace probe possible under FreeBSD? Message-ID: <20140916163927.GA36108@charmander.picturesperfect.net> References: <20140916031318.GB26720@charmander.picturesperfect.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.23 (2014-03-12) Cc: Matthew Ahrens , hackers@freebsd.org, freebsd-dtrace@freebsd.org X-BeenThere: freebsd-hackers@freebsd.org X-Mailman-Version: 2.1.18-1 Precedence: list List-Id: Technical Discussions relating to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 16 Sep 2014 16:39:50 -0000 On Tue, Sep 16, 2014 at 11:05:09AM +0100, Steven Hartland wrote: > > ----- Original Message ----- > From: "Mark Johnston" > > > > 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 #*define* > >> DTRACE_PROBE1 (name > >> , > >> type0 , > >> arg0 ) \334 > >> DTRACE_PROBE_IMPL_START > >> (name > >> , arg0 > >> , 0, > >> 0, 0, 0) \335 > >> SDT_PROBE_ARGTYPE > >> (sdt > >> , , > >> , name , > >> 0, #type0 , > >> NULL ); \336 > >> DTRACE_PROBE_IMPL_END > >> > >> > >> > >> 324 #*define* DTRACE_PROBE_IMPL_START > >> (name > >> , arg0 > >> , arg1 > >> , arg2 > >> , arg3 > >> , arg4 > >> ) *do* > >> { \325 *static* > >> SDT_PROBE_DEFINE > >> (sdt > >> , , > >> , name ); > >> \326 SDT_PROBE > >> (sdt > >> , , > >> , name , > >> arg0 , > >> arg1 , > >> arg2 , > >> arg3 , > >> arg4 );327 > >> #*define* > >> DTRACE_PROBE_IMPL_END > >> } > >> *while* (0) > >> > >> > >> 163 #*define* > >> SDT_PROBE (prov > >> , > >> mod , > >> func , > >> name , > >> arg0 , > >> arg1 , > >> arg2 , > >> arg3 , > >> arg4 ) *do* > >> { \164 *if* > >> (sdt_ ##prov > >> ##_##mod > >> ##_##func > >> ##_##name > >> ->id > >> ) \165 > >> (*sdt_probe_func > >> )(sdt_ > >> ##prov > >> ##_##mod > >> ##_##func > >> ##_##name > >> ->id > >> , \166 > >> > >> (uintptr_t ) > >> arg0 , > >> (uintptr_t ) > >> arg1 , > >> (uintptr_t ) > >> arg2 , \167 > >> > >> (uintptr_t ) > >> arg3 , > >> (uintptr_t ) > >> arg4 ); \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 > > -#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 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