some questions on kern_linker and pre-loaded modules

[Andriy Gapon]

I.
It seems that linker_preload checks a  module for being a duplicate module only
if the module has MDT_VERSION.

This is probably designed to allow different version of the same module to
co-exist (for some definition of co-exist)?

But, OTOH, this doesn't work well if the module is version-less (no
MODULE_VERSION in the code) and is pre-loaded twice (e.g. once in kernel and
once in a preloaded file).

At present a good example of this is zfsctrl module, which could be present both
in kernel (options ZFS) and in zfs.ko.

I haven't thought about any linker-level resolution for this issue.
I've just tried a plug the ZFS hole for now.

commit ed8b18f2d6c4d1be915bff94cdec0c51a479529f
Author: Andriy Gapon <avg at icyb.net.ua>
Date:   Wed Dec 19 23:29:23 2012 +0200

    [bugfix] zfs: add MODULE_VERSION for zfsctrl

    This should allow the kernel linker to easily detect a situation
    when the module is present both in a kernel and in a preloaded file (zfs.ko).

diff --git a/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_ioctl.c
b/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_ioctl.c
index 10d28c2..5721010 100644
--- a/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_ioctl.c
+++ b/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_ioctl.c
@@ -5599,6 +5599,7 @@ static moduledata_t zfs_mod = {
 	0
 };
 DECLARE_MODULE(zfsctrl, zfs_mod, SI_SUB_VFS, SI_ORDER_ANY);
+MODULE_VERSION(zfsctrl, 1);
 MODULE_DEPEND(zfsctrl, opensolaris, 1, 1, 1);
 MODULE_DEPEND(zfsctrl, krpc, 1, 1, 1);
 MODULE_DEPEND(zfsctrl, acl_nfs4, 1, 1, 1);


II.
It seems that linker_file_register_modules() for the kernel is called after
linker_file_register_modules() is called for all the pre-loaded files.
linker_file_register_modules() for the kernel is called from
linker_init_kernel_modules via SYSINIT(SI_SUB_KLD, SI_ORDER_ANY) and that
happens after linker_preload() which is executed via SYSINIT(SI_SUB_KLD,
SI_ORDER_MIDDLE).

Perhaps this is designed to allow modules in the preloaded files to override
modules compiled into the kernel?

But this doesn't seem to work well.
Because modules from the kernel are not registered yet,
linker_file_register_modules() would be successful for the duplicate modules in
a preloaded file and thus any sysinits present in the file will also be registered.
So, if the module is present both in the kernel and in the preloaded file and
the module has a module event handler (modeventhand_t), then the handler will
registered and called twice.

I cobbled together the following hack, but I am not sure if it is OK or if it
violates fundamental architecture/design of this subsystem.

commit 14ebf07633d0f0ea393801c3e4161d6c37393661
Author: Andriy Gapon <avg at icyb.net.ua>
Date:   Wed Dec 19 23:27:46 2012 +0200

    [wip][experiment] kernel linker: register kernel modules before preloaded
modules...

    Also, skip adding sysinit and sysctl stuff from preloaded modules if module
    registration fails.

    This should result in much saner behavior if a module is present in both
    the kernel and a preloaded file.
    Perhaps, the original intent was to allow the preloaded files to override
    modules present in kernel, but that was extremly fragile because of double
    sysinit registration.

diff --git a/sys/kern/kern_linker.c b/sys/kern/kern_linker.c
index b3ab4df..be46cdf 100644
--- a/sys/kern/kern_linker.c
+++ b/sys/kern/kern_linker.c
@@ -365,6 +365,7 @@ linker_file_register_modules(linker_file_t lf)
 	return (first_error);
 }

+#if 0
 static void
 linker_init_kernel_modules(void)
 {
@@ -374,6 +375,7 @@ linker_init_kernel_modules(void)

 SYSINIT(linker_kernel, SI_SUB_KLD, SI_ORDER_ANY, linker_init_kernel_modules,
     0);
+#endif

 static int
 linker_load_file(const char *filename, linker_file_t *result)
@@ -1599,7 +1601,11 @@ restart:
 		printf("KLD file %s is missing dependencies\n", lf->filename);
 		linker_file_unload(lf, LINKER_UNLOAD_FORCE);
 	}
-
+#if 1
+	error = linker_file_register_modules(linker_kernel_file);
+	if (error)
+		printf("linker_file_register_modules(linker_kernel_file) failed: %d\n", error);
+#endif
 	/*
 	 * We made it. Finish off the linking in the order we determined.
 	 */
@@ -1642,13 +1648,15 @@ restart:
 		 * Now do relocation etc using the symbol search paths
 		 * established by the dependencies
 		 */
+		error = linker_file_register_modules(lf);
+		if (error)
+			goto fail;
 		error = LINKER_LINK_PRELOAD_FINISH(lf);
 		if (error) {
 			printf("KLD file %s - could not finalize loading\n",
 			    lf->filename);
 			goto fail;
 		}
-		linker_file_register_modules(lf);
 		if (linker_file_lookup_set(lf, "sysinit_set", &si_start,
 		    &si_stop, NULL) == 0)
 			sysinit_add(si_start, si_stop);

-- 
Andriy Gapon