[PATCH] drm/drm_bridge: adjust bridge module's refcount

[Jyri Sarha]

On 11/30/16 00:06, Laurent Pinchart wrote:
>> >  /**
>> >   * drm_bridge_remove - remove the given bridge from the global bridge list
>> > @@ -114,6 +118,9 @@ int drm_bridge_attach(struct drm_device *dev, struct
>> > drm_bridge *bridge) if (bridge->dev)
>> >  		return -EBUSY;
>> > 
>> > +	if (!try_module_get(bridge->module))
>> > +		return -ENOENT;
> Isn't this still racy ? What happens if the module is removed right before 
> this call ? Won't the bridge object be freed, and this code then try to call 
> try_module_get() on freed memory ?
> 
> 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.

> The module refcount still needs to be increased in drm_bridge_attach() like 
> you do here, but you'll need to protect it with bridge_lock to avoid a race 
> between try_module_get() and drm_bridge_remove().
>