> From: Keith Packard <[email protected]> > Date: Sat, 12 Dec 2015 13:19:59 -0800 > > Mark Kettenis <[email protected]> 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.
You're excused ;). > > 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. 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. > > 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). 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. > 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. I still dont see the point as I expect all systems that support __thread to support recursive mutexes as well. 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. Cheers, Mark > --=-=-= > Content-Type: text/x-diff > Content-Disposition: inline; > filename=0005-Disable-input-thread-code-with-disable-input-thread..patch > Content-Transfer-Encoding: quoted-printable > > From=2055f483cff8d660e444a65026e57f793c06a66ce3 Mon Sep 17 00:00:00 2001 > From: Keith Packard <[email protected]> > 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 <[email protected]> > > 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 _______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
