Date: Mon, 20 May 2019 21:05:05 +1000 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Conrad Meyer <cem@freebsd.org> Cc: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r347984 - in head/sys: amd64/vmm/io arm/allwinner arm/allwinner/a10 arm/allwinner/clkng arm/arm arm/broadcom/bcm2835 arm/freescale/imx arm/mv arm/mv/armada arm/nvidia arm/nvidia/tegra12... Message-ID: <20190520202101.L1014@besplex.bde.org> In-Reply-To: <201905200038.x4K0cNoZ019920@repo.freebsd.org> References: <201905200038.x4K0cNoZ019920@repo.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Mon, 20 May 2019, Conrad Meyer wrote:
> Log:
> Extract eventfilter declarations to sys/_eventfilter.h
sys/_eventhandler.h has 2 identical 72-line copies of the correct version.
The second copy of the reinclusion guard works as a guard against the
doubling too.
>
> This allows replacing "sys/eventfilter.h" includes with "sys/_eventfilter.h"
> in other header files (e.g., sys/{bus,conf,cpu}.h) and reduces header
> pollution substantially.
>
> EVENTHANDLER_DECLARE and EVENTHANDLER_LIST_DECLAREs were moved out of .c
> files into appropriate headers (e.g., sys/proc.h, powernv/opal.h).
Thanks, but this gives even more pollution. Almost everything includes
sys/proc.h, so its already large pollution affects almost everything.
The previous pollution in it didn't include sys/eventhandler.h, except
possibly via a nested include.
> Modified: head/sys/amd64/vmm/io/iommu.c
> ==============================================================================
> --- head/sys/amd64/vmm/io/iommu.c Sun May 19 23:56:04 2019 (r347983)
> +++ head/sys/amd64/vmm/io/iommu.c Mon May 20 00:38:23 2019 (r347984)
> @@ -32,10 +32,10 @@
> __FBSDID("$FreeBSD$");
>
> #include <sys/param.h>
> -#include <sys/types.h>
> -#include <sys/systm.h>
Error. sys/systm.h must be included after sys/param.h, since it contains
macros like KASSERT() which may be used in inline functions in any kernel
header except sys/param.h and possibly sys/types.h.
> Modified: head/sys/arm/allwinner/a10/a10_intc.c
> ==============================================================================
> --- head/sys/arm/allwinner/a10/a10_intc.c Sun May 19 23:56:04 2019 (r347983)
> +++ head/sys/arm/allwinner/a10/a10_intc.c Mon May 20 00:38:23 2019 (r347984)
> @@ -30,11 +30,12 @@ __FBSDID("$FreeBSD$");
>
> #include "opt_platform.h"
>
> -#include <sys/types.h>
> +#include <sys/param.h>
Including sys/types.h and not including sys/param.h was another error.
sys/param.h supplies lots of standard pollution that may be depended
on by any other kernel header. It is impractical to even test if
sys/types.h works with all combinations of ifdefs.
> #include <sys/bus.h>
> #include <sys/cpuset.h>
> #include <sys/kernel.h>
> #include <sys/ktr.h>
> +#include <sys/lock.h>
> #include <sys/module.h>
> #include <sys/mutex.h>
> #include <sys/param.h>
sys/param.h is now included twice. It is another header that must be
included in non-alphabetical order. The second include may have
compensated for not including it first.
>
> Modified: head/sys/arm/allwinner/a10_dmac.c
> ==============================================================================
> --- head/sys/arm/allwinner/a10_dmac.c Sun May 19 23:56:04 2019 (r347983)
> +++ head/sys/arm/allwinner/a10_dmac.c Mon May 20 00:38:23 2019 (r347984)
> @@ -38,7 +38,9 @@ __FBSDID("$FreeBSD$");
> #include <sys/rman.h>
> #include <sys/condvar.h>
> #include <sys/kernel.h>
> +#include <sys/lock.h>
> #include <sys/module.h>
> +#include <sys/mutex.h>
>
> #include <machine/bus.h>
>
> Modified: head/sys/arm/allwinner/a31_dmac.c
> ==============================================================================
> --- head/sys/arm/allwinner/a31_dmac.c Sun May 19 23:56:04 2019 (r347983)
> +++ head/sys/arm/allwinner/a31_dmac.c Mon May 20 00:38:23 2019 (r347984)
> @@ -38,7 +38,9 @@ __FBSDID("$FreeBSD$");
> #include <sys/rman.h>
> #include <sys/condvar.h>
> #include <sys/kernel.h>
> +#include <sys/lock.h>
> #include <sys/module.h>
> +#include <sys/mutex.h>
> #include <sys/endian.h>
>
> #include <machine/bus.h>
I gave up trying to fix missing includes of sys/mutex.h and its
prerequisite sys/lock.h 15+ years ago before disk code added lots more
of them. sys/buf.h was polluted by adding an include of sys/mutex.h,
and most disk drivers dependended on that. The only other includes
of sys/mutex.h in a <sys> header were in sys/eventhandler.h and
sys/vnode.h, and the latter was fixed in my version. This pollution
has spread to sys/buf_ring.h, sys/fail.h, sys/jail.h, sys/mman.h,
sys/msgbuf.h, sys/rmlock.h, sys/timeet.h and sys/tty.h.
The smaller headers here are trivial to fix, and even sys/vnode.h
wasn't hard to fix 15 years ago before the pollution spread. sys/mman.h
seems to have only 1 mutex declaration, of a struct mtx, so using the
correct header sys/_mutex.h works. Only inlines using mutexes cause
pollution that is hard to avoid. sys/vnode.h still has only 1 inline
function.
Bruce
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20190520202101.L1014>