Re: [Xen-devel] [PATCH] tools/xenstore: try to get minimum thread stack size for watch thread

2018-02-25 Thread Juergen Gross 
On 24/02/18 02:22, Jim Fehlig wrote:
> On 02/22/2018 06:53 AM, Juergen Gross wrote:
>> When creating a pthread in xs_watch() try to get the minimal needed
>> size of the thread from glibc instead of using a constant. This avoids
>> problems when the library is used in programs with large per-thread
>> memory.
>>
>> Use dlsym() to get the pointer to __pthread_get_minstack() in order to
>> avoid linkage problems and fall back to the current constant size if
>> not found.
>>
>> Signed-off-by: Juergen Gross 
>> ---
>> Only compile tested. Jim, can you please verify this patch is solving
>> your original problem?
> 
> It didn't help, but it could be due to my buggy glibc

No, it was just a silly mistake in my patch.

Sending V2 soon...


Juergen


___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH] tools/xenstore: try to get minimum thread stack size for watch thread

2018-02-23 Thread Jim Fehlig 

On 02/22/2018 06:53 AM, Juergen Gross wrote:

When creating a pthread in xs_watch() try to get the minimal needed
size of the thread from glibc instead of using a constant. This avoids
problems when the library is used in programs with large per-thread
memory.

Use dlsym() to get the pointer to __pthread_get_minstack() in order to
avoid linkage problems and fall back to the current constant size if
not found.

Signed-off-by: Juergen Gross 
---
Only compile tested. Jim, can you please verify this patch is solving
your original problem?


It didn't help, but it could be due to my buggy glibc

# gdb xl
...
(gdb) r create test-hvm.xl
Starting program: /usr/sbin/xl create test-hvm.xl
Parsing config from test-hvm.xl
Program received signal SIGSEGV, Segmentation fault.
0x772d51c2 in __pthread_get_minstack () from /lib64/libpthread.so.0
(gdb) thr a a bt

Thread 1 (Thread 0x77fd8780 (LWP 2568)):
#0  0x772d51c2 in __pthread_get_minstack () from /lib64/libpthread.so.0
#1  0x766ae259 in xs_watch (h=0x5578fc90,
path=path@entry=0x55798fa0 "/local/domain/0/device-model/2/state",
token=token@entry=0x557990b0 "3/0") at xs.c:826
#2  0x779476f4 in libxl__ev_xswatch_register 
(gc=gc@entry=0x557955f0,
w=w@entry=0x55797468, func=func@entry=0x7793dd10 
,
path=0x55798fa0 "/local/domain/0/device-model/2/state") at 
libxl_event.c:638

#3  0x7793deb0 in libxl__xswait_start (gc=gc@entry=0x557955f0,
xswa=xswa@entry=0x557973e0) at libxl_aoutils.c:53
#4  0x779326b0 in libxl__spawn_spawn (egc=egc@entry=0x7fffd950,
ss=ss@entry=0x55797370) at libxl_exec.c:292
#5  0x779258d3 in libxl__spawn_local_dm (egc=0x7fffd950, 
dmss=)

at libxl_dm.c:2400
#6  0x7791d3a7 in domcreate_launch_dm (egc=0x7fffd950, 
multidev=0x55798168,

ret=) at libxl_create.c:1379
#7  0x77967275 in libxl__bootloader_run (egc=egc@entry=0x7fffd950,
bl=bl@entry=0x55796cc0) at libxl_bootloader.c:403
#8  0x7791ffe3 in initiate_domain_create (egc=egc@entry=0x7fffd950,
dcs=dcs@entry=0x55796610) at libxl_create.c:997
#9  0x779201a1 in do_domain_create (ctx=ctx@entry=0x5578f2a0,
d_config=d_config@entry=0x7fffdb70, domid=domid@entry=0x7fffdaa8,
restore_fd=restore_fd@entry=-1, send_back_fd=send_back_fd@entry=-1, 
params=params@entry=0x0,

ao_how=0x0, aop_console_how=0x0) at libxl_create.c:1682
#10 0x779204b6 in libxl_domain_create_new (ctx=0x5578f2a0,
d_config=d_config@entry=0x7fffdb70, domid=domid@entry=0x7fffdaa8,
ao_how=ao_how@entry=0x0, aop_console_how=aop_console_how@entry=0x0) at 
libxl_create.c:1885

#11 0x555780b4 in create_domain (dom_info=dom_info@entry=0x7fffe0b0)
at xl_vmcontrol.c:902
#12 0x555790c4 in main_create (argc=1, argv=0x7fffe378) at 
xl_vmcontrol.c:1207

