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 <[email protected]> > --- > 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... otherwise Reviewed-by: Peter Hutterer <[email protected]> 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 > > _______________________________________________ > [email protected]: X.Org development > Archives: http://lists.x.org/archives/xorg-devel > Info: https://lists.x.org/mailman/listinfo/xorg-devel > _______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
