svn commit: r313850 - in head/sys/cddl/dev/dtrace: amd64 i386

[Bruce Evans]

On Fri, 17 Feb 2017, Mark Johnston wrote:

> Log:
>  Directly include needed headers rather than relying on pollution.
>
>  We get machine/cpu.h via kmem.h -> proc.h -> _vm_domain.h -> seq.h.

machine/cpu.h is not added here.  Do you mean machine/cpufnc.h?

> Modified: head/sys/cddl/dev/dtrace/amd64/dtrace_subr.c
> ==============================================================================
> --- head/sys/cddl/dev/dtrace/amd64/dtrace_subr.c	Fri Feb 17 00:50:00 2017	(r313849)
> +++ head/sys/cddl/dev/dtrace/amd64/dtrace_subr.c	Fri Feb 17 03:27:20 2017	(r313850)
> @@ -41,7 +41,9 @@
> #include <sys/dtrace_impl.h>
> #include <sys/dtrace_bsd.h>
> #include <machine/clock.h>
> +#include <machine/cpufunc.h>

This was correct.  <machine/cpufunc.h> is standard pollution in <sys/systm.h>,
and it is a style bug to include it directly, and a bug to not include
<sys/systm.h> after <sys/param.h> and before all other headers in kernel
.c files.

It is pollution to include <machine/cpufunc.h> in any header except
<sys/systm.h>.  kmem.h has grosser pollution (param, proc, malloc, vmem,
vm/uma, vm/vm, vm/vm_extern, but not systm which is a prerequisite for
most of the other headers that kmem.h includes).  Most of the other headers
also have gross pollution, ending with seq.h which includes systm.h and
its standard pollution.

seq.h also includes machine/cpu.h.  Apparently kmem.h does depend on this,
and this commit doesn't fix it.

seq.h also has a nonsense include of sys/types.h after sys/systm.h.
sys/types.h is a prerequisite for sys/systm.h, and is apparently usually
suppied for seq.h in the usual way by including sys/param.h before all
other headers in kernel files.

seq.h is also polluted with sys/lock.h.  It apparently only does this
to get MPASS() defined.  MPASS() has nothing to do with the locking
in lock.h, and isn't even used there.  It is just a trivial wrapper
for KASSERT and probably belongs next to the definition of KASSERT()
in systm.h.  This wrapper mostly subtracts value by adding "Assertion...
failed" and __FILE__ and __LINE__ to KASSERT().  That is, it makes the
output format more like user assert().  But KASSERT() intentionally
doesn't use this format, since is too generic.  MPASS() is easier to
use, and I suspect most uses of it are because of this and not because
it is any good.  In kern, there are 1214 KASSERT()s and 346 MPASS(),
with MPASS().

> #include <machine/frame.h>
> +#include <machine/psl.h>

This include is not mentioned in the log message.

machine/psl.h is stamdard pollution in machine/cpu.h (on at least amd64 and
i386), so would not be needed here if machine/cpu.h had been spelled
correctly.

> #include <vm/pmap.h>
>
> extern void dtrace_getnanotime(struct timespec *tsp);

The timestamping seems over-engineered, but it has a chance of working,
unlike KTR timestamps which just used unscaled get_cyclecount() whatever
that is (it is raw TSC on most x86, and KTR doesn't even record its
current frequency to give the user a chance of scaling).

Bruce