Mark Kettenis <mark.kette...@xs4all.nl> writes: > I'd say that would be overkill. The use of recursive mutexes is > somewhat controversal, which is almost certainly why they weren't part > of POSIX initially. But they were part of Unix98 and present as an > XSI extension in POSIX until they were moved to base in 2008.
I would argue that recursive mutexes aren't 'controversial', they're just a bad idea used by lazy programmers. However, given that the existign code uses what is effectively a recursive mutex, converting to threaded input *and* eliminating the mutex recursion at the same time seems like a worse idea. > So unless there is real evidence that there are still supported > systems out there that don't provide PTHREAD_MUTEX_RECURSIVE, I'd stop > at 1) and disable the input thread support on systems that don't > provide it. Adding fallbacks just increases the maintenance burden. Linux doesn't appear to provide PTHREAD_RECURSIVE_MUTEX_INITIALIZER, which seems rather odd to me. > Must admit that I have an agenda here. I'd like to avoid 3) as this > might encourage people to use __thread in other places in the Xorg > codebase. Mesa already uses __thread extensively, and it looks to provide significant performance benefits. I did some simple benchmarking today with PTHREAD_RECURSIVE_MUTEX_INITIALIZER_NP vs __thread, and my test program ran twice as fast using __thread compared with recursive mutexes (see attached). So, what I suggest is that we use recursive mutexes where supported, falling back to __thread/__declspec(thread). Two paths seems like a fine plan to me, offering portability to a wider set of systems than either one alone, while preferring the POSIX standard where supported.
#define _GNU_SOURCE 1
#include <pthread.h>
#define LOOPS 10000000
#define DEPTH 10
#define THREADS 10
#define RECURSIVE 0
#if RECURSIVE
static pthread_mutex_t mutex = PTHREAD_RECURSIVE_MUTEX_INITIALIZER_NP;
static inline void lock() {
pthread_mutex_lock(&mutex);
}
static inline void unlock() {
pthread_mutex_unlock(&mutex);
}
#else
static pthread_mutex_t mutex = PTHREAD_MUTEX_INITIALIZER;
static __thread int count;
static inline void lock() {
if (count++ == 0)
pthread_mutex_lock(&mutex);
}
static inline void unlock() {
if (--count == 0)
pthread_mutex_unlock(&mutex);
}
#endif
void idle(void);
void idle(void) {
}
static inline void acquire(int depth) {
while (depth--) {
lock();
idle();
}
}
static inline void release(int depth) {
while (depth--) {
idle();
unlock();
}
}
static void *doit(void *arg) {
int i;
for (i = 0; i < LOOPS; i++) {
acquire(DEPTH);
release(DEPTH);
}
}
int
main (int argc, char **argv)
{
int t;
pthread_t threads[THREADS];
for (t = 0; t < THREADS; t++)
pthread_create(&threads[t], NULL, doit, NULL);
for (t = 0; t < THREADS; t++)
pthread_join(threads[t], NULL);
}
From 55f483cff8d660e444a65026e57f793c06a66ce3 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 | 21 ++++++++++---
hw/xfree86/sdksyms.sh | 4 ---
include/dix-config.h.in | 9 ++++++
include/input.h | 23 ++------------
m4/ax_tls.m4 | 78 +++++++++++++++++++++++++++++++++++++++++++++++
os/inputthread.c | 80 +++++++++++++++++++++++++++++++++++++++++++++++++
6 files changed, 187 insertions(+), 28 deletions(-)
create mode 100644 m4/ax_tls.m4
diff --git a/configure.ac b/configure.ac
index bc15581..6f04062 100644
--- a/configure.ac
+++ b/configure.ac
@@ -821,13 +821,25 @@ 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_RECURSIVE_MUTEX_INITIALIZER], [HAVE_RECURSIVE_MUTEX=yes], [HAVE_RECURSIVE_MUTEX=no], [[#include <pthread.h>]])
+
+AX_TLS()
+
+THREAD_DEFAULT=no
+
+case x$HAVE_RECURSIVE_MUTEX in
+xyes)
+ AC_DEFINE(HAVE_PTHREAD_RECURSIVE_MUTEX_INITIALIZER, 1, [Have PTHREAD_RECURSIVE_MUTEX_INITIALIZER])
THREAD_DEFAULT=yes ;;
*)
- THREAD_DEFAULT=no ;;
+ case "x$ac_cv_tls" in
+ xnone)
+ ;;
+ *)
+ THREAD_DEFAULT=yes ;;
+ esac
esac
+
AC_ARG_ENABLE(input-thread, AS_HELP_STRING([--enable-input-thread],
[Enable input threads]),
[INPUTTHREAD=$enableval], [INPUTTHREAD=$THREAD_DEFAULT])
@@ -848,6 +860,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..7d1774f 100644
--- a/include/dix-config.h.in
+++ b/include/dix-config.h.in
@@ -521,4 +521,13 @@
/* Have setitimer support */
#undef HAVE_SETITIMER
+/* Have recursive mutexes */
+#undef HAVE_PTHREAD_RECURSIVE_MUTEX_INITIALIZER
+
+/* Use this define for thread-specific variables */
+#undef TLS
+
+/* 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/m4/ax_tls.m4 b/m4/ax_tls.m4
new file mode 100644
index 0000000..41620a7
--- /dev/null
+++ b/m4/ax_tls.m4
@@ -0,0 +1,78 @@
+# ===========================================================================
+# http://autoconf-archive.cryp.to/ax_tls.html
+# ===========================================================================
+#
+# SYNOPSIS
+#
+# AX_TLS
+#
+# DESCRIPTION
+#
+# Provides a test for the compiler support of thread local storage (TLS)
+# extensions. Defines TLS if it is found. Currently only knows about GCC
+# and MSVC. I think SunPro uses the same as GCC, and Borland apparently
+# supports either.
+#
+# LAST MODIFICATION
+#
+# 2008-04-12
+#
+# COPYLEFT
+#
+# Copyright (c) 2008 Alan Woodland <aj...@aber.ac.uk>
+#
+# This program is free software: you can redistribute it and/or modify it
+# under the terms of the GNU General Public License as published by the
+# Free Software Foundation, either version 3 of the License, or (at your
+# option) any later version.
+#
+# This program is distributed in the hope that it will be useful, but
+# WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General
+# Public License for more details.
+#
+# You should have received a copy of the GNU General Public License along
+# with this program. If not, see <http://www.gnu.org/licenses/>.
+#
+# As a special exception, the respective Autoconf Macro's copyright owner
+# gives unlimited permission to copy, distribute and modify the configure
+# scripts that are the output of Autoconf when processing the Macro. You
+# need not follow the terms of the GNU General Public License when using
+# or distributing such scripts, even though portions of the text of the
+# Macro appear in them. The GNU General Public License (GPL) does govern
+# all other use of the material that constitutes the Autoconf Macro.
+#
+# This special exception to the GPL applies to versions of the Autoconf
+# Macro released by the Autoconf Macro Archive. When you make and
+# distribute a modified version of the Autoconf Macro, you may extend this
+# special exception to the GPL to apply to your modified version as well.
+
+AC_DEFUN([AX_TLS], [
+ AC_MSG_CHECKING(for thread local storage (TLS) class)
+ AC_CACHE_VAL(ac_cv_tls, [
+ ax_tls_keywords="__thread __declspec(thread) none"
+ for ax_tls_keyword in $ax_tls_keywords; do
+ case $ax_tls_keyword in
+ none) ac_cv_tls=none ; break ;;
+ *)
+ AC_TRY_COMPILE(
+ [#include <stdlib.h>
+ static void
+ foo(void) {
+ static ] $ax_tls_keyword [ int bar;
+ exit(1);
+ }],
+ [],
+ [ac_cv_tls=$ax_tls_keyword ; break],
+ ac_cv_tls=none
+ )
+ esac
+ done
+])
+
+ if test "$ac_cv_tls" != "none"; then
+ dnl AC_DEFINE([TLS], [], [If the compiler supports a TLS storage class define it to that here])
+ AC_DEFINE_UNQUOTED([TLS], $ac_cv_tls, [If the compiler supports a TLS storage class define it to that here])
+ fi
+ AC_MSG_RESULT($ac_cv_tls)
+])
diff --git a/os/inputthread.c b/os/inputthread.c
index 7c87e87..a80b8b6 100644
--- a/os/inputthread.c
+++ b/os/inputthread.c
@@ -40,6 +40,8 @@
#include "opaque.h"
#include "osdep.h"
+#if INPUTTHREAD
+
Bool InputThreadEnable = TRUE;
/**
@@ -377,3 +379,81 @@ InputThreadFini(void)
free(inputThreadInfo);
inputThreadInfo = NULL;
}
+
+#if HAVE_PTHREAD_RECURSIVE_MUTEX_INITIALIZER
+
+static pthread_mutex_t input_mutex = PTHREAD_RECURSIVE_MUTEX_INITIALIZER;
+static int input_mutex_count;
+
+void input_lock(void) {
+ 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);
+ }
+}
+
+#else
+
+#ifdef TLS
+
+static __thread int input_mutex_count;
+static pthread_mutex_t input_mutex = PTHREAD_MUTEX_INITIALIZER;
+
+void input_lock(void) {
+ if (input_mutex_count++ == 0)
+ pthread_mutex_lock(&input_mutex);
+}
+
+void input_unlock(void) {
+ if (--input_mutex_count == 0)
+ pthread_mutex_unlock(&input_mutex);
+}
+
+void input_force_unlock(void) {
+ if (input_mutex_count) {
+ input_mutex_count = 0;
+ pthread_mutex_unlock(&input_mutex);
+ }
+}
+
+#endif /* TLS */
+
+#endif /* HAVE_PTHREAD_RECURSIVE_MUTEX_INITIALIZER */
+
+#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
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
