Messages in this thread | | | Subject | Re: [PATCH v5 2/6] drm/bridge: Add a devm_ allocator for panel bridge. | From | Noralf Trønnes <> | Date | Sat, 5 Aug 2017 16:47:27 +0200 |
| |
Den 05.08.2017 12.59, skrev Noralf Trønnes: > (I had to switch to Daniel's Intel address to get this sent) > > Den 05.08.2017 00.19, skrev Ilia Mirkin: >> On Fri, Aug 4, 2017 at 4:43 PM, Eric Anholt <eric@anholt.net> wrote: >>> Laurent Pinchart <laurent.pinchart@ideasonboard.com> writes: >>> >>>> Hi Eric, >>>> >>>> (CC'ing Daniel) >>>> >>>> Thank you for the patch. >>>> >>>> On Tuesday 18 Jul 2017 14:05:06 Eric Anholt wrote: >>>>> This will let drivers reduce the error cleanup they need, in >>>>> particular the "is_panel_bridge" flag. >>>>> >>>>> v2: Slight cleanup of remove function by Andrzej >>>> I just want to point out that, in the context of Daniel's work on >>>> hot-unplug, >>>> 90% of the devm_* allocations are wrong and will get in the way. >>>> All DRM core >>>> objects that are accessible one way or another from userspace will >>>> need to be >>>> properly reference-counted and freed only when the last reference >>>> disappears, >>>> which could be well after the corresponding device is removed. I >>>> believe this >>>> could be one such objects :-/ >>> Sure, if you're hotplugging, your life is pain. For non-hotpluggable >>> devices, like our SOC platform devices (current panel-bridge >>> consumers), >>> this still seems like an excellent simplification of memory management. >> At that point you may as well make your module non-unloadable, and >> return failure when trying to remove a device from management by the >> driver (whatever the opposite of "probe" is, I forget). Hotplugging >> doesn't only happen when physically removing, it can happen for all >> kinds of reasons... and userspace may still hold references in some of >> those cases. > > If drm_open() gets a ref on dev->dev and puts it in drm_release(), > won't that delay devm_* cleanup until userspace is done? >
It seems plausible looking at the code:
void device_initialize(struct device *dev) { [...] kobject_init(&dev->kobj, &device_ktype); [...] }
static struct kobj_type device_ktype = { .release = device_release, };
/** * device_release - free device structure. * @kobj: device's kobject. * * This is called once the reference count for the object * reaches 0. We forward the call to the device's release * method, which should handle actually freeing the structure. */ static void device_release(struct kobject *kobj) { [...] devres_release_all(dev); [...] }
Last put call chain: put_device() -> kobject_put() -> kref_put() -> kobject_release() -> kobject_cleanup() -> device_release() -> devres_release_all()
But I haven't actually tried it, so I might be mistaken.
| |