[Mesa-dev] XA tracker pipe-loader woes.

[Ilia Mirkin]

On Wed, Jul 2, 2014 at 9:42 AM, Thomas Hellstrom <thellstrom at vmware.com> wrote:
> On 07/02/2014 12:50 PM, Maarten Lankhorst wrote:
>> op 02-07-14 12:34, Thomas Hellstrom schreef:
>>> Hi, Rob
>>>
>>> It turns out that your patch  st/xa: use pipe-loader to get screen,
>>> makes the xa tracker close the X server driver's drm file descriptor when
>>> XA initialization fails or when XA is closed.
>>>
>>> This breaks X for example on Ubuntu 14.10 if 3D is disabled (typically
>>> on ESX). Pretty nasty bug for us.
>>>
>>> I'm not sure about the correct fix. Personally I don't think the pipe
>>> loader should close the file descriptor on cleanup.
>>> If people insist on it doing so, we can either intentionally leak pipe
>>> loader state or we can perhaps dup() the file descriptor before opening
>>> the loader. I'm not completely sure how DRM responds to that, though.
>>>
>> duping fd's is fine, you get the same underlying file, but the kernel state doesn't change at all.
>> Nouveau uses this solution too, and it works for us. :-)
>>
>> ~Maarten
> Thanks. Good to know. Although I still think it's incorrect of the
> pipe-loader to hijack the file descriptor in that way.

DRI2 does the same thing... in dri2_glx.c:

static void
dri2DestroyScreen(struct glx_screen *base)
{
   struct dri2_screen *psc = (struct dri2_screen *) base;

   /* Free the direct rendering per screen data */
   (*psc->core->destroyScreen) (psc->driScreen);
   driDestroyConfigs(psc->driver_configs);
   close(psc->fd);
   free(psc);
}

As a result of nouveau's screen sharing logic, this would cause a
screen 1 create/screen 2 create/screen 1 destroy/screen 2 destroy
sequence to break rather badly [which would happen if you had multiple
X screens] since the shared screen was using screen 1's fd. If your
object expects the fd to live potentially longer than whatever's
creating you, dupfd seems like a pretty sane choice. (Unfortunately I
don't quite understand the specifics of your situation, but thought
this might help you.)

  -ilia