Mark Kettenis <mark.kette...@xs4all.nl> writes:

> It isn't odd, because PTHREAD_RECURSIVE_MUTEX_INITIALIZE isn't in the
> standard.  You'll have to explicitly initialize the mutex with
> pthread_mutex_init() to get a recursive mutex.

Sigh. That's a pain in this case; the first use of the mutex occurs
before the DDX initializes the input thread. I'll use
PTHREAD_RECURSIVE_MUTEX_INITIALIZER_NP where it exists, and fallback to
a test inside input_lock() that initializes the mutex on first
use. That's "safe" because the first use will always happen before the
input thread is started.

> You can always prove a point with an appropriately constructed
> microbenchmark ;).  Seriously though, I do hope that the overhead of
> the recursive mutex isn't noticable in the Xorg input thread.

I was surprised at how bad the Linux recursive mutexes performed
compared with a thread-local counter, if you actually used recursion. In
the (we hope common) case where the mutex gets acquired only once, they
are the same.

Depth   Recursive       Counter
10      30.3ns          13.2ns 
1       100.4ns         100.4ns

> Anyway, the diff below won't fly as
> PTHREAD_RECURSIVE_MUTEX_INITIALIZER isn't in the standard and
> therefore not widely available.  On OpenBSD we don't even have
> PTHREAD_RECURSIVE_MUTEX_INITIALIZER_NP, and supporting such a thing
> would be difficult.

That's unfortunate, but easy enough to work around with a bit of
conditional code.

Here's code which works only with recursive mutexes, initializing it
explicitly when PTHREAD_RECURSIVE_MUTEX_INITIALIZER_NP is not available.

From 700fb83a0afbfce19bb91967f619cde6ead900a6 Mon Sep 17 00:00:00 2001
From: Keith Packard <kei...@keithp.com>
Date: Thu, 10 Dec 2015 11:36:34 -0800
Subject: [PATCH xserver 05/22] Disable input thread code with
 --disable-input-thread. RECURSIVE_MUTEX support

This is a pair of changes to the threaded input code which should be
merged with that patch; I'm publishing it separately for review before
doing that.

The first change is to respect the --disable-input-thread
configuration option and disable all of the threaded input code in
that case. What remains are just stubs that get input drivers to work
in this situation without changing the API or ABI.

The second is to add ax_tls.m4 which detects
PTHREAD_RECURSIVE_MUTEX_INITIALIZER and either __thread or
__declspec(thread) support in the system and to use that to figure out
how to get recursive mutex support in the server. We prefer
PTHREAD_RECURSIVE_MUTEX_SUPPORt where available, falling back to
__thread/__declspec(thread) support if necessary.

Signed-off-by: Keith Packard <kei...@keithp.com>

fixup for thread variables
---
 configure.ac            | 14 ++++++----
 hw/xfree86/sdksyms.sh   |  4 ---
 include/dix-config.h.in |  3 +++
 include/input.h         | 23 +++-------------
 os/inputthread.c        | 72 +++++++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 87 insertions(+), 29 deletions(-)

diff --git a/configure.ac b/configure.ac
index bc15581..abea281 100644
--- a/configure.ac
+++ b/configure.ac
@@ -821,13 +821,16 @@ SDK_REQUIRED_MODULES="$XPROTO $RANDRPROTO $RENDERPROTO $XEXTPROTO $INPUTPROTO $K
 # Make SDK_REQUIRED_MODULES available for inclusion in xorg-server.pc
 AC_SUBST(SDK_REQUIRED_MODULES)
 
