Re: some questions on kern_linker and pre-loaded modules

On Saturday, January 26, 2013 6:52:01 am Andriy Gapon wrote:
> 
> 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)?

Yes, that is likely true, but it is a bit dubious.

> 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).

Yes.

> 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.

I think we should require all modules declared by DECLARE_MODULE() to have a
version.  You might be able to enforce that by failing to register a linker
file if it contains any modules that do not include at least one version
metadata note in the same linker file.  You could check this before running
the MOD_LOAD handlers (though after running SYSINITs).  Truly fixing this would
mean making module data true metadata that is parsed by the linker rather than
having it all provided to the kernel via SYSINITs so that you could evaluate
this before running SYSINITs.  That is a larger project however.  I think your
fix for zfsctrl is correct.
 
> 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?

Yes, likely so.

> 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.

Yes, I think it is too hard at present to safely allow a linker file to
override the same module in a kernel, so the duplicate linker file should
just be rejected entirely.  I'm not sure if your change is completely
correct for that.

-- 
John Baldwin