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

[Emil Velikov]

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).

-Emil