style(9) rules for nested includes
mdf at FreeBSD.org
mdf at FreeBSD.org
Thu Mar 10 20:45:05 UTC 2011
On Thu, Mar 10, 2011 at 12:28 PM, John Baldwin <jhb at freebsd.org> wrote:
> On Thursday, March 10, 2011 3:10:58 pm mdf at freebsd.org wrote:
>> On Thu, Mar 10, 2011 at 11:46 AM, John Baldwin <jhb at freebsd.org> wrote:
>> > On Thursday, March 10, 2011 12:17:28 pm mdf at 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. 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 <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. 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.
More information about the freebsd-arch