Hi Jyri, On Wednesday 30 Nov 2016 10:55:10 Jyri Sarha wrote: > On 11/30/16 10:37, Laurent Pinchart wrote: > >>> To fix this properly I think we need to make the bridge object > >>> refcounted, > >>> > >>>>> with a release callback to signal to the bridge driver that memory > >>>>> can be freed. The refcount should be increased in > >>>>> of_drm_find_bridge(), and decreased in a new drm_bridge_put() function > >>>>> (the "fun" part will be to update drivers to call that :-S). > >>> > >>> I think another option would be to find and attach the bridge object > >>> atomically by holding the bridge_lock until the try_module_get() has > >>> succeeded. > >>> > >>> AFAIU, if the module unload is triggered while holding the bridge_lock > >>> but before the try_module_get() call, then try_module_get() would > >>> return false and the attach call will return failure. > > > > This would require combining lookup and attach in all cases. I'm not sure > > that would be very convenient for drivers. Keeping the two operations > > separate would be more flexible. > > That could be avoided if of_drm_find_bridge() would copy the content of > bridge object, in stead of providing a pointer. /me shivers > The attach call could then search for the bridge object again before > continuing. But still in the end the impact to individual drivers would be > equal to adding a simple drm_bridge_put() call. Probably better to go > forward with your suggestion. Please :-) > Even adding the drm_bridge_put() would not be necessary in most cases if > we would add a devm version of getting the bridge object. In 99% of > cases the device probe will fail if the bridge attach fails, and bridge > object refcount would return to zero automatically. devm_* is evil in most cases, especially the devm_kzalloc() function as it ties object lifetime to devices, releasing memory at unbind time when the object can still be referenced elsewhere. Regarding bridges, a devm_of_drm_find_bridge() might make sense as the bridge seems at first glance to (nearly) always be a resource that can be released at DRM unbind time. -- Regards, Laurent Pinchart _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel