Received-SPF: pass (google.com: domain of linux-kernel+bounces-50536-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Precedence: bulk
MIME-Version: 1.0
References: <20240201060437.861155-2-davidgow@google.com>
In-Reply-To: <20240201060437.861155-2-davidgow@google.com>
From: Rae Moar <rmoar@google.com>
Date: Fri, 2 Feb 2024 15:16:04 -0500
Message-ID: <CA+GJov6Swgvc+wrCvW3Ujqh_UW1BSRDrp9ccHUX2CVRjWpe1dQ@mail.gmail.com>
Subject: Re: [PATCH] kunit: device: Unregister the kunit_bus on shutdown
To: David Gow <davidgow@google.com>
Cc: Shuah Khan <skhan@linuxfoundation.org>, Matti Vaittinen <mazziesaccount@gmail.com>, 
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>, Brendan Higgins <brendan.higgins@linux.dev>, 
	Maxime Ripard <mripard@kernel.org>, linux-kselftest@vger.kernel.org, 
	kunit-dev@googlegroups.com, linux-kernel@vger.kernel.org, 
	Chaitanya Kumar <chaitanya.kumar.borah@intel.com>, Jani <jani.saarinen@intel.com>, 
	Richard Fitzgerald <rf@opensource.cirrus.com>, intel-gfx@lists.freedesktop.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Thu, Feb 1, 2024 at 1:06=E2=80=AFAM David Gow <davidgow@google.com> wrot=
e:
>
> 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 =3D 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
>