From owner-freebsd-current@FreeBSD.ORG Mon Jan 28 18:58:17 2013 Return-Path: Delivered-To: freebsd-current@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by hub.freebsd.org (Postfix) with ESMTP id 1039C21D; Mon, 28 Jan 2013 18:58:17 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from bigwig.baldwin.cx (bigknife-pt.tunnel.tserv9.chi1.ipv6.he.net [IPv6:2001:470:1f10:75::2]) by mx1.freebsd.org (Postfix) with ESMTP id E238825D; Mon, 28 Jan 2013 18:58:16 +0000 (UTC) Received: from pakbsde14.localnet (unknown [38.105.238.108]) by bigwig.baldwin.cx (Postfix) with ESMTPSA id 51DC2B953; Mon, 28 Jan 2013 13:58:16 -0500 (EST) From: John Baldwin To: freebsd-current@freebsd.org Subject: Re: some questions on kern_linker and pre-loaded modules Date: Mon, 28 Jan 2013 10:40:38 -0500 User-Agent: KMail/1.13.5 (FreeBSD/8.2-CBSD-20110714-p22; KDE/4.5.5; amd64; ; ) References: <5103C361.6060605@FreeBSD.org> In-Reply-To: <5103C361.6060605@FreeBSD.org> MIME-Version: 1.0 Content-Type: Text/Plain; charset="iso-8859-15" Content-Transfer-Encoding: 7bit Message-Id: <201301281040.38727.jhb@freebsd.org> X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.2.7 (bigwig.baldwin.cx); Mon, 28 Jan 2013 13:58:16 -0500 (EST) Cc: freebsd-hackers@freebsd.org, Andriy Gapon X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 28 Jan 2013 18:58:17 -0000 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