From owner-svn-src-head@FreeBSD.ORG Mon May 11 02:48:02 2015 Return-Path: Delivered-To: svn-src-head@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id D63446D2; Mon, 11 May 2015 02:48:01 +0000 (UTC) Received: from mail-pd0-x22a.google.com (mail-pd0-x22a.google.com [IPv6:2607:f8b0:400e:c02::22a]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id B00F91C21; Mon, 11 May 2015 02:48:01 +0000 (UTC) Received: by pdea3 with SMTP id a3so136539916pde.3; Sun, 10 May 2015 19:48:01 -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=lMEEclmenH9JiqB5ISHCsATiLwrrSpWZW1Ejp5xIKOc=; b=mtA9bt62HpbnOcn0VizoG7pdMlHlTFZ7UmZX3ngxNR6IHpAmPmJWfe7ZOv+xv9HOCp h+gUTQV2a/9xRL8YWB/HmeYZxfe9sVzhvwnc1FBuVfgxnUKWhTGA90yk7eiHfAES0RnH HbGwLXtQFXjligVxf6WzDRyCC6ugwr8jysmZxx2M2DGTrM7f+9ne4LOckJwQn/PHdG55 KHAjc+WNhcz2n9WjPJJdhb0leAGrTyUjQOSwVxXV1X4wnPWOX0wB51yLk00+S2e/p6Mx uUCGo5cXwVOQ60B5fA0SQchvEZ0QSHuKy4gfZB9yakc6eR7BoxY9UgLG1mdRqyF++3zj VEDA== X-Received: by 10.66.221.34 with SMTP id qb2mr15504711pac.42.1431312481156; Sun, 10 May 2015 19:48:01 -0700 (PDT) Received: from raichu ([104.232.114.184]) by mx.google.com with ESMTPSA id sl9sm11413297pac.41.2015.05.10.19.47.59 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Sun, 10 May 2015 19:47:59 -0700 (PDT) Sender: Mark Johnston Date: Sun, 10 May 2015 19:47:55 -0700 From: Mark Johnston To: Julian Elischer Cc: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r282744 - in head/sys/cddl/dev/dtrace: amd64 i386 Message-ID: <20150511024754.GA65659@raichu> References: <201505102227.t4AMRn1g007294@svn.freebsd.org> <55500C0D.7090802@freebsd.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <55500C0D.7090802@freebsd.org> User-Agent: Mutt/1.5.23 (2014-03-12) X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 11 May 2015 02:48:02 -0000 On Mon, May 11, 2015 at 09:55:25AM +0800, Julian Elischer wrote: > On 5/11/15 6:27 AM, Mark Johnston wrote: > > Author: markj > > Date: Sun May 10 22:27:48 2015 > > New Revision: 282744 > > URL: https://svnweb.freebsd.org/changeset/base/282744 > > > > Log: > > Remove some commented-out upstream code for handling traps from usermode > > DTrace probes. This handling is already done in trap() on i386 and amd64. > > I take issue with the implied assertion that upstream code that is > not needed for FreeBSD should be removed. I don't agree with that assertion either. The truly upstream DTrace code, i.e. the DTrace code that is updated by merges from the vendor branch, lives under sys/cddl/contrib/opensolaris. When DTrace was imported, some parts of it had to be modified significantly enough that they were placed outside the vendor import dir, in sys/cddl/dev/. In particular, this code isn't modified by MFVs and is maintained solely within FreeBSD. I try quite hard to keep the upstream code around for changes under the former directory, but I don't see the value in keeping upstream code under the latter. In many cases it is substantially different from what we actually use and just ends up being confusing. It also becomes inconsistent as different arches gain DTrace support and add varying amounts of upstream code to their MD DTrace files. See r281916 for an example where the (dead) upstream code was kept for years and copied around before it was realized that it wasn't needed. This split isn't ideal, but upstream DTrace development has been inactive enough that it doesn't end up being too painful an issue, IMHO. > > Commented out upstream code makes it easier to merge in changes from > upstream. > it allows patches to be applied and for a lot less worry on the part > of the merger. > If the code is not there, and there is an upstream change to it, one > has to ask > "is there equivalent code somewhere else that needs to be patched > instead?" > But if there is just a > > #ifndef __FreeBSD__ /* this code is not needed in FreeBSD because ...*/ > blah > #endif /*__FreeBSD__ this code is not needed in FreeBSD */ > > then the merger has a much easier task. I've caught several cases (specifically in merging DTrace code) where the upstream code block is so large the merger didn't even notice the #ifndef __FreeBSD__ was present and thought that the merge was fine because the upstream diff applied cleanly. > > I've just gone through large code merge (where FreeBSD was "upstream") > and the cases > there the original FreebSD code was still present but #Ifdef'd out > were SO MUCH easier > to handle.