[PATCH xserver] dix: Allow secondary threads to use the work queue

[Peter Hutterer]

On Fri, Feb 24, 2017 at 07:31:32AM +1000, Peter Hutterer wrote:
> On Thu, Feb 23, 2017 at 04:25:35PM -0500, Adam Jackson wrote:
> > The work queue code is reentrant with itself on a single thread, but not
> > when the queue is modified from thread A while being run on thread B.
> > We will only ever run it from the main thread, but the input thread
> > wants to be able to queue work for event emission. Mutex the list so it
> > can do so safely.
> > 
> > This does mean queued work should be cheap to execute since if the lock
> > is contended the input thread will block. That was always true though.
> > If this contention turns out to be a problem, we can fix how we walk the
> > work queue to hold the lock less.
> > 
> > Signed-off-by: Adam Jackson <ajax at redhat.com>
> > ---
> >  dix/dixutils.c | 22 ++++++++++++++++++++++
> >  dix/main.c     |  2 ++
> >  include/dix.h  |  3 +--
> >  3 files changed, 25 insertions(+), 2 deletions(-)
> > 
> > diff --git a/dix/dixutils.c b/dix/dixutils.c
> > index 540023c..a282b15 100644
> > --- a/dix/dixutils.c
> > +++ b/dix/dixutils.c
> > @@ -506,12 +506,29 @@ InitBlockAndWakeupHandlers(void)
> >  
> >  WorkQueuePtr workQueue;
> >  static WorkQueuePtr *workQueueLast = &workQueue;
> > +static pthread_mutex_t wq_mutex;
> 
> I'm not a fan of changing the naming scheme here...

looking at this again because I realised that timers are used as well and it
turns out they already have input_lock(), see 8174daa6bd3f. Can we just use
input_lock()/input_unlock() here rather than a new mutex?

Cheers,
   Peter

> > +
> > +Bool
> > +InitWorkQueue(void)
> > +{
> > +    if (serverGeneration == 1) {
> > +        pthread_mutexattr_t attr;
> > +
> > +        pthread_mutexattr_init(&attr);
> > +        pthread_mutexattr_settype(&attr, PTHREAD_MUTEX_RECURSIVE);
> > +        if (pthread_mutex_init(&wq_mutex, &attr))
> > +            FatalError("Could not initialize work queue lock");
> > +    }
> > +
> > +    return TRUE;
> > +}
> >  
> >  void
> >  ProcessWorkQueue(void)
> >  {
> >      WorkQueuePtr q, *p;
> >  
> > +    pthread_mutex_lock(&wq_mutex);
> >      p = &workQueue;
> >      /*
> >       * Scan the work queue once, calling each function.  Those
> > @@ -530,6 +547,7 @@ ProcessWorkQueue(void)
> >          }
> >      }
> >      workQueueLast = p;
> > +    pthread_mutex_unlock(&wq_mutex);
> >  }
> >  
> >  void
> > @@ -537,6 +555,7 @@ ProcessWorkQueueZombies(void)
> >  {
> >      WorkQueuePtr q, *p;
> >  
> > +    pthread_mutex_lock(&wq_mutex);
> >      p = &workQueue;
> >      while ((q = *p)) {
> >          if (q->client && q->client->clientGone) {
> > @@ -550,6 +569,7 @@ ProcessWorkQueueZombies(void)
> >          }
> >      }
> >      workQueueLast = p;
> > +    pthread_mutex_unlock(&wq_mutex);
> >  }
> >  
> >  Bool
> > @@ -565,8 +585,10 @@ QueueWorkProc(Bool (*function) (ClientPtr pClient, void *closure),
> >      q->client = client;
> >      q->closure = closure;
> >      q->next = NULL;
> > +    pthread_mutex_lock(&wq_mutex);
> >      *workQueueLast = q;
> >      workQueueLast = &q->next;
> > +    pthread_mutex_unlock(&wq_mutex);
> >      return TRUE;
> >  }
> >  
> > diff --git a/dix/main.c b/dix/main.c
> > index 4947062..0278bf5 100644
> > --- a/dix/main.c
> > +++ b/dix/main.c
> > @@ -154,6 +154,8 @@ dix_main(int argc, char *argv[], char *envp[])
> >          DPMSPowerLevel = 0;
> >  #endif
> >          InitBlockAndWakeupHandlers();
> > +        InitWorkQueue();
> > +
> >          /* Perform any operating system dependent initializations you'd like */
> >          OsInit();
> >          if (serverGeneration == 1) {
> > diff --git a/include/dix.h b/include/dix.h
> > index 240018b..3e3a672 100644
> > --- a/include/dix.h
> > +++ b/include/dix.h
> > @@ -240,10 +240,9 @@ extern _X_EXPORT void RemoveBlockAndWakeupHandlers(ServerBlockHandlerProcPtr blo
> >  
> >  extern _X_EXPORT void InitBlockAndWakeupHandlers(void);
> >  
> > +extern _X_EXPORT Bool InitWorkQueue(void);
> >  extern _X_EXPORT void ProcessWorkQueue(void);
> > -
> >  extern _X_EXPORT void ProcessWorkQueueZombies(void);
> > -
> >  extern _X_EXPORT Bool QueueWorkProc(Bool (*function)(ClientPtr clientUnused,
> >                                                       void *closure),
> >                                      ClientPtr client,
> > -- 
> > 2.9.3
> > 
> > _______________________________________________
> > xorg-devel at lists.x.org: X.Org development
> > Archives: http://lists.x.org/archives/xorg-devel
> > Info: https://lists.x.org/mailman/listinfo/xorg-devel
> > 
> _______________________________________________
> xorg-devel at lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel