On Fri, Mar 24, 2023 at 02:34:19PM +0800, David Gow wrote: > On Fri, 24 Mar 2023 at 14:11, Matti Vaittinen <mazziesaccount@xxxxxxxxx> wrote: > > > > On 3/23/23 18:36, Maxime Ripard wrote: > > > On Thu, Mar 23, 2023 at 03:02:03PM +0200, Matti Vaittinen wrote: > > >> On 3/23/23 14:29, Maxime Ripard wrote: > > >>> On Thu, Mar 23, 2023 at 02:16:52PM +0200, Matti Vaittinen wrote: > > >>> > > >>> This is the description of what was happening: > > >>> https://lore.kernel.org/dri-devel/20221117165311.vovrc7usy4efiytl@houat/ > > >> > > >> Thanks Maxime. Do I read this correcty. The devm_ unwinding not being done > > >> when root_device_register() is used is not because root_device_unregister() > > >> would not trigger the unwinding - but rather because DRM code on top of this > > >> device keeps the refcount increased? > > > > > > There's a difference of behaviour between a root_device and any device > > > with a bus: the root_device will only release the devm resources when > > > it's freed (in device_release), but a bus device will also do it in > > > device_del (through bus_remove_device() -> device_release_driver() -> > > > device_release_driver_internal() -> __device_release_driver() -> > > > device_unbind_cleanup(), which are skipped (in multiple places) if > > > there's no bus and no driver attached to the device). > > > > > > It does affect DRM, but I'm pretty sure it will affect any framework > > > that deals with device hotplugging by deferring the framework structure > > > until the last (userspace) user closes its file descriptor. So I'd > > > assume that v4l2 and cec at least are also affected, and most likely > > > others. > > > > Thanks for the explanation and patience :) > > > > Thanks from me as well -- this has certainly helped me understand some > of the details of the driver model that had slipped past me. > > > > > > >> If this is the case, then it sounds like a DRM specific issue to me. > > > > > > I mean, I guess. One could also argue that it's because IIO doesn't > > > properly deal with hotplugging. > > > > I must say I haven't been testing the IIO registration API. I've only > > tested the helper API which is not backed up by any "IIO device". (This > > is fine for the helper because it must by design be cleaned-up only > > after the IIO-deregistration). > > > > After your explanation here, I am not convinced IIO wouldn't see the > > same issue if I was testing the devm_iio_device_alloc() & co. > > > > > I'm not sure how that helps. Those are > > > common helpers which should accommodate every framework, > > > > Ok. Fair enough. Besides, if the root-device was sufficient - then I > > would actually not see the need for a helper. People could in that case > > directly use the root_device_register(). So, if helpers are provided > > they should be backed up by a device with a bus then. > > > > I think there is _some_ value in helpers even without a bus, but it's > much more limited: > - It's less confusing if KUnit test devices are using kunit labelled > structs and functions. > - Helpers could use KUnit's resource management API to ensure any > device created is properly unregistered and removed when the test > exits (particularly if it exits early due to, e.g., an assertion). > > I've played around implementing those with a proper struct > kunit_device and the automatic cleanup on test failure, and thus far > it -- like root_device_register -- works for all of the tests except > the drm-test-managed one. Yeah, like I said you need a device that has been bound to a driver for it to work at the moment. I guess for driver mocks we could move to a setup where we get kunit-specific drivers like what Stephen has been implementing for the clocks but I guess we would need to register the kunit tests in the driver probe which doesn't look like it's possible at the moment? Maxime
