Date: Thu, 10 Mar 2011 12:45:03 -0800 From: mdf@FreeBSD.org To: John Baldwin <jhb@freebsd.org> Cc: freebsd-arch@freebsd.org Subject: Re: style(9) rules for nested includes Message-ID: <AANLkTinQ13-PDnU04sSNUO5LmMMTxRUe6%2Bmjd3=h6Jy0@mail.gmail.com> In-Reply-To: <201103101528.18987.jhb@freebsd.org> References: <AANLkTikqBJON46-EJFPPktT82L8dgX6dwwDrxWwFqumU@mail.gmail.com> <201103101446.37589.jhb@freebsd.org> <AANLkTikbRaCE628wJvKbBUbuBsJo6d6wJhvozSZA8kWW@mail.gmail.com> <201103101528.18987.jhb@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Thu, Mar 10, 2011 at 12:28 PM, John Baldwin <jhb@freebsd.org> 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 <jhb@freebsd.org> 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 <sys/linker_set.h> and <sys/queue.h> being a few of the only ones >> >> allowed. =A0However, I don't see anything in style(9) about this. >> > >> > bde@ is probably the most authoritative. =A0My understanding is that t= he only >> > nested includes allowed in sys/sys/*.h are the two listed above and an= y header >> > that starts with an underscore (sys/_mutex.h, etc.). =A0The underscore= variants >> > were added to allow nested includes when absolutely necessary, but tho= se >> > includes are the bare minimum required to define structures, etc. >> > >> >> Now we come to the reason I ask. =A0I'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 t= o >> >> either change quite a few places that include <sys/sysctl.h>, or >> >> include <sys/kernel.h> instead of <sys/linker_set.h> 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. =A0We >> saw this happen once at Isilon. > > Hmm, this is a legitimate reason, though I'd be tempted to fix that by ju= st > registering sysctls after sysinit's have been invoked and vice versa on > unload. =A0It 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. Cheers, matthew
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?AANLkTinQ13-PDnU04sSNUO5LmMMTxRUe6%2Bmjd3=h6Jy0>