Xen devel - Jun 2012 - [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.

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

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

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.