Date: Fri, 23 Mar 2018 15:38:08 +1100 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Jeff Roberson <jroberson@jroberson.net> Cc: Cy Schubert <Cy.Schubert@cschubert.com>, Justin Hibbits <jrh29@alumni.cwru.edu>, Jeff Roberson <jeff@freebsd.org>, src-committers <src-committers@freebsd.org>, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r331369 - head/sys/vm Message-ID: <20180323150709.H968@besplex.bde.org> In-Reply-To: <alpine.BSF.2.21.1803221518200.2307@desktop> References: <201803230059.w2N0x2fw077291@slippy.cwsent.com> <alpine.BSF.2.21.1803221518200.2307@desktop>
next in thread | previous in thread | raw e-mail | index | archive | help
On Thu, 22 Mar 2018, Jeff Roberson wrote: > On Thu, 22 Mar 2018, Cy Schubert wrote: > >> It broke i386 too. > > I just did > TARGET_ARCH=i386 make buildworld > TARGET_ARCH=i386 make buildkernel > > This worked for me? >> >> Index: sys/vm/vm_reserv.c >> =================================================================== >> --- sys/vm/vm_reserv.c (revision 331399) >> +++ sys/vm/vm_reserv.c (working copy) >> @@ -45,8 +45,6 @@ >> >> #include <sys/param.h> >> #include <sys/kernel.h> >> -#include <sys/counter.h> >> -#include <sys/ktr.h> >> #include <sys/lock.h> >> #include <sys/malloc.h> >> #include <sys/mutex.h> >> @@ -55,6 +53,8 @@ >> #include <sys/sbuf.h> >> #include <sys/sysctl.h> >> #include <sys/systm.h> >> +#include <sys/counter.h> >> +#include <sys/ktr.h> >> #include <sys/vmmeter.h> >> #include <sys/smp.h> >> >> This is because sys/i386/include/machine.h uses critical_enter() and >> critical_exit() which are defined in sys/systm.h. Wrong fix. I see you committed this. Now there are more bugs to fix. <sys/systm.h> is a prerequisite for all kernel headers except <sys/param.h>, since it defines and declares things like KASSERT() and critical_enter() which might be used in other headers (except sys/param.h and its standard pollution). Sometimes sys/systm.h is included as undocumented namespace pollution in headers that are accidentally included before the (other) ones that use KASSERT(), etc. The headers that have this bug have it to work around bugs in .c files like the one above. It is more usual to have this bug by not including sys/systm.h at all than to have it by including it in a wrong order. Sorting it alphabetically almost always gives a wrong order. It must be included after sys/param.h and that must be included first. It is a related bug to include only sys/types.h and not sys/param.h. This requires chumminess with the current implementation and all future implementations. sys/param.h provides certain undocumented but standard namespace pollution which might vary with the implementation, as necessary to satisfy some of the pollution requirements of all current and future implementations of other headers. (The pollution should be monotonically decreasing but it was only that for a few years about 20 years ago when I worked on fixing it.) .c files that include sys/types.h instead of sys/param.h have do some subset of the includes in sys/param.h. Since nothing is documented and the subset might depend on the arch and user options, it is hard to know the minimal subset. .c files that include sys/types.h tend to have lots of other #include bugs like not including sys/systm.h. Again it is hard to know the minimal replacement for sys/systm.h and its undocumented but standard pollution. It is a style bug to include both sys/types.h and sys/param.h. style(9) even explicitly forbids including both. It is a larger style bug to include the standard pollution in sys/systm.h direction. This includes especially <machine/atomic.h> and <machine/cpufunc.h>. These should be considered as being implemented in sys/systm.h, with the <machine> headers for them only and implementation detail. Similarly for <sys/libkern.h>. >> It built nicely on my amd64's though. amd64 apparently has more namespace pollution which breaks detection of the bug. But I couldn't find where it is. sys/systm.h isn't included nested in any amd64 or x86 headers. Apparently some amd64 option gives it. Bruce
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20180323150709.H968>