On Wed, Jul 20, 2022 at 05:26:02PM +0800, David Gow wrote: > On Wed, Jul 20, 2022 at 1:58 AM Luis Chamberlain <mcgrof@xxxxxxxxxx> wrote: > > > > On Wed, Jul 13, 2022 at 08:24:32AM -0700, Daniel Latypov wrote: > > > On Tue, Jul 12, 2022 at 5:52 PM David Gow <davidgow@xxxxxxxxxx> wrote: > > > > > > > > The new KUnit module handling has KUnit test suites listed in a > > > > .kunit_test_suites section of each module. This should be loaded when > > > > the module is, but at the moment this only happens if KUnit is built-in. > > > > > > > > Also load this when KUnit is enabled as a module: it'll not be usable > > > > unless KUnit is loaded, but such modules are likely to depend on KUnit > > > > anyway, so it's unlikely to ever be loaded needlessly. > > > > > > This seems reasonable to me. > > > > > > Question: what happens in this case? > > > 1. insmod <test-module> > > > 2. insmod kunit > > > 3. rmmod <test-module> > > > > > > I think on 3, we'll call the cleanup code, __kunit_test_suites_exit(), > > > for <test-module>, I think? > > > But we never called __kunit_test_suites_init(). > > > My fear is what breaks as a result of this precondition break. > > I don't think this should be possible: any module with KUnit tests > will depend on the 'kunit' module (or, at least, kunit symbols), so > shouldn't load without kunit already present. > > If modprobe is used, kunit will automatically be loaded. If insmod is > used directly, loading the first module should error out with > something like: > [ 82.393629] list_test: loading test module taints kernel. > [ 82.409607] list_test: Unknown symbol kunit_binary_ptr_assert_format (err -2) > [ 82.409657] list_test: Unknown symbol kunit_do_failed_assertion (err -2) > [ 82.409799] list_test: Unknown symbol kunit_binary_assert_format (err -2) > [ 82.409820] list_test: Unknown symbol kunit_unary_assert_format (err -2) > insmod: ERROR: could not insert module > /lib/modules/5.19.0-rc1-15284-g9ec67db0c271/kernel/lib/list-test.ko: > Unknown symbol in module This can be fixed with a request_module() call. And since this is a generic requirement, you can have the wrappers do it for you. > Maybe you could get into some trouble by force-removing modules at > various points, but you're in undefined behaviour generally at that > point, so I don't think there's much point going out-of-our-way to try > to support that. You can prevent that by refcounting the kunit module / symbols, by each test. Luis
