Cygwin and upstream (Was: Re: [PATCH xserver v2.1] os: Treat ssh as a non-local client (v2))

[Emil Velikov]

On 11 December 2015 at 15:47, Jon Turney  wrote:
> On 11/12/2015 10:31, Emil Velikov wrote:

>> There's no (intentional) MSVC or mingw support in xserver that I know
>> of - only Cygwin. There is also the GNU version of the function
>> (#define _GNU_SOURCE + #include ) although I'm not sure if
>> Cygwin has either of them. I'd assume Jon can follow up as needed.
>
>
> No, there should be support in the X server for building it for MinGW.
> doc/c-extension touches upon this subject briefly.
>
Must admit that I wasn't expecting to see a note in there. To make it
even more embarrassing, I've even briefly used Xming some 7+years ago,
yet I somehow thought it was dead.

> I suspect it may be broken in master due to recent changes, but I do try to
> upstream patches from Xming to keep it building for MinGW, even though my
> primary interest is building xserver for Cygwin.
>
> (There also exists VcXsrv, which uses a much larger set of patches and some
> special tools to build the X.Org Xserver with MSVC)
>
Thanks for this one Jon. I haven't heard of VcXsrv before.

I've just reached towards both projects hoping for the best, and it's
time for me to do the same with Cygwin :-)

How is Cygwin + X (the graphics stack in general) going ? Do you have
some patches that you can thrown upstream :-P Or, it's just the
windowsdri work that lives out-of-tree ?

Thanks
Emil
___
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

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

[Mark Kettenis]

> From: Keith Packard 
> Date: Fri, 11 Dec 2015 15:37:24 -0800
> 
> Keith Packard  writes:
> 
> > Mark Kettenis  writes:
> >
> >> However, is there a reason why you didn't use the
> >> PTHREAD_MUTEX_RECURSIVE mtex type that is standardized by POSIX?
> >
> > Sorry, my documentation only mentions a non-portable version of
> > recursive mutexes. I didn't even know that recursive mutexes had been
> > standardized. Looks like we've got options.
> 
> So, how about this preference plan, picking the first one which works:
> 
>  1) PTHREAD_MUTEX_RECURSIVE
>  2) PTHREAD_MUTEX_RECURSIVE_NP
>  3) PTHREAD_MUTEX_NORMAL + __thread variable
>  4) PTHREAD_MUTEX_NORMAL + thread specific variables

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.

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.

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

Re: [PATCH xserver] os: Treat ssh as a non-local client (v2)

[Mark Kettenis]

> From: =?UTF-8?q?Michel=20D=C3=A4nzer?= 
> Date: Fri, 11 Dec 2015 11:28:51 +0900
> 
> By the time we get to ComputeLocalClient, we've already done
> NextAvailableClient → ReserveClientIds →
> DetermineClientCmd (assuming we're built with #define CLIENTIDS), so
> we can look up the name of the client process and refuse to treat
> ssh's X forwarding as if it were local.
> 
> v2: (Michel Dänzer)
> * Only match "ssh" itself, not other executable names starting with
>   that prefix.
> * Ignore executable path for the match.
> 
> Signed-off-by: Adam Jackson 
> Signed-off-by: Michel Dänzer 
> ---
>  os/access.c | 27 ---
>  1 file changed, 24 insertions(+), 3 deletions(-)
> 
> diff --git a/os/access.c b/os/access.c
> index 10a48c3..6ff6977 100644
> --- a/os/access.c
> +++ b/os/access.c
> @@ -101,6 +101,7 @@ SOFTWARE.
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #ifndef NO_LOCAL_CLIENT_CRED
>  #include 
> @@ -1081,9 +1082,8 @@ ResetHosts(const char *display)
>  }
>  }
>  
> -/* Is client on the local host */
> -Bool
> -ComputeLocalClient(ClientPtr client)
> +static Bool
> +xtransLocalClient(ClientPtr client)
>  {
>  int alen, family, notused;
>  Xtransaddr *from = NULL;
> @@ -1116,6 +1116,27 @@ ComputeLocalClient(ClientPtr client)
>  return FALSE;
>  }
>  
> +/* Is client on the local host */
> +Bool
> +ComputeLocalClient(ClientPtr client)
> +{
> +if (!xtransLocalClient(client))
> +return FALSE;
> +
> +#ifndef WIN32
> +/* If the executable name is "ssh", consider this client not local */
> +if (client->clientIds->cmdname) {

Isn't it better to use GetClientCmdName() here to keep avoid a
potential null-pointer dereference?  Actually include/client.h tells
you you shuld ;).
___
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

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

[Keith Packard]

Mark Kettenis  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 

#define LOOPS		1000
#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();
}

