Re: [PATCH xserver 5/9] Create a threaded mechanism for input [v5]

2016-05-12 Thread Keith Packard
Emil Velikov  writes:

> The commit summary does not mention anything, just the revision log
> suggests "linux only". Worth adding a couple of words to clarify
> things ?

good point, I reworded the 'v2' comment to say:

   v2: Fix non-Xorg link. Enable where supported by default.

-- 
-keith


signature.asc
Description: PGP signature
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH xserver 5/9] Create a threaded mechanism for input [v5]

2016-05-12 Thread Emil Velikov
On 12 May 2016 at 18:47, Keith Packard  wrote:
> Emil Velikov  writes:
>
>> This will enable it on more platforms than just Linux.
>
> Right, the goal is to use it where available.
>
The commit summary does not mention anything, just the revision log
suggests "linux only". Worth adding a couple of words to clarify
things ?

>> You can use AX_PTHREAD here, instead of open-coding it.
>
> Thanks. I've updated the series to do this; it's on my input-thread
> branch in git://people.freedesktop.org/~keithp/xserver
>
>> Then again I'm wondering if linking with pthread, won't lead to some
>> noticeable perf degradation, as a fair few POSIX functions will now
>> pthread_mutex_lock/unlock as opposed using the Glibc stub.
>
> The big concern I had years ago when we looked at this was malloc, but
> glibc uses arenas to avoid contention, and uncontended mutexes are
> pretty cheap these days.
>
> stdio is a disaster, but we don't use that in any critical paths.
>
> I haven't been able to measure any performance impact from this series,
> but I've only tested on Linux.
>
Good enough for me. Thanks Keith.

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

Re: [PATCH xserver 5/9] Create a threaded mechanism for input [v5]

2016-05-12 Thread Alan Coopersmith

On 05/12/16 10:47 AM, Keith Packard wrote:

Emil Velikov  writes:

Then again I'm wondering if linking with pthread, won't lead to some
noticeable perf degradation, as a fair few POSIX functions will now
pthread_mutex_lock/unlock as opposed using the Glibc stub.


The big concern I had years ago when we looked at this was malloc, but
glibc uses arenas to avoid contention, and uncontended mutexes are
pretty cheap these days.

stdio is a disaster, but we don't use that in any critical paths.

I haven't been able to measure any performance impact from this series,
but I've only tested on Linux.


Solaris 10 and later have the real pthread & mutex functions in libc
and dropped the stubs that were in libc in older releases, so we
already use the thread safe versions and I wouldn't expect any
performance problems on Solaris from that change.

--
-Alan Coopersmith-  alan.coopersm...@oracle.com
 Oracle Solaris Engineering - http://blogs.oracle.com/alanc
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH xserver 5/9] Create a threaded mechanism for input [v5]

2016-05-12 Thread Keith Packard
Emil Velikov  writes:

> This will enable it on more platforms than just Linux.

Right, the goal is to use it where available.

> You can use AX_PTHREAD here, instead of open-coding it.

Thanks. I've updated the series to do this; it's on my input-thread
branch in git://people.freedesktop.org/~keithp/xserver

> Then again I'm wondering if linking with pthread, won't lead to some
> noticeable perf degradation, as a fair few POSIX functions will now
> pthread_mutex_lock/unlock as opposed using the Glibc stub.

The big concern I had years ago when we looked at this was malloc, but
glibc uses arenas to avoid contention, and uncontended mutexes are
pretty cheap these days.

stdio is a disaster, but we don't use that in any critical paths.

I haven't been able to measure any performance impact from this series,
but I've only tested on Linux.

-- 
-keith


signature.asc
Description: PGP signature
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH xserver 5/9] Create a threaded mechanism for input [v5]

2016-05-12 Thread Emil Velikov
On 11 May 2016 at 21:54, Keith Packard  wrote:
> The current SIGIO signal handler method, used at generation of input events,
> has a bunch of oddities. This patch introduces an alternative way using a
> thread, which is used to select() all input device file descriptors.
>
> A mutex was used to control the access to input structures by the main and 
> input
> threads. Two pipes to emit alert events (such hotplug ones) and guarantee the
> proper communication between them was also used.
>
> Co-authored-by: Fernando Carrijo 
> Signed-off-by: Tiago Vignatti 
>
> v2: Fix non-Xorg link, and disable by default on non-Linux
>
> This also splits out the actual enabling of input threads to
> DDX-specific patches which follow
>
> v3: Make the input lock recursive
>
> v4: Use regular RECURSIVE_MUTEXes instead of rolling our own
> Respect the --disable-input-thread configuration option by
> providing stubs that expose the same API/ABI.
>
> Respond to style comments from Peter Hutterer.
>
> v5: use __func__ in inputthread debug and error mesages.
>
> Respond to style comments from Peter Hutterer.
>
> Signed-off-by: Adam Jackson 
> Signed-off-by: Keith Packard 
> ---
>  configure.ac|  31 
>  dix/globals.c   |   9 +
>  dix/main.c  |   9 +-
>  include/dix-config.h.in |   3 +
>  include/input.h |  20 ++-
>  include/misc.h  |   1 +
>  mi/mieq.c   |  38 +---
>  os/Makefile.am  |   1 +
>  os/inputthread.c| 455 
> 
>  os/utils.c  |   9 +-
>  10 files changed, 527 insertions(+), 49 deletions(-)
>  create mode 100644 os/inputthread.c
>
> diff --git a/configure.ac b/configure.ac
> index 914c972..59619ae 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -833,6 +833,37 @@ 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)
>
> +AC_CHECK_DECL([PTHREAD_MUTEX_RECURSIVE], [HAVE_RECURSIVE_MUTEX=yes], 
> [HAVE_RECURSIVE_MUTEX=no], [[#include ]])
> +
> +THREAD_DEFAULT=no
> +
> +if test "x$HAVE_RECURSIVE_MUTEX" = "xyes" ; then
> +   THREAD_DEFAULT=yes
> +fi
> +
This will enable it on more platforms than just Linux.


> +AC_ARG_ENABLE(input-thread, AS_HELP_STRING([--enable-input-thread],
> +[Enable input threads]),
> +[INPUTTHREAD=$enableval], [INPUTTHREAD=$THREAD_DEFAULT])
> +if test "x$INPUTTHREAD" = "xyes" ; then
> +case $host_os in
> +linux*|openbsd*|gnu*|k*bsd*-gnu)
> +   THREAD_LIB="-lpthread" ;;
> +netbsd*)
> +   THREAD_CFLAGS="-D_POSIX_THREAD_SAFE_FUNCTIONS"
> +   THREAD_LIB="-lpthread" ;;
> +freebsd*)
> +   THREAD_CFLAGS="-D_THREAD_SAFE"
> +   THREAD_LIB="-pthread" ;;
> +dragonfly*)
> +   THREAD_LIB="-pthread" ;;
> +solaris*)
> +   THREAD_CFLAGS="-D_REENTRANT -D_POSIX_PTHREAD_SEMANTICS" ;;
> +esac
You can use AX_PTHREAD here, instead of open-coding it.

Then again I'm wondering if linking with pthread, won't lead to some
noticeable perf degradation, as a fair few POSIX functions will now
pthread_mutex_lock/unlock as opposed using the Glibc stub.

Not my call, just a shout out to those interested.

Regards,
Emil
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel