[PATCH xserver 4/8] Create a threaded mechanism for input [v3]

Mark Kettenis mark.kettenis at xs4all.nl
Sat Dec 12 14:10:32 PST 2015


> From: Keith Packard <keithp at keithp.com>
> Date: Sat, 12 Dec 2015 13:19:59 -0800
> 
> Mark Kettenis <mark.kettenis at 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.

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 <keithp at 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 <keithp at 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


More information about the xorg-devel mailing list