Re: [RFC PATCH] xs: use system's default stack size for xs_watch's reader thread

[Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>]

On Tue, Sep 20, 2016 at 05:29:39PM -0400, Chris Patterson wrote:
> From: Chris Patterson <pattersonc@ainfosec.com>
> 
> xs_watch() creates a thread to listen to xenstore events.  Currently, the
> thread is created with the greater of 16K or PTHREAD_MIN_SIZE.
> 
> There have been several bug reports and workarounds related to the issue
> where xs_watch() fails because its attempt to create the reader thread with
> pthread_create() fails. This is due to insufficient stack space size
> given the requirements for thread-local storage usage in the applications
> and libraries that are linked against libxenstore.  [1,2,3,4].
> 
> Specifying the stack size appears to have been added to reduce memory
> footprint (1d00c73b983b09fbee4d9dc0f58f6663c361c345).

Ugh. 8MB.
> 
> This has already been bumped up once to the greater of 16k and
> PTHREAD_STACK_MIN (da6a0e86d6a079102abdd0858a19f1e1fae584fc).

And that was too low (2048). CC-ing Roger.
> 
> This patch reverts to sticking with the system's default stack size and
> removes the code used to set the thread's stack size.

Which brings effectively reverts 1d00c73b983b09fbee4d9dc0f58f6663c361c345

CC-ing Simon to see what was the drive behind the memory consumption.
As in whether this was a real problem or they saw an issue with a large
stack size.

And whether (if this is XenServer product related) they could recompile
pthread library to a have smaller pthread default (instead of 8MB say something
like 16Kb).
> 
> Of course, the alternative is to bump it to another arbitrary value, but the
> requirements may change depending on the application and its libraries that
> are linking against xenstore.
> 
> [1] https://lists.nongnu.org/archive/html/qemu-devel/2016-07/msg03341.html
> [2] https://lists.xenproject.org/archives/html/xen-users/2016-07/msg00012.html
> [3] https://lists.xenproject.org/archives/html/xen-users/2016-07/msg00085.html
> [4] https://bugzilla.redhat.com/show_bug.cgi?id=1350264

Thanks for providing the pointers!
> 
> Signed-off-by: Chris Patterson <pattersonc@ainfosec.com>
> ---
>  tools/xenstore/xs.c | 10 ----------
>  1 file changed, 10 deletions(-)
> 
> diff --git a/tools/xenstore/xs.c b/tools/xenstore/xs.c
> index d1e01ba..c62b120 100644
> --- a/tools/xenstore/xs.c
> +++ b/tools/xenstore/xs.c
> @@ -723,11 +723,6 @@ bool xs_watch(struct xs_handle *h, const char *path, const char *token)
>  	struct iovec iov[2];
>  
>  #ifdef USE_PTHREAD
> -#define DEFAULT_THREAD_STACKSIZE (16 * 1024)
> -#define READ_THREAD_STACKSIZE 					\
> -	((DEFAULT_THREAD_STACKSIZE < PTHREAD_STACK_MIN) ? 	\
> -	PTHREAD_STACK_MIN : DEFAULT_THREAD_STACKSIZE)
> -
>  	/* We dynamically create a reader thread on demand. */
>  	mutex_lock(&h->request_mutex);
>  	if (!h->read_thr_exists) {
> @@ -738,11 +733,6 @@ bool xs_watch(struct xs_handle *h, const char *path, const char *token)
>  			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);
> -- 
> 2.1.4
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> https://lists.xen.org/xen-devel

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel