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/tegra1
Hi Peter, Can you share what your kernel configuration is? I believe the error is masked in existing tinderbox kernel configurations ( https://ci.freebsd.org/tinderbox/ ) by some remaining header pollution that must be conditional on an option. The files you've pointed out are missing the eventhandler header (and fixing that is trivial), but I'd like to better understand where the leak is so that other misses can be located. Thanks, Conrad On Mon, May 20, 2019 at 12:31 PM Conrad Meyer wrote: > > Hi Peter, > > Thanks for reporting this. I ran a full tinderbox on this change > (many many tinderboxes) and am not sure how I missed this. I have to > run to an appointment now, but will fix these in a couple hours if no > one else beats me to it. The fix is straightforward — include > . > > Best, > Conrad > > On Mon, May 20, 2019 at 12:21 PM Peter Jeremy wrote: > > > > On 2019-May-20 00:38:23 +, Conrad Meyer wrote: > > >Author: cem > > >Date: Mon May 20 00:38:23 2019 > > >New Revision: 347984 > > >URL: https://svnweb.freebsd.org/changeset/base/347984 > > > > > >Log: > > > Extract eventfilter declarations to sys/_eventfilter.h > > ... > > > No functional change (intended). Of course, any out of tree modules that > > > relied on header pollution for sys/eventhandler.h, sys/lock.h, or > > > sys/mutex.h inclusion need to be fixed. __FreeBSD_version has been > > > bumped. > > > > This seems to have broken at least netmap and netdump for me: > > /usr/src/sys/dev/netmap/netmap_freebsd.c:191:3: error: implicit declaration > > of function 'EVENTHANDLER_REGISTER' is invalid in C99 > > [-Werror,-Wimplicit-function-declaration] > > EVENTHANDLER_REGISTER(ifnet_arrival_event, > > ^ > > ... > > /usr/src/sys/netinet/netdump/netdump_client.c:1458:22: error: implicit > > declaration of function 'EVENTHANDLER_REGISTER' is invalid in C99 > > [-Werror,-Wimplicit-function-declaration] > > nd_detach_cookie = > > EVENTHANDLER_REGISTER(ifnet_departure_event, > >^ > > > > -- > > Peter Jeremy ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
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/tegra1
Hi Peter, Thanks for reporting this. I ran a full tinderbox on this change (many many tinderboxes) and am not sure how I missed this. I have to run to an appointment now, but will fix these in a couple hours if no one else beats me to it. The fix is straightforward — include . Best, Conrad On Mon, May 20, 2019 at 12:21 PM Peter Jeremy wrote: > > On 2019-May-20 00:38:23 +, Conrad Meyer wrote: > >Author: cem > >Date: Mon May 20 00:38:23 2019 > >New Revision: 347984 > >URL: https://svnweb.freebsd.org/changeset/base/347984 > > > >Log: > > Extract eventfilter declarations to sys/_eventfilter.h > ... > > No functional change (intended). Of course, any out of tree modules that > > relied on header pollution for sys/eventhandler.h, sys/lock.h, or > > sys/mutex.h inclusion need to be fixed. __FreeBSD_version has been bumped. > > This seems to have broken at least netmap and netdump for me: > /usr/src/sys/dev/netmap/netmap_freebsd.c:191:3: error: implicit declaration > of function 'EVENTHANDLER_REGISTER' is invalid in C99 > [-Werror,-Wimplicit-function-declaration] > EVENTHANDLER_REGISTER(ifnet_arrival_event, > ^ > ... > /usr/src/sys/netinet/netdump/netdump_client.c:1458:22: error: implicit > declaration of function 'EVENTHANDLER_REGISTER' is invalid in C99 > [-Werror,-Wimplicit-function-declaration] > nd_detach_cookie = > EVENTHANDLER_REGISTER(ifnet_departure_event, >^ > > -- > Peter Jeremy ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
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/tegra1
On 2019-May-20 00:38:23 +, Conrad Meyer wrote: >Author: cem >Date: Mon May 20 00:38:23 2019 >New Revision: 347984 >URL: https://svnweb.freebsd.org/changeset/base/347984 > >Log: > Extract eventfilter declarations to sys/_eventfilter.h ... > No functional change (intended). Of course, any out of tree modules that > relied on header pollution for sys/eventhandler.h, sys/lock.h, or > sys/mutex.h inclusion need to be fixed. __FreeBSD_version has been bumped. This seems to have broken at least netmap and netdump for me: /usr/src/sys/dev/netmap/netmap_freebsd.c:191:3: error: implicit declaration of function 'EVENTHANDLER_REGISTER' is invalid in C99 [-Werror,-Wimplicit-function-declaration] EVENTHANDLER_REGISTER(ifnet_arrival_event, ^ ... /usr/src/sys/netinet/netdump/netdump_client.c:1458:22: error: implicit declaration of function 'EVENTHANDLER_REGISTER' is invalid in C99 [-Werror,-Wimplicit-function-declaration] nd_detach_cookie = EVENTHANDLER_REGISTER(ifnet_departure_event, ^ -- Peter Jeremy signature.asc Description: PGP signature
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/tegra1
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
-#include
-#include
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
+#include
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
#include
#include
#include
+#include
#include
#include
#include
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
#include
#include
+#include
#include
+#include
#include
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
#include
#include
+#include
#include
+#include
#include
#include
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 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
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
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/tegra1
> On 20. May 2019, at 02:38, Conrad Meyer wrote:
>
> Author: cem
> Date: Mon May 20 00:38:23 2019
> New Revision: 347984
> URL: https://svnweb.freebsd.org/changeset/base/347984
>
> Log:
> Extract eventfilter declarations to sys/_eventfilter.h
>
> 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).
>
> As a side effect of reduced header pollution, many .c files and headers no
> longer contain needed definitions. The remainder of the patch addresses
> adding appropriate includes to fix those files.
>
> LOCK_DEBUG and LOCK_FILE_LINE_ARG are moved to sys/_lock.h, as required by
> sys/mutex.h since r326106 (but silently protected by header pollution prior
> to this change).
>
> No functional change (intended). Of course, any out of tree modules that
> relied on header pollution for sys/eventhandler.h, sys/lock.h, or
> sys/mutex.h inclusion need to be fixed. __FreeBSD_version has been bumped.
This breaks compiling kernels having the options
options KCOV# Kernel Coverage Sanitizer
options COVERAGE
enabled.
sys/kern/kern_kcov.c also needs to include sys/eventhandler.h
Best regards
Michael
>
> Added:
> head/sys/sys/_eventhandler.h (contents, props changed)
> Modified:
> head/sys/amd64/vmm/io/iommu.c
> head/sys/arm/allwinner/a10/a10_intc.c
> head/sys/arm/allwinner/a10_dmac.c
> head/sys/arm/allwinner/a31_dmac.c
> head/sys/arm/allwinner/aw_ccu.c
> head/sys/arm/allwinner/aw_reset.c
> head/sys/arm/allwinner/aw_rsb.c
> head/sys/arm/allwinner/aw_spi.c
> head/sys/arm/allwinner/aw_thermal.c
> head/sys/arm/allwinner/aw_wdog.c
> head/sys/arm/allwinner/clkng/aw_ccung.c
> head/sys/arm/arm/machdep.c
> head/sys/arm/arm/pl190.c
> head/sys/arm/broadcom/bcm2835/bcm2835_rng.c
> head/sys/arm/broadcom/bcm2835/bcm2835_wdog.c
> head/sys/arm/broadcom/bcm2835/bcm2836.c
> head/sys/arm/freescale/imx/imx_wdog.c
> head/sys/arm/mv/armada/thermal.c
> head/sys/arm/mv/armada/wdt.c
> head/sys/arm/mv/mv_spi.c
> head/sys/arm/mv/timer.c
> head/sys/arm/nvidia/tegra124/tegra124_machdep.c
> head/sys/arm/nvidia/tegra124/tegra124_pmc.c
> head/sys/arm/nvidia/tegra_xhci.c
> head/sys/arm/ti/ti_pruss.c
> head/sys/arm/ti/ti_wdt.c
> head/sys/arm/versatile/versatile_pci.c
> head/sys/arm/versatile/versatile_sic.c
> head/sys/arm64/arm64/gicv3_its.c
> head/sys/arm64/arm64/machdep.c
> head/sys/arm64/coresight/coresight.c
> head/sys/arm64/rockchip/clk/rk_cru.c
> head/sys/cam/cam_periph.h
> head/sys/cam/ctl/ctl_ha.c
> head/sys/cddl/compat/opensolaris/kern/opensolaris.c
> head/sys/contrib/ipfilter/netinet/ip_fil_freebsd.c
> head/sys/contrib/vchiq/interface/vchiq_arm/vchiq_arm.c
> head/sys/crypto/aesni/aesni.c
> head/sys/crypto/armv8/armv8_crypto.c
> head/sys/crypto/blake2/blake2_cryptodev.c
> head/sys/crypto/ccp/ccp.c
> head/sys/crypto/ccp/ccp_hardware.c
> head/sys/ddb/db_command.c
> head/sys/dev/acpi_support/acpi_panasonic.c
> head/sys/dev/acpica/acpi.c
> head/sys/dev/acpica/acpi_lid.c
> head/sys/dev/acpica/acpi_thermal.c
> head/sys/dev/acpica/acpi_video.c
> head/sys/dev/acpica/acpivar.h
> head/sys/dev/adb/adb_kbd.c
> head/sys/dev/adb/adb_mouse.c
> head/sys/dev/amdsbwd/amdsbwd.c
> head/sys/dev/atkbdc/psm.c
> head/sys/dev/cardbus/cardbus.c
> head/sys/dev/cmx/cmx.c
> head/sys/dev/coretemp/coretemp.c
> head/sys/dev/cxgbe/cxgbei/cxgbei.c
> head/sys/dev/cxgbe/cxgbei/icl_cxgbei.c
> head/sys/dev/cxgbe/tom/t4_tls.c
> head/sys/dev/dcons/dcons_crom.c
> head/sys/dev/dcons/dcons_os.c
> head/sys/dev/dcons/dcons_os.h
> head/sys/dev/evdev/evdev_private.h
> head/sys/dev/extres/syscon/syscon_generic.c
> head/sys/dev/firewire/firewire.c
> head/sys/dev/firewire/fwohci.c
> head/sys/dev/ichwd/ichwd.c
> head/sys/dev/ida/ida_disk.c
> head/sys/dev/ida/ida_pci.c
> head/sys/dev/iir/iir_ctrl.c
> head/sys/dev/ioat/ioat.c
> head/sys/dev/ipmi/ipmi.c
> head/sys/dev/ipmi/ipmi_opal.c
> head/sys/dev/ips/ips.c
> head/sys/dev/iscsi/icl_soft_proxy.c
> head/sys/dev/iscsi_initiator/iscsivar.h
> head/sys/dev/iwm/if_iwm_notif_wait.c
> head/sys/dev/led/led.c
> head/sys/dev/liquidio/lio_bsd.h
> head/sys/dev/mfi/mfi_disk.c
> head/sys/dev/mfi/mfi_pci.c
> head/sys/dev/mfi/mfi_syspd.c
> head/sys/dev/mlx/mlxvar.h
> head/sys/dev/mmc/host/dwmmc.c
> head/sys/dev/mpr/mprvar.h
> head/sys/dev/mps/mpsvar.h
> head/sys/dev/mrsas/mrsas.h
> head/sys/dev/nmdm/nmdm.c
> head/sys/dev/ntb/if_ntb/if_ntb.c
> head/sys/dev/ntb/ntb_hw/ntb_hw_intel.c
> head/sys/dev/ow/ow.c
> head/sys/dev/pccard/pccard.c
> head/sys/dev/pci/pci.c
> head/sys/dev/pci/pci_iov.c
> head/sys/dev/pci/pci_pci.c
> head/sys/dev/pci/pcivar.h
> head/sys/dev/scc/scc_core.c
> head/sys/dev/scc/scc_dev_quicc.c
> head/sys/dev/scc/scc_dev_sab8
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/tegra1
On Mon, May 20, 2019 at 2:38 AM Conrad Meyer wrote:
> Author: cem
> Date: Mon May 20 00:38:23 2019
> New Revision: 347984
> URL: https://svnweb.freebsd.org/changeset/base/347984
>
> Log:
> Extract eventfilter declarations to sys/_eventfilter.h
>
> 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).
>
> As a side effect of reduced header pollution, many .c files and headers no
> longer contain needed definitions. The remainder of the patch addresses
> adding appropriate includes to fix those files.
>
> LOCK_DEBUG and LOCK_FILE_LINE_ARG are moved to sys/_lock.h, as required by
> sys/mutex.h since r326106 (but silently protected by header pollution prior
> to this change).
>
> No functional change (intended). Of course, any out of tree modules that
> relied on header pollution for sys/eventhandler.h, sys/lock.h, or
> sys/mutex.h inclusion need to be fixed. __FreeBSD_version has been bumped.
Hi,
Why request an exp-run and commit the patch before the exp-run has finished?
Cheers,
Antoine
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
