* [RFC PATCH] xs: use system's default stack size for xs_watch's reader thread @ 2016-09-20 21:29 Chris Patterson 2016-09-21 12:51 ` Konrad Rzeszutek Wilk 2016-09-27 10:06 ` Wei Liu 0 siblings, 2 replies; 11+ messages in thread From: Chris Patterson @ 2016-09-20 21:29 UTC (permalink / raw) To: xen-devel; +Cc: wei.liu2, Chris Patterson, ian.jackson 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). This has already been bumped up once to the greater of 16k and PTHREAD_STACK_MIN (da6a0e86d6a079102abdd0858a19f1e1fae584fc). This patch reverts to sticking with the system's default stack size and removes the code used to set the thread's stack size. 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 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 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [RFC PATCH] xs: use system's default stack size for xs_watch's reader thread 2016-09-20 21:29 [RFC PATCH] xs: use system's default stack size for xs_watch's reader thread Chris Patterson @ 2016-09-21 12:51 ` Konrad Rzeszutek Wilk 2016-09-21 13:00 ` Wei Liu 2016-09-27 10:06 ` Wei Liu 1 sibling, 1 reply; 11+ messages in thread From: Konrad Rzeszutek Wilk @ 2016-09-21 12:51 UTC (permalink / raw) To: Chris Patterson, simon.rowe, roger.pau Cc: ian.jackson, Chris Patterson, wei.liu2, xen-devel 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFC PATCH] xs: use system's default stack size for xs_watch's reader thread 2016-09-21 12:51 ` Konrad Rzeszutek Wilk @ 2016-09-21 13:00 ` Wei Liu 2016-09-21 13:50 ` Chris Patterson 0 siblings, 1 reply; 11+ messages in thread From: Wei Liu @ 2016-09-21 13:00 UTC (permalink / raw) To: Konrad Rzeszutek Wilk Cc: Chris Patterson, wei.liu2, Chris Patterson, ian.jackson, simon.rowe, xen-devel, roger.pau On Wed, Sep 21, 2016 at 08:51:07AM -0400, Konrad Rzeszutek Wilk wrote: > 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. OOI isn't that 8MB virtual memory, which means it shouldn't have real impact unless it is used? Wei. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFC PATCH] xs: use system's default stack size for xs_watch's reader thread 2016-09-21 13:00 ` Wei Liu @ 2016-09-21 13:50 ` Chris Patterson 2016-09-21 14:07 ` Konrad Rzeszutek Wilk 0 siblings, 1 reply; 11+ messages in thread From: Chris Patterson @ 2016-09-21 13:50 UTC (permalink / raw) To: Wei Liu; +Cc: Chris Patterson, Ian Jackson, simon.rowe, xen-devel, roger.pau On Wed, Sep 21, 2016 at 9:00 AM, Wei Liu <wei.liu2@citrix.com> wrote: > On Wed, Sep 21, 2016 at 08:51:07AM -0400, Konrad Rzeszutek Wilk wrote: >> 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. > > OOI isn't that 8MB virtual memory, which means it shouldn't have real > impact unless it is used? > From what I understand, that is correct. At least in the Linux/glibc case, I believe the stack is allocated using anonymous mmap() and that resident memory usage shouldn't be greater than what you actually end up writing. However, I do not know if this holds true universally... _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFC PATCH] xs: use system's default stack size for xs_watch's reader thread 2016-09-21 13:50 ` Chris Patterson @ 2016-09-21 14:07 ` Konrad Rzeszutek Wilk 2016-09-21 14:23 ` Chris Patterson 2016-09-26 16:53 ` Roger Pau Monné 0 siblings, 2 replies; 11+ messages in thread From: Konrad Rzeszutek Wilk @ 2016-09-21 14:07 UTC (permalink / raw) To: Chris Patterson Cc: Chris Patterson, Wei Liu, Ian Jackson, simon.rowe, xen-devel, roger.pau On Wed, Sep 21, 2016 at 09:50:30AM -0400, Chris Patterson wrote: > On Wed, Sep 21, 2016 at 9:00 AM, Wei Liu <wei.liu2@citrix.com> wrote: > > On Wed, Sep 21, 2016 at 08:51:07AM -0400, Konrad Rzeszutek Wilk wrote: > >> 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. > > > > OOI isn't that 8MB virtual memory, which means it shouldn't have real > > impact unless it is used? /me nods. That is my recollection too. But it does mean that 'top' shows the application as bigger (by 8MB). > > > > >From what I understand, that is correct. At least in the Linux/glibc > case, I believe the stack is allocated using anonymous mmap() and that > resident memory usage shouldn't be greater than what you actually end > up writing. However, I do not know if this holds true universally... That should be faily easy to find out. One just needs to check the RSS size. Not sure how to do that on FreeBSD thought.. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFC PATCH] xs: use system's default stack size for xs_watch's reader thread 2016-09-21 14:07 ` Konrad Rzeszutek Wilk @ 2016-09-21 14:23 ` Chris Patterson 2016-09-21 14:39 ` Konrad Rzeszutek Wilk 2016-09-26 16:53 ` Roger Pau Monné 1 sibling, 1 reply; 11+ messages in thread From: Chris Patterson @ 2016-09-21 14:23 UTC (permalink / raw) To: Konrad Rzeszutek Wilk Cc: Chris Patterson, Wei Liu, Ian Jackson, simon.rowe, xen-devel, roger.pau On Wed, Sep 21, 2016 at 10:07 AM, Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote: > On Wed, Sep 21, 2016 at 09:50:30AM -0400, Chris Patterson wrote: >> On Wed, Sep 21, 2016 at 9:00 AM, Wei Liu <wei.liu2@citrix.com> wrote: >> > On Wed, Sep 21, 2016 at 08:51:07AM -0400, Konrad Rzeszutek Wilk wrote: >> >> 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. >> > >> > OOI isn't that 8MB virtual memory, which means it shouldn't have real >> > impact unless it is used? > > /me nods. That is my recollection too. But it does mean that 'top' > shows the application as bigger (by 8MB). > >> > >> >> >From what I understand, that is correct. At least in the Linux/glibc >> case, I believe the stack is allocated using anonymous mmap() and that >> resident memory usage shouldn't be greater than what you actually end >> up writing. However, I do not know if this holds true universally... > > That should be faily easy to find out. One just needs to check > the RSS size. Not sure how to do that on FreeBSD thought.. I did validate that yesterday (using pmap -x <pid>) and it appeared to hold true (for that case anyways). If using the default stack size (as this patch reverts to), the stack size can be controlled with ulimit -s <num_kb> _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFC PATCH] xs: use system's default stack size for xs_watch's reader thread 2016-09-21 14:23 ` Chris Patterson @ 2016-09-21 14:39 ` Konrad Rzeszutek Wilk 0 siblings, 0 replies; 11+ messages in thread From: Konrad Rzeszutek Wilk @ 2016-09-21 14:39 UTC (permalink / raw) To: Chris Patterson Cc: Chris Patterson, Wei Liu, Ian Jackson, simon.rowe, xen-devel, roger.pau On Wed, Sep 21, 2016 at 10:23:09AM -0400, Chris Patterson wrote: > On Wed, Sep 21, 2016 at 10:07 AM, Konrad Rzeszutek Wilk > <konrad.wilk@oracle.com> wrote: > > On Wed, Sep 21, 2016 at 09:50:30AM -0400, Chris Patterson wrote: > >> On Wed, Sep 21, 2016 at 9:00 AM, Wei Liu <wei.liu2@citrix.com> wrote: > >> > On Wed, Sep 21, 2016 at 08:51:07AM -0400, Konrad Rzeszutek Wilk wrote: > >> >> 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. > >> > > >> > OOI isn't that 8MB virtual memory, which means it shouldn't have real > >> > impact unless it is used? > > > > /me nods. That is my recollection too. But it does mean that 'top' > > shows the application as bigger (by 8MB). > > > >> > > >> > >> >From what I understand, that is correct. At least in the Linux/glibc > >> case, I believe the stack is allocated using anonymous mmap() and that > >> resident memory usage shouldn't be greater than what you actually end > >> up writing. However, I do not know if this holds true universally... > > > > That should be faily easy to find out. One just needs to check > > the RSS size. Not sure how to do that on FreeBSD thought.. > > I did validate that yesterday (using pmap -x <pid>) and it appeared to > hold true (for that case anyways). Thank you for checking that. > > If using the default stack size (as this patch reverts to), the stack > size can be controlled with ulimit -s <num_kb> That should be part of the patch description and perhaps even in libxs.h header. Lets wait for Simon to chime in. It may be that he had some other reasons to limit the stack size that weren't mentioned in the commit. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFC PATCH] xs: use system's default stack size for xs_watch's reader thread 2016-09-21 14:07 ` Konrad Rzeszutek Wilk 2016-09-21 14:23 ` Chris Patterson @ 2016-09-26 16:53 ` Roger Pau Monné 1 sibling, 0 replies; 11+ messages in thread From: Roger Pau Monné @ 2016-09-26 16:53 UTC (permalink / raw) To: Konrad Rzeszutek Wilk Cc: Chris Patterson, Wei Liu, Chris Patterson, Ian Jackson, simon.rowe, xen-devel On Wed, Sep 21, 2016 at 10:07:10AM -0400, Konrad Rzeszutek Wilk wrote: > On Wed, Sep 21, 2016 at 09:50:30AM -0400, Chris Patterson wrote: > > On Wed, Sep 21, 2016 at 9:00 AM, Wei Liu <wei.liu2@citrix.com> wrote: > > > On Wed, Sep 21, 2016 at 08:51:07AM -0400, Konrad Rzeszutek Wilk wrote: > > >> 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. > > > > > > OOI isn't that 8MB virtual memory, which means it shouldn't have real > > > impact unless it is used? > > /me nods. That is my recollection too. But it does mean that 'top' > shows the application as bigger (by 8MB). > > > > > > > > >From what I understand, that is correct. At least in the Linux/glibc > > case, I believe the stack is allocated using anonymous mmap() and that > > resident memory usage shouldn't be greater than what you actually end > > up writing. However, I do not know if this holds true universally... > > That should be faily easy to find out. One just needs to check > the RSS size. Not sure how to do that on FreeBSD thought.. In any case, I don't think we should be setting the pthread stack size at all, we don't really know how much stack libc functions will try to use, so setting this to any value is likely wrong IMHO. Roger. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFC PATCH] xs: use system's default stack size for xs_watch's reader thread 2016-09-20 21:29 [RFC PATCH] xs: use system's default stack size for xs_watch's reader thread Chris Patterson 2016-09-21 12:51 ` Konrad Rzeszutek Wilk @ 2016-09-27 10:06 ` Wei Liu 2016-09-27 10:56 ` Simon Rowe 2016-09-27 11:01 ` Ian Jackson 1 sibling, 2 replies; 11+ messages in thread From: Wei Liu @ 2016-09-27 10:06 UTC (permalink / raw) To: Chris Patterson; +Cc: wei.liu2, Chris Patterson, ian.jackson, xen-devel 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). > > This has already been bumped up once to the greater of 16k and > PTHREAD_STACK_MIN (da6a0e86d6a079102abdd0858a19f1e1fae584fc). > > This patch reverts to sticking with the system's default stack size and > removes the code used to set the thread's stack size. > > 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 > > Signed-off-by: Chris Patterson <pattersonc@ainfosec.com> I'm tempted to just ack and apply this patch. If I hear no objection by Friday I will do so. > --- > 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFC PATCH] xs: use system's default stack size for xs_watch's reader thread 2016-09-27 10:06 ` Wei Liu @ 2016-09-27 10:56 ` Simon Rowe 2016-09-27 11:01 ` Ian Jackson 1 sibling, 0 replies; 11+ messages in thread From: Simon Rowe @ 2016-09-27 10:56 UTC (permalink / raw) To: xen-devel, wei.liu2 On 27/09/16 11:06, Wei Liu wrote: > 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). >> > >> > This has already been bumped up once to the greater of 16k and >> > PTHREAD_STACK_MIN (da6a0e86d6a079102abdd0858a19f1e1fae584fc). >> > >> > This patch reverts to sticking with the system's default stack size and >> > removes the code used to set the thread's stack size. >> > >> > 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 >> > >> > Signed-off-by: Chris Patterson <pattersonc@ainfosec.com> > I'm tempted to just ack and apply this patch. If I hear no objection by > Friday I will do so. I think the reason we added this patch has gone away, Simon _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFC PATCH] xs: use system's default stack size for xs_watch's reader thread 2016-09-27 10:06 ` Wei Liu 2016-09-27 10:56 ` Simon Rowe @ 2016-09-27 11:01 ` Ian Jackson 1 sibling, 0 replies; 11+ messages in thread From: Ian Jackson @ 2016-09-27 11:01 UTC (permalink / raw) To: Wei Liu; +Cc: Chris Patterson, Chris Patterson, xen-devel Wei Liu writes ("Re: [RFC PATCH] xs: use system's default stack size for xs_watch's reader thread"): > On Tue, Sep 20, 2016 at 05:29:39PM -0400, Chris Patterson wrote: > > 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. Personally I think the libraries and applications that are putting big structures into TLS are mad. But I guess we have to live in the world of madness. > > [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 > > > > Signed-off-by: Chris Patterson <pattersonc@ainfosec.com> > > I'm tempted to just ack and apply this patch. If I hear no objection by > Friday I will do so. Can we please at least retain the ability for an application that wants to make lots of threads, to set the stack size used by libxenstore ? I guess this would have to be a global function, callable by anyone (even bypassing libxl). Ian. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2016-09-27 11:01 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-09-20 21:29 [RFC PATCH] xs: use system's default stack size for xs_watch's reader thread Chris Patterson 2016-09-21 12:51 ` Konrad Rzeszutek Wilk 2016-09-21 13:00 ` Wei Liu 2016-09-21 13:50 ` Chris Patterson 2016-09-21 14:07 ` Konrad Rzeszutek Wilk 2016-09-21 14:23 ` Chris Patterson 2016-09-21 14:39 ` Konrad Rzeszutek Wilk 2016-09-26 16:53 ` Roger Pau Monné 2016-09-27 10:06 ` Wei Liu 2016-09-27 10:56 ` Simon Rowe 2016-09-27 11:01 ` Ian Jackson
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).