On Fri, Mar 24, 2023 at 02:34:19PM +0800, David Gow wrote:
On Fri, 24 Mar 2023 at 14:11, Matti Vaittinen mazziesaccount@gmail.com 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