Fix build.

[kib]

Summary

Namely, vmm.ko cannot be compiled without 'option SMP', the code uses IPIs and LAPIC.
Recently systrace was forced over any configs, check for KDTRACE_HOOK before compiling the dtrace/systrace* modules.

[mjg]

I was looking at this myself.

In sys/sdt.h you will see all SDT probes are also gated by KDTRACE_HOOKS, meaning dtrace is probably not usable without it (modulo perhaps a small subset) and the entire modules/dtrace dir should be gated by said option in my opinion. Have not tested yet the build yet.

[mjg]

I think we should get another kernel config to build with tinderbox. Could be GENERIC without: SMP, NUMA, KDTRACE_HOOKS and maybe few others. Then not only we cover these, people can be asked to always make sure that kernel builds.

[kib]

If module is buildable, I do not see a reason to not include it into the build. Mark can provide more input on this issue.

For separate kernel config, I think it is fine. We definitely should provide stripped down UP i386, for amd64 I am not sure that UP in amd64/conf is too important but would not object if somebody adds it. In principle, LINT should have exactly the opposite set of options comparing with GENERIC, but I think it is no longer compatible with other idea behind LINT/NOTES, 'compile in as much as possible'.

[se]

This patch resolves the issue I had with a kernel configuration without KDTRACE_HOOK.

[mjg]

I think building dtrace without KDTRACE_HOOKS partially defeats the point of not including it in the kernel config. The module is heavily neutered if not straight up useless, but adds to the installation. On top of that it may be misleading to the end user who might think they can still dtrace like on stock config.

[se]

Whether dtrace is useless without KDTRACE_HOOKS is orthogonal to the issue that the currently committed code breaks "make buildkernel" if the kernel option KDTRACE_HOOKS is not defined.

The issue is not usability of dtrace but the breaking of a fundamental build process, unless further action is taken (by manually excluding the systrace module from the build).

In my particular case the system without KDTRACE_HOOKS is meant to be a production system.
dtrace is irrelevant there, it will never be used on that host, and I do not think this is an uncommon case.

[mjg]

I'm arguing for exclusion of the entire dtrace/ directory which would also take care of systrace.

Basically this (untested):

diff --git a/sys/modules/Makefile b/sys/modules/Makefile
index 6055587fa0c..2e57e31522b 100644
--- a/sys/modules/Makefile
+++ b/sys/modules/Makefile
@@ -394,10 +394,12 @@ _autofs=  autofs
 .endif
 
 .if ${MK_CDDL} != "no" || defined(ALL_MODULES)
+.if ${KERN_OPTS:MKDTRACE_HOOKS}
 .if (${MACHINE_CPUARCH} != "arm" || ${MACHINE_ARCH:Marmv[67]*} != "") && \
        ${MACHINE_CPUARCH} != "mips"
 SUBDIR+=       dtrace
 .endif
+.endif
 SUBDIR+=       opensolaris
 .endif

[markj]

I don't see a problem with excluding the dtrace modules if KDTRACE_HOOKS is not defined, but the patch doesn't completely fix the problem. KDTRACE_HOOKS is a kernel option that means, "the kernel implements the interfaces required by dtrace.ko and friends." It doesn't make sense for the modules themselves to be modulated by this option. Right now a standalone build of systrace.ko will fail because it doesn't define KDTRACE_HOOKS. With this patch it fails also, but for a different reason (SYSDIR is not defined in standalone builds).

I don't see why sysent.h can't have

#ifdef KDTRACE_HOOKS
#define SYSTRACE_ENABLED systrace_enabled
#else
#define SYSTRACE_ENABLED 0
#endif

and use SYSTRACE_ENABLED instead of the systrace_enabled in kernel sources. That would work around the problem.

[kib]

In D23699#520445, @markj wrote:

I don't see a problem with excluding the dtrace modules if KDTRACE_HOOKS is not defined, but the patch doesn't completely fix the problem. KDTRACE_HOOKS is a kernel option that means, "the kernel implements the interfaces required by dtrace.ko and friends." It doesn't make sense for the modules themselves to be modulated by this option.

Why ? This is exactly how options work, at least for the coherent build of kernel and modules.

Right now a standalone build of systrace.ko will fail because it doesn't define KDTRACE_HOOKS. With this patch it fails also, but for a different reason (SYSDIR is not defined in standalone builds).

I will fix that (SYSDIR part).

I don't see why sysent.h can't have

#ifdef KDTRACE_HOOKS
#define SYSTRACE_ENABLED systrace_enabled
#else
#define SYSTRACE_ENABLED 0
#endif

and use SYSTRACE_ENABLED instead of the systrace_enabled in kernel sources. That would work around the problem.

Perhaps this is fine, but it does not fix bhyve module after SMP-force was removed.

[markj]

In D23699#520446, @kib wrote:

In D23699#520445, @markj wrote:

I don't see a problem with excluding the dtrace modules if KDTRACE_HOOKS is not defined, but the patch doesn't completely fix the problem. KDTRACE_HOOKS is a kernel option that means, "the kernel implements the interfaces required by dtrace.ko and friends." It doesn't make sense for the modules themselves to be modulated by this option.

Why ? This is exactly how options work, at least for the coherent build of kernel and modules.

It is fine in general, but IMO it doesn't really make sense for this particular option. KDTRACE_HOOKS controls the KBI visible to modules, not the modules themselves.

Right now a standalone build of systrace.ko will fail because it doesn't define KDTRACE_HOOKS. With this patch it fails also, but for a different reason (SYSDIR is not defined in standalone builds).

I will fix that (SYSDIR part).

I don't see why sysent.h can't have

#ifdef KDTRACE_HOOKS
#define SYSTRACE_ENABLED systrace_enabled
#else
#define SYSTRACE_ENABLED 0
#endif

and use SYSTRACE_ENABLED instead of the systrace_enabled in kernel sources. That would work around the problem.

Perhaps this is fine, but it does not fix bhyve module after SMP-force was removed.

I think the approach in this diff is fine, though I agree with mjg that we might as well exclude all of sys/modules/dtrace if KDTRACE_HOOKS is not defined in kernel options. But standalone builds must work.

[kib]

Give up. Do not build dtrace.

Standalone build seems to work.

[markj]

Something is still needed to fix a "make -C sys/modules/dtrace/systrace". I will commit the patch I suggested earlier if no one has any objections.