Re: [PATCH 07/22] drm/tegra: Remove module ownership from the tegra_fb_ops

[Dmitry Osipenko <digetx-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]

On 13.06.2017 18:07, Thierry Reding wrote:
> On Tue, Jun 13, 2017 at 05:00:28PM +0300, Dmitry Osipenko wrote:
>> On 13.06.2017 16:43, Thierry Reding wrote:
>>> On Tue, May 23, 2017 at 03:14:22AM +0300, Dmitry Osipenko wrote:
>>>> The framebuffers console fbcon_startup() increments the tegra_drm module
>>>> 'use' refcount via try_module_get(), causing an interlock of the DRM subsys
>>>> and the tegra_drm modules. In result, the tegra_drm module can't be unloaded
>>>> using rmmod.
>>>>
>>>> Signed-off-by: Dmitry Osipenko <digetx-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>>> ---
>>>>  drivers/gpu/drm/tegra/fb.c | 1 -
>>>>  1 file changed, 1 deletion(-)
>>>
>>> That's done on purpose because otherwise you could just rip out the
>>> driver from under the framebuffer emulation and things would crash.
>>>
>>> My understanding is that the right way to unload a module is to unbind
>>> the driver first (which will cause the framebuffer to be removed and
>>> hence the reference to be dropped) before the rmmod.
>>>
>>> Thierry
>>>
>>
>> Aha, interesting. I'll try the unbinding and will drop this patch from the
>> series, thank you for the clarification. I haven't observed any crashes on a
>> module reloading (framebuffer detached/attached just fine), maybe I was lucky then.
> 
> It's possible that it works by accident. The driver removes the
> framebuffer as part of the driver removal process, so technically
> nothing should crash. However, if, for any reason, anyone was holding
> on to a reference to the framebuffer (not sure if that is even possible)
> the module needs to stay around long enough as well, otherwise the
> function pointers would become dangling. The module reference makes sure
> that this doesn't happen (as long as a framebuffer exists, the ops will
> stick around).
> 
> So even if there isn't a way to make it crash today, the code is still
> correct in taking the reference.

Thank you very much for a more detailed explanation. I've successfully unloaded
the tegra_drm module after unbinding the VT's, the patch is dropped from the series.

echo 0 | tee /sys/devices/virtual/vtconsole/vtcon*/bind

-- 
Dmitry