#13 0x55560c5b in main (argc=2, argv=0x7fffe370) at xl.c:384

If you like, I can try a patched glibc after the weekend :-).

Regards,
Jim


---
  tools/xenstore/Makefile |  4 
  tools/xenstore/xs.c | 19 ++-
  2 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/tools/xenstore/Makefile b/tools/xenstore/Makefile
index 2b99d2bc1b..fb6c73e297 100644
--- a/tools/xenstore/Makefile
+++ b/tools/xenstore/Makefile
@@ -100,6 +100,10 @@ libxenstore.so.$(MAJOR): libxenstore.so.$(MAJOR).$(MINOR)
ln -sf $< $@
  
  xs.opic: CFLAGS += -DUSE_PTHREAD

+ifeq ($(CONFIG_Linux),y)
+xs.opic: CFLAGS += -DUSE_DLSYM
+xs.opic: LDFLAGS += -ldl
+endif
  
  libxenstore.so.$(MAJOR).$(MINOR): xs.opic xs_lib.opic

$(CC) $(LDFLAGS) $(PTHREAD_LDFLAGS) -Wl,$(SONAME_LDFLAG) 
-Wl,libxenstore.so.$(MAJOR) $(SHLIB_LDFLAGS) -o $@ $^ $(LDLIBS_libxentoolcore) 
$(SOCKET_LIBS) $(PTHREAD_LIBS) $(APPEND_LDFLAGS)
diff --git a/tools/xenstore/xs.c b/tools/xenstore/xs.c
index abffd9cd80..8372f5b1a4 100644
--- a/tools/xenstore/xs.c
+++ b/tools/xenstore/xs.c
@@ -47,6 +47,11 @@ struct xs_stored_msg {
  
  #include 
  
+#ifdef USE_DLSYM

+#define __USE_GNU
+#include 
+#endif
+
  struct xs_handle {
/* Communications channel to xenstore daemon. */
int fd;
@@ -810,12 +815,24 @@ bool xs_watch(struct xs_handle *h, const char *path, 
const char *token)
if (!h->read_thr_exists) {
sigset_t set, old_set;
pthread_attr_t attr;
+   static size_t stack_size;
+#ifdef USE_DLSYM
+   size_t (*getsz)(void);
+#endif
  
+		if (!stack_size) {

+#ifdef USE_DLSYM
+   getsz = dlsym(RTLD_DEFAULT, "__pthread_get_minstack");
+   stack_size = getsz ? getsz() : READ_THREAD_STACKSIZE;
+#else
+   stack_size = READ_THREAD_STACKSIZE;
+#endif
+   }
if (pthread_attr_init() != 0) {
mutex_unlock(>request_mutex);
return false;

Re: [Xen-devel] [PATCH] tools/xenstore: try to get minimum thread stack size for watch thread

2018-02-22 Thread Juergen Gross 
On 22/02/18 19:55, Wei Liu wrote:
> On Thu, Feb 22, 2018 at 06:53:28PM +, Wei Liu wrote:
>> On Thu, Feb 22, 2018 at 07:44:22PM +0100, Juergen Gross wrote:
>>> On 22/02/18 18:28, Wei Liu wrote:
 On Thu, Feb 22, 2018 at 02:53:35PM +0100, Juergen Gross wrote:
> When creating a pthread in xs_watch() try to get the minimal needed
> size of the thread from glibc instead of using a constant. This avoids
> problems when the library is used in programs with large per-thread
> memory.
>
> Use dlsym() to get the pointer to __pthread_get_minstack() in order to
> avoid linkage problems and fall back to the current constant size if
> not found.
>
> Signed-off-by: Juergen Gross 
> ---
> Only compile tested. Jim, can you please verify this patch is solving
> your original problem?
> ---
>  tools/xenstore/Makefile |  4 
>  tools/xenstore/xs.c | 19 ++-
>  2 files changed, 22 insertions(+), 1 deletion(-)
>
> diff --git a/tools/xenstore/Makefile b/tools/xenstore/Makefile
> index 2b99d2bc1b..fb6c73e297 100644
> --- a/tools/xenstore/Makefile
> +++ b/tools/xenstore/Makefile
> @@ -100,6 +100,10 @@ libxenstore.so.$(MAJOR): 
> libxenstore.so.$(MAJOR).$(MINOR)
>   ln -sf $< $@
>  
>  xs.opic: CFLAGS += -DUSE_PTHREAD
> +ifeq ($(CONFIG_Linux),y)
> +xs.opic: CFLAGS += -DUSE_DLSYM
> +xs.opic: LDFLAGS += -ldl
> +endif
>  
>  libxenstore.so.$(MAJOR).$(MINOR): xs.opic xs_lib.opic
>   $(CC) $(LDFLAGS) $(PTHREAD_LDFLAGS) -Wl,$(SONAME_LDFLAG) 
> -Wl,libxenstore.so.$(MAJOR) $(SHLIB_LDFLAGS) -o $@ $^ 
> $(LDLIBS_libxentoolcore) $(SOCKET_LIBS) $(PTHREAD_LIBS) $(APPEND_LDFLAGS)
> diff --git a/tools/xenstore/xs.c b/tools/xenstore/xs.c
> index abffd9cd80..8372f5b1a4 100644
> --- a/tools/xenstore/xs.c
> +++ b/tools/xenstore/xs.c
> @@ -47,6 +47,11 @@ struct xs_stored_msg {
>  
>  #include 
>  
> +#ifdef USE_DLSYM
> +#define __USE_GNU

 Where does this come from? DLSYM(3) says _GNU_SOURCE (which we already
 have).
>>>
>>> On my machine build failed, so I looked into the header...
>>>
>>
>> Does putting _GNU_SOURCE at the beginning of that file before all
>> headers solve the issue? I don't think we want to use an internal
>> definition.

Aah, I put it just before including dlfcn.h and it didn't work. Putting
it at the beginning solves the problem.

I'll send V2 after waiting a bit for other comments.

> And I was wrong about "we already have _GNU_SOURCE" bit in my previous
> email. At least not in the file you modified.

Xenstore uses it already, too.


Juergen

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH] tools/xenstore: try to get minimum thread stack size for watch thread

2018-02-22 Thread Wei Liu 
On Thu, Feb 22, 2018 at 06:53:28PM +, Wei Liu wrote:
> On Thu, Feb 22, 2018 at 07:44:22PM +0100, Juergen Gross wrote:
> > On 22/02/18 18:28, Wei Liu wrote:
> > > On Thu, Feb 22, 2018 at 02:53:35PM +0100, Juergen Gross wrote:
> > >> When creating a pthread in xs_watch() try to get the minimal needed
> > >> size of the thread from glibc instead of using a constant. This avoids
> > >> problems when the library is used in programs with large per-thread
> > >> memory.
> > >>
> > >> Use dlsym() to get the pointer to __pthread_get_minstack() in order to
> > >> avoid linkage problems and fall back to the current constant size if
> > >> not found.
> > >>
> > >> Signed-off-by: Juergen Gross 
> > >> ---
> > >> Only compile tested. Jim, can you please verify this patch is solving
> > >> your original problem?
> > >> ---
> > >>  tools/xenstore/Makefile |  4 
> > >>  tools/xenstore/xs.c | 19 ++-
> > >>  2 files changed, 22 insertions(+), 1 deletion(-)
> > >>
> > >> diff --git a/tools/xenstore/Makefile b/tools/xenstore/Makefile
> > >> index 2b99d2bc1b..fb6c73e297 100644
> > >> --- a/tools/xenstore/Makefile
> > >> +++ b/tools/xenstore/Makefile
> > >> @@ -100,6 +100,10 @@ libxenstore.so.$(MAJOR): 
> > >> libxenstore.so.$(MAJOR).$(MINOR)
> > >>  ln -sf $< $@
> > >>  
> > >>  xs.opic: CFLAGS += -DUSE_PTHREAD
> > >> +ifeq ($(CONFIG_Linux),y)
> > >> +xs.opic: CFLAGS += -DUSE_DLSYM
> > >> +xs.opic: LDFLAGS += -ldl
> > >> +endif
> > >>  
> > >>  libxenstore.so.$(MAJOR).$(MINOR): xs.opic xs_lib.opic
> > >>  $(CC) $(LDFLAGS) $(PTHREAD_LDFLAGS) -Wl,$(SONAME_LDFLAG) 
> > >> -Wl,libxenstore.so.$(MAJOR) $(SHLIB_LDFLAGS) -o $@ $^ 
> > >> $(LDLIBS_libxentoolcore) $(SOCKET_LIBS) $(PTHREAD_LIBS) $(APPEND_LDFLAGS)
> > >> diff --git a/tools/xenstore/xs.c b/tools/xenstore/xs.c
> > >> index abffd9cd80..8372f5b1a4 100644
> > >> --- a/tools/xenstore/xs.c
> > >> +++ b/tools/xenstore/xs.c
> > >> @@ -47,6 +47,11 @@ struct xs_stored_msg {
> > >>  
> > >>  #include 
> > >>  
> > >> +#ifdef USE_DLSYM
> > >> +#define __USE_GNU
> > > 
> > > Where does this come from? DLSYM(3) says _GNU_SOURCE (which we already
> > > have).
> > 
> > On my machine build failed, so I looked into the header...
> > 
> 
> Does putting _GNU_SOURCE at the beginning of that file before all
> headers solve the issue? I don't think we want to use an internal
> definition.

And I was wrong about "we already have _GNU_SOURCE" bit in my previous
email. At least not in the file you modified.

Wei.

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH] tools/xenstore: try to get minimum thread stack size for watch thread

2018-02-22 Thread Wei Liu 
On Thu, Feb 22, 2018 at 07:44:22PM +0100, Juergen Gross wrote:
> On 22/02/18 18:28, Wei Liu wrote:
> > On Thu, Feb 22, 2018 at 02:53:35PM +0100, Juergen Gross wrote:
> >> When creating a pthread in xs_watch() try to get the minimal needed
> >> size of the thread from glibc instead of using a constant. This avoids
> >> problems when the library is used in programs with large per-thread
> >> memory.
> >>
> >> Use dlsym() to get the pointer to __pthread_get_minstack() in order to
> >> avoid linkage problems and fall back to the current constant size if
> >> not found.
> >>
> >> Signed-off-by: Juergen Gross 
> >> ---
> >> Only compile tested. Jim, can you please verify this patch is solving
> >> your original problem?
> >> ---
> >>  tools/xenstore/Makefile |  4 
> >>  tools/xenstore/xs.c | 19 ++-
> >>  2 files changed, 22 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/tools/xenstore/Makefile b/tools/xenstore/Makefile
> >> index 2b99d2bc1b..fb6c73e297 100644
> >> --- a/tools/xenstore/Makefile
> >> +++ b/tools/xenstore/Makefile
> >> @@ -100,6 +100,10 @@ libxenstore.so.$(MAJOR): 
> >> libxenstore.so.$(MAJOR).$(MINOR)
> >>ln -sf $< $@
> >>  
> >>  xs.opic: CFLAGS += -DUSE_PTHREAD
> >> +ifeq ($(CONFIG_Linux),y)
> >> +xs.opic: CFLAGS += -DUSE_DLSYM
> >> +xs.opic: LDFLAGS += -ldl
> >> +endif
> >>  
> >>  libxenstore.so.$(MAJOR).$(MINOR): xs.opic xs_lib.opic
> >>$(CC) $(LDFLAGS) $(PTHREAD_LDFLAGS) -Wl,$(SONAME_LDFLAG) 
> >> -Wl,libxenstore.so.$(MAJOR) $(SHLIB_LDFLAGS) -o $@ $^ 
> >> $(LDLIBS_libxentoolcore) $(SOCKET_LIBS) $(PTHREAD_LIBS) $(APPEND_LDFLAGS)
> >> diff --git a/tools/xenstore/xs.c b/tools/xenstore/xs.c
> >> index abffd9cd80..8372f5b1a4 100644
> >> --- a/tools/xenstore/xs.c
> >> +++ b/tools/xenstore/xs.c
> >> @@ -47,6 +47,11 @@ struct xs_stored_msg {
> >>  
> >>  #include 
> >>  
> >> +#ifdef USE_DLSYM
> >> +#define __USE_GNU
> > 
> > Where does this come from? DLSYM(3) says _GNU_SOURCE (which we already
> > have).
> 
> On my machine build failed, so I looked into the header...
> 

Does putting _GNU_SOURCE at the beginning of that file before all
headers solve the issue? I don't think we want to use an internal
definition.

Wei.

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH] tools/xenstore: try to get minimum thread stack size for watch thread

2018-02-22 Thread Juergen Gross 
On 22/02/18 18:28, Wei Liu wrote:
> On Thu, Feb 22, 2018 at 02:53:35PM +0100, Juergen Gross wrote:
>> When creating a pthread in xs_watch() try to get the minimal needed
>> size of the thread from glibc instead of using a constant. This avoids
>> problems when the library is used in programs with large per-thread
>> memory.
>>
>> Use dlsym() to get the pointer to __pthread_get_minstack() in order to
>> avoid linkage problems and fall back to the current constant size if
>> not found.
>>
>> Signed-off-by: Juergen Gross 
>> ---
>> Only compile tested. Jim, can you please verify this patch is solving
>> your original problem?
>> ---
>>  tools/xenstore/Makefile |  4 
>>  tools/xenstore/xs.c | 19 ++-
>>  2 files changed, 22 insertions(+), 1 deletion(-)
>>
>> diff --git a/tools/xenstore/Makefile b/tools/xenstore/Makefile
>> index 2b99d2bc1b..fb6c73e297 100644
>> --- a/tools/xenstore/Makefile
>> +++ b/tools/xenstore/Makefile
>> @@ -100,6 +100,10 @@ libxenstore.so.$(MAJOR): 
>> libxenstore.so.$(MAJOR).$(MINOR)
>>  ln -sf $< $@
>>  
>>  xs.opic: CFLAGS += -DUSE_PTHREAD
>> +ifeq ($(CONFIG_Linux),y)
>> +xs.opic: CFLAGS += -DUSE_DLSYM
>> +xs.opic: LDFLAGS += -ldl
>> +endif
>>  
>>  libxenstore.so.$(MAJOR).$(MINOR): xs.opic xs_lib.opic
>>  $(CC) $(LDFLAGS) $(PTHREAD_LDFLAGS) -Wl,$(SONAME_LDFLAG) 
>> -Wl,libxenstore.so.$(MAJOR) $(SHLIB_LDFLAGS) -o $@ $^ 
>> $(LDLIBS_libxentoolcore) $(SOCKET_LIBS) $(PTHREAD_LIBS) $(APPEND_LDFLAGS)
>> diff --git a/tools/xenstore/xs.c b/tools/xenstore/xs.c
>> index abffd9cd80..8372f5b1a4 100644
>> --- a/tools/xenstore/xs.c
>> +++ b/tools/xenstore/xs.c
>> @@ -47,6 +47,11 @@ struct xs_stored_msg {
>>  
>>  #include 
>>  
>> +#ifdef USE_DLSYM
>> +#define __USE_GNU
> 
> Where does this come from? DLSYM(3) says _GNU_SOURCE (which we already
> have).

On my machine build failed, so I looked into the header...


Juergen

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH] tools/xenstore: try to get minimum thread stack size for watch thread

2018-02-22 Thread Wei Liu 
On Thu, Feb 22, 2018 at 02:53:35PM +0100, Juergen Gross wrote:
> When creating a pthread in xs_watch() try to get the minimal needed
> size of the thread from glibc instead of using a constant. This avoids
> problems when the library is used in programs with large per-thread
> memory.
> 
> Use dlsym() to get the pointer to __pthread_get_minstack() in order to
> avoid linkage problems and fall back to the current constant size if
> not found.
> 
> Signed-off-by: Juergen Gross 
> ---
> Only compile tested. Jim, can you please verify this patch is solving
> your original problem?
> ---
>  tools/xenstore/Makefile |  4 
>  tools/xenstore/xs.c | 19 ++-
>  2 files changed, 22 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/xenstore/Makefile b/tools/xenstore/Makefile
> index 2b99d2bc1b..fb6c73e297 100644
> --- a/tools/xenstore/Makefile
> +++ b/tools/xenstore/Makefile
> @@ -100,6 +100,10 @@ libxenstore.so.$(MAJOR): libxenstore.so.$(MAJOR).$(MINOR)
>   ln -sf $< $@
>  
>  xs.opic: CFLAGS += -DUSE_PTHREAD
> +ifeq ($(CONFIG_Linux),y)
> +xs.opic: CFLAGS += -DUSE_DLSYM
> +xs.opic: LDFLAGS += -ldl
> +endif
>  
>  libxenstore.so.$(MAJOR).$(MINOR): xs.opic xs_lib.opic
>   $(CC) $(LDFLAGS) $(PTHREAD_LDFLAGS) -Wl,$(SONAME_LDFLAG) 
> -Wl,libxenstore.so.$(MAJOR) $(SHLIB_LDFLAGS) -o $@ $^ 
> $(LDLIBS_libxentoolcore) $(SOCKET_LIBS) $(PTHREAD_LIBS) $(APPEND_LDFLAGS)
> diff --git a/tools/xenstore/xs.c b/tools/xenstore/xs.c
> index abffd9cd80..8372f5b1a4 100644
> --- a/tools/xenstore/xs.c
> +++ b/tools/xenstore/xs.c
> @@ -47,6 +47,11 @@ struct xs_stored_msg {
>  
>  #include 
>  
> +#ifdef USE_DLSYM
> +#define __USE_GNU

Where does this come from? DLSYM(3) says _GNU_SOURCE (which we already
have).

Wei.

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel