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

Keith Packard keithp at keithp.com
Sat Dec 12 13:19:59 PST 2015


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.

> 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.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: mutex.c
Type: text/x-csrc
Size: 1146 bytes
Desc: not available
URL: <http://lists.x.org/archives/xorg-devel/attachments/20151212/5456cb7c/attachment.c>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0005-Disable-input-thread-code-with-disable-input-thread..patch
Type: text/x-diff
Size: 10082 bytes
Desc: not available
URL: <http://lists.x.org/archives/xorg-devel/attachments/20151212/5456cb7c/attachment.patch>
-------------- next part --------------


-- 
-keith
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 810 bytes
Desc: not available
URL: <http://lists.x.org/archives/xorg-devel/attachments/20151212/5456cb7c/attachment.sig>


More information about the xorg-devel mailing list