From owner-freebsd-arch@FreeBSD.ORG Fri Mar 11 12:36:02 2011 Return-Path: Delivered-To: freebsd-arch@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 85A94106566B; Fri, 11 Mar 2011 12:36:02 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from cyrus.watson.org (cyrus.watson.org [65.122.17.42]) by mx1.freebsd.org (Postfix) with ESMTP id 426398FC15; Fri, 11 Mar 2011 12:36:02 +0000 (UTC) Received: from bigwig.baldwin.cx (66.111.2.69.static.nyinternet.net [66.111.2.69]) by cyrus.watson.org (Postfix) with ESMTPSA id BCBE746B03; Fri, 11 Mar 2011 07:36:01 -0500 (EST) Received: from jhbbsd.localnet (unknown [209.249.190.10]) by bigwig.baldwin.cx (Postfix) with ESMTPSA id 5A1608A01B; Fri, 11 Mar 2011 07:36:01 -0500 (EST) From: John Baldwin To: mdf@freebsd.org Date: Thu, 10 Mar 2011 16:25:42 -0500 User-Agent: KMail/1.13.5 (FreeBSD/7.4-CBSD-20110107; KDE/4.4.5; amd64; ; ) References: <201103101528.18987.jhb@freebsd.org> In-Reply-To: MIME-Version: 1.0 Content-Type: Text/Plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Message-Id: <201103101625.42883.jhb@freebsd.org> X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.2.6 (bigwig.baldwin.cx); Fri, 11 Mar 2011 07:36:01 -0500 (EST) Cc: freebsd-arch@freebsd.org Subject: Re: style(9) rules for nested includes X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 11 Mar 2011 12:36:02 -0000 On Thursday, March 10, 2011 3:45:03 pm mdf@freebsd.org wrote: > On Thu, Mar 10, 2011 at 12:28 PM, John Baldwin wrote: > > On Thursday, March 10, 2011 3:10:58 pm mdf@freebsd.org wrote: > >> On Thu, Mar 10, 2011 at 11:46 AM, John Baldwin wrote: > >> > On Thursday, March 10, 2011 12:17:28 pm mdf@freebsd.org wrote: > >> >> I recall a recent discussion/PR about nested includes in the context > >> >> of and being a few of the only ones > >> >> allowed. However, I don't see anything in style(9) about this. > >> > > >> > bde@ is probably the most authoritative. My understanding is that the only > >> > nested includes allowed in sys/sys/*.h are the two listed above and any header > >> > that starts with an underscore (sys/_mutex.h, etc.). The underscore variants > >> > were added to allow nested includes when absolutely necessary, but those > >> > includes are the bare minimum required to define structures, etc. > >> > > >> >> Now we come to the reason I ask. I'm working on a patch to change the > >> >> static sysctl code to use the standard SYSININT/SYSUNINIT code rather > >> >> than have special treatment in kern_linker.c, but to do this I need to > >> >> either change quite a few places that include , or > >> >> include instead of in sysctl.h, as > >> >> the SI_SUB_SYSCTLS value isn't visible otherwise. > >> > > >> > Hmm, what is the reason to use SYSINIT's instead of a dedicated linker set? > >> > >> There's also a minor bug in initialization ordering where a static > >> SYSCTL_PROC could use a lock initialized by SX_SYSINIT or MTX_SYSINIT, > >> but at runtime module load the sysctl is exposed before the > >> SI_SUB_LOCK stage has run, so in theory someone doing sysctl -a would > >> crash the kernel on an attempt to lock an uninitialized mtx/sx. We > >> saw this happen once at Isilon. > > > > Hmm, this is a legitimate reason, though I'd be tempted to fix that by just > > registering sysctls after sysinit's have been invoked and vice versa on > > unload. It seems that would be a simpler fix with far less code churn and > > not having to deal with the nested include mess, etc. > > I'm not committed to committing this change; I want to see how it > looks when finished and run it by -arch. But I hit the nested include > file problem first. :-) > > Changing the sysctl to be after all sysinit I *think* runs into a > problem if one has a mix of SYSCTL_ADD_FOO and SYSCTL_FOO on a new > static node defined in a loadable module, but I'd have to test that. > That is, one would be trying to add a node as a child of a node that > hasn't been set up. I think it may work today because the list of > child nodes of a static node is actually a separate entity that exists > at compile time. Yes, that's correct. Hmm, I'd be tempted to say that the modules that want to do what you do should use a single SYSINIT or module event handler to explicitly order all the dependencies and then use SYSCTL_ADD_*() in that routine. -- John Baldwin