[Mesa-dev] [PATCH] gallium/winsys/kms: Close drm device filedescriptor on kms_dri_sw_winsys release

[Emil Velikov]

On 14 September 2016 at 15:04, Emil Velikov <emil.l.velikov at gmail.com> wrote:
> Hi Lukasz,
>
> On 5 September 2016 at 17:48, Lukasz Spintzyk <lukasz.spintzyk at gmail.com> wrote:
>> This closes filedescriptor owned by kms_dri_sw_winsys struct. It fixes issue
>> where removal of udl or evdi module used by DisplayLink devices was impossible
>> due to not closed filedescriptors.
>>
>> When this file descriptor was not closed then command
>> rmmod udl was returning error "Module udl is in use".
>> By this fix xserver does not prevent module removal when usb device
>> is unplugged.
>>
>> Signed-off-by: Lukasz Spintzyk <lukasz.spintzyk at gmail.com>
>> ---
>>  src/gallium/winsys/sw/kms-dri/kms_dri_sw_winsys.c | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/src/gallium/winsys/sw/kms-dri/kms_dri_sw_winsys.c b/src/gallium/winsys/sw/kms-dri/kms_dri_sw_winsys.c
>> index 07eca99..f06ccef 100644
>> --- a/src/gallium/winsys/sw/kms-dri/kms_dri_sw_winsys.c
>> +++ b/src/gallium/winsys/sw/kms-dri/kms_dri_sw_winsys.c
>> @@ -371,6 +371,9 @@ kms_sw_displaytarget_display(struct sw_winsys *ws,
>>  static void
>>  kms_destroy_sw_winsys(struct sw_winsys *winsys)
>>  {
>> +   struct kms_sw_winsys *kms_sw = kms_sw_winsys(winsys);
>> +
>> +   close(kms_sw->fd);
> AFAICT not even a single driver/winsys takes ownership of the fd. As
> such they should not close() it.
>
> From a quick skim - on the st/dri side, we explicitly provide new fd
> (we dup the one passed from the upper layers) to the driver. Similarly
> in vl/dri3 we open the device, yet neither of these closes the fd.
>
> Imho it might be worth going through the code paths adding comments
> about the fd ownership at different stages while fixing any leaks that
> you/others come across.
>
> Note: Multiple displays/Xinerama and GL-VDPAU interop are sensitive on
> the area, so I'd suggest testing things carefully and poking the last
> dev who was working in the area (git log/blame are your friend).
>
Actually before embarking on the above make sure that the mesa that
you're using has commit 459cc94507071eec18b746f57a4ec82578a38b54. I've
had a chat with Hans yesterday and seemingly the origin/story behind
it is (almost) the same as yours.

-Emil


> -Emil