| Message ID | 20240201060437.861155-2-davidgow@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | kunit: device: Unregister the kunit_bus on shutdown | expand |
On Thu, Feb 1, 2024 at 1:06 AM David Gow <davidgow@google.com> wrote: > > If KUnit is built as a module, and it's unloaded, the kunit_bus is not > unregistered. This causes an error if it's then re-loaded later, as we > try to re-register the bus. > > Unregister the bus and root_device on shutdown, if it looks valid. > > In addition, be more specific about the value of kunit_bus_device. It > is: > - a valid struct device* if the kunit_bus initialised correctly. > - an ERR_PTR if it failed to initialise. > - NULL before initialisation and after shutdown. > > Fixes: d03c720e03bd ("kunit: Add APIs for managing devices") > Signed-off-by: David Gow <davidgow@google.com> Hello, I have tested this with modules and it looks good to me! Thanks! -Rae Reviewed-by: Rae Moar <rmoar@google.com> > --- > > This will hopefully resolve some of the issues linked to from: > https://lore.kernel.org/intel-gfx/DM4PR11MB614179CB9C387842D8E8BB40B97C2@DM4PR11MB6141.namprd11.prod.outlook.com/ > > --- > lib/kunit/device-impl.h | 2 ++ > lib/kunit/device.c | 14 ++++++++++++++ > lib/kunit/test.c | 3 +++ > 3 files changed, 19 insertions(+) > > diff --git a/lib/kunit/device-impl.h b/lib/kunit/device-impl.h > index 54bd55836405..5fcd48ff0f36 100644 > --- a/lib/kunit/device-impl.h > +++ b/lib/kunit/device-impl.h > @@ -13,5 +13,7 @@ > > // For internal use only -- registers the kunit_bus. > int kunit_bus_init(void); > +// For internal use only -- unregisters the kunit_bus. > +void kunit_bus_shutdown(void); > > #endif //_KUNIT_DEVICE_IMPL_H > diff --git a/lib/kunit/device.c b/lib/kunit/device.c > index 074c6dd2e36a..644a38a1f5b1 100644 > --- a/lib/kunit/device.c > +++ b/lib/kunit/device.c > @@ -54,6 +54,20 @@ int kunit_bus_init(void) > return error; > } > > +/* Unregister the 'kunit_bus' in case the KUnit module is unloaded. */ > +void kunit_bus_shutdown(void) > +{ > + /* Make sure the bus exists before we unregister it. */ > + if (IS_ERR_OR_NULL(kunit_bus_device)) > + return; > + > + bus_unregister(&kunit_bus_type); > + > + root_device_unregister(kunit_bus_device); > + > + kunit_bus_device = NULL; > +} > + > /* Release a 'fake' KUnit device. */ > static void kunit_device_release(struct device *d) > { > diff --git a/lib/kunit/test.c b/lib/kunit/test.c > index 31a5a992e646..1d1475578515 100644 > --- a/lib/kunit/test.c > +++ b/lib/kunit/test.c > @@ -928,6 +928,9 @@ static void __exit kunit_exit(void) > #ifdef CONFIG_MODULES > unregister_module_notifier(&kunit_mod_nb); > #endif > + > + kunit_bus_shutdown(); > + > kunit_debugfs_cleanup(); > } > module_exit(kunit_exit); > -- > 2.43.0.429.g432eaa2c6b-goog >
On Fri, 02 Feb 2024, Rae Moar <rmoar@google.com> wrote: > On Thu, Feb 1, 2024 at 1:06 AM David Gow <davidgow@google.com> wrote: >> >> If KUnit is built as a module, and it's unloaded, the kunit_bus is not >> unregistered. This causes an error if it's then re-loaded later, as we >> try to re-register the bus. >> >> Unregister the bus and root_device on shutdown, if it looks valid. >> >> In addition, be more specific about the value of kunit_bus_device. It >> is: >> - a valid struct device* if the kunit_bus initialised correctly. >> - an ERR_PTR if it failed to initialise. >> - NULL before initialisation and after shutdown. >> >> Fixes: d03c720e03bd ("kunit: Add APIs for managing devices") >> Signed-off-by: David Gow <davidgow@google.com> > > Hello, > > I have tested this with modules and it looks good to me! > > Thanks! > -Rae > > Reviewed-by: Rae Moar <rmoar@google.com> Thanks for the patch and review! Is this on its way to some v6.8-rc's? The regression in -rc1 is hurting our CI. Thanks, Jani.
On Wed, Feb 7, 2024 at 8:36 AM Jani Nikula <jani.nikula@linux.intel.com> wrote: > > On Fri, 02 Feb 2024, Rae Moar <rmoar@google.com> wrote: > > On Thu, Feb 1, 2024 at 1:06 AM David Gow <davidgow@google.com> wrote: > >> > >> If KUnit is built as a module, and it's unloaded, the kunit_bus is not > >> unregistered. This causes an error if it's then re-loaded later, as we > >> try to re-register the bus. > >> > >> Unregister the bus and root_device on shutdown, if it looks valid. > >> > >> In addition, be more specific about the value of kunit_bus_device. It > >> is: > >> - a valid struct device* if the kunit_bus initialised correctly. > >> - an ERR_PTR if it failed to initialise. > >> - NULL before initialisation and after shutdown. > >> > >> Fixes: d03c720e03bd ("kunit: Add APIs for managing devices") > >> Signed-off-by: David Gow <davidgow@google.com> > > > > Hello, > > > > I have tested this with modules and it looks good to me! > > > > Thanks! > > -Rae > > > > Reviewed-by: Rae Moar <rmoar@google.com> > > Thanks for the patch and review! > > Is this on its way to some v6.8-rc's? The regression in -rc1 is hurting > our CI. Hello! This patch has been accepted on the kselftest/kunit-fixes branch (https://git.kernel.org/pub/scm/linux/kernel/git/shuah/linux-kselftest.git/commit/?h=kunit-fixes&id=829388b725f8d266ccec32a2f446717d8693eaba) and is heading towards a future v6.8-rc. Thanks! -Rae > > > Thanks, > Jani. > > > -- > Jani Nikula, Intel
diff --git a/lib/kunit/device-impl.h b/lib/kunit/device-impl.h index 54bd55836405..5fcd48ff0f36 100644 --- a/lib/kunit/device-impl.h +++ b/lib/kunit/device-impl.h @@ -13,5 +13,7 @@ // For internal use only -- registers the kunit_bus. int kunit_bus_init(void); +// For internal use only -- unregisters the kunit_bus. +void kunit_bus_shutdown(void); #endif //_KUNIT_DEVICE_IMPL_H diff --git a/lib/kunit/device.c b/lib/kunit/device.c index 074c6dd2e36a..644a38a1f5b1 100644 --- a/lib/kunit/device.c +++ b/lib/kunit/device.c @@ -54,6 +54,20 @@ int kunit_bus_init(void) return error; } +/* Unregister the 'kunit_bus' in case the KUnit module is unloaded. */ +void kunit_bus_shutdown(void) +{ + /* Make sure the bus exists before we unregister it. */ + if (IS_ERR_OR_NULL(kunit_bus_device)) + return; + + bus_unregister(&kunit_bus_type); + + root_device_unregister(kunit_bus_device); + + kunit_bus_device = NULL; +} + /* Release a 'fake' KUnit device. */ static void kunit_device_release(struct device *d) { diff --git a/lib/kunit/test.c b/lib/kunit/test.c index 31a5a992e646..1d1475578515 100644 --- a/lib/kunit/test.c +++ b/lib/kunit/test.c @@ -928,6 +928,9 @@ static void __exit kunit_exit(void) #ifdef CONFIG_MODULES unregister_module_notifier(&kunit_mod_nb); #endif + + kunit_bus_shutdown(); + kunit_debugfs_cleanup(); } module_exit(kunit_exit);
If KUnit is built as a module, and it's unloaded, the kunit_bus is not unregistered. This causes an error if it's then re-loaded later, as we try to re-register the bus. Unregister the bus and root_device on shutdown, if it looks valid. In addition, be more specific about the value of kunit_bus_device. It is: - a valid struct device* if the kunit_bus initialised correctly. - an ERR_PTR if it failed to initialise. - NULL before initialisation and after shutdown. Fixes: d03c720e03bd ("kunit: Add APIs for managing devices") Signed-off-by: David Gow <davidgow@google.com> --- This will hopefully resolve some of the issues linked to from: https://lore.kernel.org/intel-gfx/DM4PR11MB614179CB9C387842D8E8BB40B97C2@DM4PR11MB6141.namprd11.prod.outlook.com/ --- lib/kunit/device-impl.h | 2 ++ lib/kunit/device.c | 14 ++++++++++++++ lib/kunit/test.c | 3 +++ 3 files changed, 19 insertions(+)