glamor and the sync extension

[Mark Kettenis]

> Date: Sun, 15 Nov 2015 09:53:57 -0800
> From: "Jasper St. Pierre" <jstpierre at mecheye.net>
> 
> Should we just unconditionally enable xshmfence? Are there any OSes
> we care about that can't implement a fence primitive?

There currently is no implementation on OpenBSD, and that's the
platform I care about ;).

The futex implementation supportsLinux and FreeBSD.  The pthreads
implementation should in theory supports many other OSes.  But in
practice support for process-shared mutexes and condition variables
isn't very widespread.  It probably works on Solaris, but not much
else.

An important reason why I haven't pushed for an xshmfence
implementation on OpenBSD is that I have some security concerns.
Especially with the pthreads implementation.

It is trivial to block any application that calls xshmfence_await() by
simply never triggering the fence.  I guess that isn't a major concern
as long the xserver never waits on a fence and only triggers them.
I'm not sure that's the case though with dri3-enable glamor.

However, with the pthreads implementation, even triggering a fence
becomes dangerous since it needs to acquire a shared mutex before
broadcasting the condition.  So a client could trivially DOS the
server by locking the mutex and never unlocking it.  Doesn't even have
to be mailicious.  A buggy client could simply crash after locking the
mutex and the server would be toast.  So in my view the pthreads
implementation is not a robust xshmfence implementationand should
probably be avoided.

> On Sun, Nov 15, 2015 at 8:59 AM, Mark Kettenis <mark.kettenis at xs4all.nl> wrote:
> > Currently glamor hits an assertion on systems that don't have
> > xshmfence.  This happens when the glamor code calls
> > miSyncGetScreenFuncs() because the miSyncScreenPrivateKey has not been
> > set up.  For systems with xshmfence, this happens when
> > miSyncShmScreenInit() gets called, but that code is wrapped within
> > #ifdef HAVE_XSHMFENCE.  The diff below simply calls miSyncSetup()
> > instead if HAVE_XSHMFENCE is not defined.  This makes things work, but
> > I'm not sure if this is the right approach.
> >
> > Thoughts?
> >
> >
> > Index: glamor/glamor_sync.c
> > ===================================================================
> > RCS file: /cvs/xenocara/xserver/glamor/glamor_sync.c,v
> > retrieving revision 1.1
> > diff -u -p -r1.1 glamor_sync.c
> > --- glamor/glamor_sync.c        16 Sep 2015 19:10:21 -0000      1.1
> > +++ glamor/glamor_sync.c        15 Nov 2015 13:02:31 -0000
> > @@ -97,6 +97,9 @@ glamor_sync_init(ScreenPtr screen)
> >  #ifdef HAVE_XSHMFENCE
> >         if (!miSyncShmScreenInit(screen))
> >                 return FALSE;
> > +#else
> > +       if (!miSyncSetup(screen))
> > +               return FALSE;
> >  #endif
> >
> >         screen_funcs = miSyncGetScreenFuncs(screen);
> >