From owner-freebsd-hackers@FreeBSD.ORG Tue Sep 16 03:13:22 2014 Return-Path: Delivered-To: hackers@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id DB6D08B0; Tue, 16 Sep 2014 03:13:22 +0000 (UTC) Received: from mail-pd0-x22a.google.com (mail-pd0-x22a.google.com [IPv6:2607:f8b0:400e:c02::22a]) (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 A00D08BF; Tue, 16 Sep 2014 03:13:22 +0000 (UTC) Received: by mail-pd0-f170.google.com with SMTP id fp1so7708247pdb.1 for ; Mon, 15 Sep 2014 20:13:22 -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=QMMYsCFVjzcAGS1Fm8gSJdKEH2u7fklKQcul7IXUo2Y=; b=fZ/ElARs9m3gPErSxdzyIGTs1Krv0dTE/+bAgv4zZ5GLnQxK7l7o32EFYUY1AJZQyG Mz4GJBC3k+ugUFTlSdgvn9E2LZ3NN2YjCwk91XlphC3wkNmURbGsfCu38KuOxf+shlAf XzraPHtvtzi87Hl+eQ62k2WyLasF2rScW6mC4YoBqlUQBY2sZxivavRf+Ycy8xIxNVpo gyjPTz4gVjq1mw3bPdwE/H4EIZ7Xy9+M18KxpT1/32sbH3erj7nsaU90pEw6j4jvqYZa +hmb3ACrKCK4OPBi5iv2vmtguhKXifKAWwp4BRIQ44hCtPRb8DwFQC4fH7FZi3xnUCy2 cjcQ== X-Received: by 10.66.182.227 with SMTP id eh3mr45375966pac.68.1410837202191; Mon, 15 Sep 2014 20:13:22 -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 wc5sm13182102pab.2.2014.09.15.20.13.21 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 15 Sep 2014 20:13:21 -0700 (PDT) Sender: Mark Johnston Date: Mon, 15 Sep 2014 20:13:19 -0700 From: Mark Johnston To: Matthew Ahrens Subject: Re: ZFS SET_ERROR dtrace probe possible under FreeBSD? Message-ID: <20140916031318.GB26720@charmander.picturesperfect.net> References: 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: Steven Hartland , 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 03:13:23 -0000 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. -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 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