Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 10 Mar 2011 16:25:42 -0500
From:      John Baldwin <jhb@freebsd.org>
To:        mdf@freebsd.org
Cc:        freebsd-arch@freebsd.org
Subject:   Re: style(9) rules for nested includes
Message-ID:  <201103101625.42883.jhb@freebsd.org>
In-Reply-To: <AANLkTinQ13-PDnU04sSNUO5LmMMTxRUe6%2Bmjd3=h6Jy0@mail.gmail.com>
References:  <AANLkTikqBJON46-EJFPPktT82L8dgX6dwwDrxWwFqumU@mail.gmail.com> <201103101528.18987.jhb@freebsd.org> <AANLkTinQ13-PDnU04sSNUO5LmMMTxRUe6%2Bmjd3=h6Jy0@mail.gmail.com>

next in thread | previous in thread | raw e-mail | index | archive | help
On Thursday, March 10, 2011 3:45:03 pm mdf@freebsd.org wrote:
> 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.  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.

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



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201103101625.42883.jhb>