-# input thread setup
-case $host_os in
-linux*)
+AC_CHECK_DECL([PTHREAD_MUTEX_RECURSIVE], [HAVE_RECURSIVE_MUTEX=yes], [HAVE_RECURSIVE_MUTEX=no], [[#include <pthread.h>]])
+
+THREAD_DEFAULT=no
+
+case x$HAVE_RECURSIVE_MUTEX in
+xyes)
+	AC_DEFINE(HAVE_PTHREAD_MUTEX_RECURSIVE, 1, [Have recursive mutexes])
 	THREAD_DEFAULT=yes ;;
-*)
-	THREAD_DEFAULT=no ;;
 esac
+
 AC_ARG_ENABLE(input-thread, AS_HELP_STRING([--enable-input-thread],
 	     [Enable input threads]),
 	     [INPUTTHREAD=$enableval], [INPUTTHREAD=$THREAD_DEFAULT])
@@ -848,6 +851,7 @@ if test "x$INPUTTHREAD" = "xyes" ; then
     esac
     SYS_LIBS="$SYS_LIBS $THREAD_LIB"
     CFLAGS="$CFLAGS $THREAD_CFLAGS"
+    AC_DEFINE(INPUTTHREAD, 1, [Use a separate input thread])
 fi
 
 REQUIRED_MODULES="$FIXESPROTO $DAMAGEPROTO $XCMISCPROTO $XTRANS $BIGREQSPROTO $SDK_REQUIRED_MODULES"
diff --git a/hw/xfree86/sdksyms.sh b/hw/xfree86/sdksyms.sh
index fb2eaa1..5391b72 100755
--- a/hw/xfree86/sdksyms.sh
+++ b/hw/xfree86/sdksyms.sh
@@ -344,10 +344,6 @@ BEGIN {
            n = 1;
         }
 
-	if ($n ~ /__thread/) {
-	   next;
-	}
-
 	# skip attribute, if any
 	while ($n ~ /^(__attribute__|__global)/ ||
 	    # skip modifiers, if any
diff --git a/include/dix-config.h.in b/include/dix-config.h.in
index 940d2b7..0773b6d 100644
--- a/include/dix-config.h.in
+++ b/include/dix-config.h.in
@@ -521,4 +521,7 @@
 /* Have setitimer support */
 #undef HAVE_SETITIMER
 
+/* Use input thread */
+#undef INPUTTHREAD
+
 #endif /* _DIX_CONFIG_H_ */
diff --git a/include/input.h b/include/input.h
index 01ea4d9..afd066f 100644
--- a/include/input.h
+++ b/include/input.h
@@ -713,26 +713,9 @@ extern _X_HIDDEN void input_constrain_cursor(DeviceIntPtr pDev, ScreenPtr screen
                                              int *out_x, int *out_y,
                                              int *nevents, InternalEvent* events);
 
-extern _X_EXPORT pthread_mutex_t input_mutex;
-extern _X_EXPORT __thread int input_mutex_count;
-
-static inline void input_lock(void) {
-    if (input_mutex_count++ == 0)
-        pthread_mutex_lock(&input_mutex);
-}
-
-static inline void input_unlock(void) {
-    if (--input_mutex_count == 0)
-        pthread_mutex_unlock(&input_mutex);
-    assert(input_mutex_count >= 0);
-}
-
-static inline void input_force_unlock(void) {
-    if (input_mutex_count > 0) {
-        input_mutex_count = 0;
-        pthread_mutex_unlock(&input_mutex);
-    }
-}
+extern _X_EXPORT void input_lock(void);
+extern _X_EXPORT void input_unlock(void);
+extern _X_EXPORT void input_force_unlock(void);
 
 extern void InputThreadPreInit(void);
 extern void InputThreadInit(void);
diff --git a/os/inputthread.c b/os/inputthread.c
index 7c87e87..163a2b3 100644
--- a/os/inputthread.c
+++ b/os/inputthread.c
@@ -40,6 +40,8 @@
 #include "opaque.h"
 #include "osdep.h"
 
+#if INPUTTHREAD
+
 Bool InputThreadEnable = TRUE;
 
 /**
@@ -70,6 +72,49 @@ static InputThreadInfo *inputThreadInfo;
 static int hotplugPipeRead = -1;
 static int hotplugPipeWrite = -1;
 
+static int input_mutex_count;
+
+#ifdef PTHREAD_RECURSIVE_MUTEX_INITIALIZER_NP
+static pthread_mutex_t input_mutex = PTHREAD_RECURSIVE_MUTEX_INITIALIZER_NP;
+#else
+static pthread_mutex_t input_mutex;
+static Bool input_mutex_initialized;
+#endif
+
+void
+input_lock(void)
+{
+#ifndef PTHREAD_RECURSIVE_MUTEX_INITIALIZER_NP
+    if (!input_mutex_initialized) {
+        pthread_mutexattr_t mutex_attr;
+
+        input_mutex_initialized = TRUE;
+        pthread_mutexattr_init(&mutex_attr);
+        pthread_mutexattr_settype(&mutex_attr, PTHREAD_MUTEX_RECURSIVE);
+        pthread_mutex_init(&input_mutex, &mutex_attr);
+    }
+#endif
+    pthread_mutex_lock(&input_mutex);
+    ++input_mutex_count;
+}
+
+void
+input_unlock(void)
+{
+    --input_mutex_count;
+    pthread_mutex_unlock(&input_mutex);
+}
+
+void
+input_force_unlock(void)
+{
+    if (pthread_mutex_trylock(&input_mutex) == 0) {
+        /* unlock +1 times for the trylock */
+        while (input_mutex_count-- >= 0)
+            pthread_mutex_unlock(&input_mutex);
+    }
+}
+
 /**
  * Notify a thread about the availability of new asynchronously enqueued input
  * events.
@@ -377,3 +422,30 @@ InputThreadFini(void)
     free(inputThreadInfo);
     inputThreadInfo = NULL;
 }
+
+#else /* INPUTTHREAD */
+
+Bool InputThreadEnable = FALSE;
+
+void input_lock(void) {}
+void input_unlock(void) {}
+void input_force_unlock(void) {}
+
+void InputThreadPreInit(void) {}
+void InputThreadInit(void) {}
+void InputThreadFini(void) {}
+
+int InputThreadRegisterDev(int fd,
+                           NotifyFdProcPtr readInputProc,
+                           void *readInputArgs)
+{
+    return SetNotifyFd(fd, readInputProc, X_NOTIFY_READ, readInputArgs);
+}
+
+extern int InputThreadUnregisterDev(int fd)
+{
+    RemoveNotifyFd(fd);
+    return 1;
+}
+
+#endif
-- 
2.6.4


-- 
-keith

Attachment: signature.asc
Description: PGP signature

_______________________________________________
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Reply via email to