static inline void unlock() {
	pthread_mutex_unlock();
}

#else

static pthread_mutex_t	mutex = PTHREAD_MUTEX_INITIALIZER;
static __thread int count;

static inline void lock() {
	if (count++ == 0)
		pthread_mutex_lock();
}

static inline void unlock() {
	if (--count == 0)
		pthread_mutex_unlock();
}
#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([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 
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 

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 ]])
+
+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])
 	

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

[Keith Packard]

Mark Kettenis  writes:

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

Sigh. That's a pain in this case; the first use of the mutex occurs
before the DDX initializes the input thread. I'll use
PTHREAD_RECURSIVE_MUTEX_INITIALIZER_NP where it exists, and fallback to
a test inside input_lock() that initializes the mutex on first
use. That's "safe" because the first use will always happen before the
input thread is started.

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

I was surprised at how bad the Linux recursive mutexes performed
compared with a thread-local counter, if you actually used recursion. In
the (we hope common) case where the mutex gets acquired only once, they
are the same.

Depth   Recursive   Counter
10  30.3ns  13.2ns 
1   100.4ns 100.4ns

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

That's unfortunate, but easy enough to work around with a bit of
conditional code.

Here's code which works only with recursive mutexes, initializing it
explicitly when PTHREAD_RECURSIVE_MUTEX_INITIALIZER_NP is not available.

From 700fb83a0afbfce19bb91967f619cde6ead900a6 Mon Sep 17 00:00:00 2001
From: Keith Packard 
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 

fixup for thread variables
---
 configure.ac| 14 ++
 hw/xfree86/sdksyms.sh   |  4 ---
 include/dix-config.h.in |  3 +++
 include/input.h | 23 +++-
 os/inputthread.c| 72 +
 5 files changed, 87 insertions(+), 29 deletions(-)

diff --git a/configure.ac b/configure.ac
index bc15581..abea281 100644
--- a/configure.ac
+++ b/configure.ac
@@ -821,13 +821,16 @@ 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_MUTEX_RECURSIVE], [HAVE_RECURSIVE_MUTEX=yes], [HAVE_RECURSIVE_MUTEX=no], [[#include ]])
+
+THREAD_DEFAULT=no
+
+case x$HAVE_RECURSIVE_MUTEX in
+xyes)
+	AC_DEFINE(HAVE_PTHREAD_MUTEX_RECURSIVE, 1, [Have recursive mutexes])
 	THREAD_DEFAULT=yes ;;
-*)
-	THREAD_DEFAULT=no ;;
 esac
+
 AC_ARG_ENABLE(input-thread, AS_HELP_STRING([--enable-input-thread],
 	 [Enable input threads]),
 	 [INPUTTHREAD=$enableval], [INPUTTHREAD=$THREAD_DEFAULT])
@@ -848,6 +851,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..0773b6d 100644
--- a/include/dix-config.h.in
+++ b/include/dix-config.h.in
@@ -521,4 +521,7 @@
 /* Have setitimer support */
 #undef HAVE_SETITIMER
 
+/* 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, 

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

[Mark Kettenis]

> From: Keith Packard 
> Date: Sat, 12 Dec 2015 13:19:59 -0800
> 
> Mark Kettenis  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 
> 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 
> 
> 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
___
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