[PATCH 0 of 2 V3] xs: fixes to watch thread creation

[PATCH 0 of 2 V3] xs: fixes to watch thread creation

[Simon Rowe]

A pair of patches to the xenstore library that:

  1) blocks signal delivery before creating the watch wakeup thread

  2) reduces the stack size of the watch wakeup thread.

[PATCH 1 of 2 V3] xs: block signals in watch wakeup thread

[Simon Rowe]

The thread created to wakeup watchers is not intended to handle signals
(and a later patch will reduce it's stack size which makes it unsuitable
for doing so).

Signed-off-by: Simon Rowe <simon.rowe@eu.citrix.com>

diff -r 6bea63e6c780 -r 711a707dc776 tools/xenstore/xs.c
--- a/tools/xenstore/xs.c	Sat Jun 02 08:39:45 2012 +0100
+++ b/tools/xenstore/xs.c	Wed Jun 06 10:04:15 2012 +0100
@@ -705,11 +705,18 @@ bool xs_watch(struct xs_handle *h, const
 	/* We dynamically create a reader thread on demand. */
 	mutex_lock(&h->request_mutex);
 	if (!h->read_thr_exists) {
+		sigset_t set, old_set;
+
+		sigfillset(&set);
+		pthread_sigmask(SIG_SETMASK, &set, &old_set);
+
 		if (pthread_create(&h->read_thr, NULL, read_thread, h) != 0) {
+			pthread_sigmask(SIG_SETMASK, &old_set, NULL);
 			mutex_unlock(&h->request_mutex);
 			return false;
 		}
 		h->read_thr_exists = 1;
+		pthread_sigmask(SIG_SETMASK, &old_set, NULL);
 	}
 	mutex_unlock(&h->request_mutex);
 #endif

[PATCH 2 of 2 V3] xs: set read_thread stacksize

[Simon Rowe]

xs_watch() creates a thread to wake watchers using default attributes. The
stacksize can be quite large (8 MB on Linux), applications that link against
xenstore end up having a larger memory footprint than necessary.

Signed-off-by: Simon Rowe <simon.rowe@eu.citrix.com>

---
Changed since v1:
  * remove test for _POSIX_THREAD_ATTR_STACKSIZE
  * use define for constant

diff -r 711a707dc776 -r ae3a05009962 tools/xenstore/xs.c
--- a/tools/xenstore/xs.c	Wed Jun 06 10:04:15 2012 +0100
+++ b/tools/xenstore/xs.c	Wed Jun 06 10:06:44 2012 +0100
@@ -702,21 +702,36 @@ bool xs_watch(struct xs_handle *h, const
 	struct iovec iov[2];
 
 #ifdef USE_PTHREAD
+#define READ_THREAD_STACKSIZE (16 * 1024)
+
 	/* We dynamically create a reader thread on demand. */
 	mutex_lock(&h->request_mutex);
 	if (!h->read_thr_exists) {
 		sigset_t set, old_set;
+		pthread_attr_t attr;
+
+		if (pthread_attr_init(&attr) != 0) {
+			mutex_unlock(&h->request_mutex);
+			return false;
+		}
+		if (pthread_attr_setstacksize(&attr, READ_THREAD_STACKSIZE) != 0) {
+			pthread_attr_destroy(&attr);
+			mutex_unlock(&h->request_mutex);
+			return false;
+		}
 
 		sigfillset(&set);
 		pthread_sigmask(SIG_SETMASK, &set, &old_set);
 
-		if (pthread_create(&h->read_thr, NULL, read_thread, h) != 0) {
+		if (pthread_create(&h->read_thr, &attr, read_thread, h) != 0) {
 			pthread_sigmask(SIG_SETMASK, &old_set, NULL);
+			pthread_attr_destroy(&attr);
 			mutex_unlock(&h->request_mutex);
 			return false;
 		}
 		h->read_thr_exists = 1;
 		pthread_sigmask(SIG_SETMASK, &old_set, NULL);
+		pthread_attr_destroy(&attr);
 	}
 	mutex_unlock(&h->request_mutex);
 #endif

Re: [PATCH 0 of 2 V3] xs: fixes to watch thread creation

[Ian Jackson]

Simon Rowe writes ("[Xen-devel] [PATCH 0 of 2 V3] xs: fixes to watch thread creation"):
> A pair of patches to the xenstore library that:
> 
>   1) blocks signal delivery before creating the watch wakeup thread
> 
>   2) reduces the stack size of the watch wakeup thread.

Thanks, both:

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
Committed-by: Ian Jackson <ian.jackson@eu.citrix.com>

Ian.