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

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Fri, Mar 02, 2018 at 10:23:08AM -0700, Jim Fehlig wrote:
> On 03/02/2018 05:40 AM, Wei Liu wrote:
> > On Fri, Mar 02, 2018 at 12:29:31PM +0000, Wei Liu wrote:
> > > On Mon, Feb 26, 2018 at 09:53:38AM -0700, Jim Fehlig wrote:
> > > > On 02/26/2018 01:46 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 <jgross@xxxxxxxx>
> > > > > ---
> > > > > V2:
> > > > > - use _GNU_SOURCE (Wei Liu)
> > > > > - call __pthread_get_minstack() with parameter
> > > > > - add -ldl to correct make flags
> > > > > - ensure to not using smaller stack size than today
> > > > > ---
> > > > >    tools/xenstore/Makefile |  4 ++++
> > > > >    tools/xenstore/xs.c     | 21 ++++++++++++++++++++-
> > > > >    2 files changed, 24 insertions(+), 1 deletion(-)
> > > > > 
> > > > > diff --git a/tools/xenstore/Makefile b/tools/xenstore/Makefile
> > > > > index 2b99d2bc1b..0831be0b6f 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
> > > > > +libxenstore.so.$(MAJOR).$(MINOR): LDFLAGS += -ldl
> > > > > +endif
> > > > 
> > > > Dropping this patch in one of my automated builds caused a libxenstore 
> > > > link failure
> > > > 
> > > > [   99s] gcc    -lsystemd -ldl -pthread -Wl,-soname 
> > > > -Wl,libxenstore.so.3.0
> > > > -shared -o libxenstore.so.3.0.3 xs.opic xs_lib.opic 
> > > > /home/abuild/rpmbuild/BUILD/xen-4.10.0-testing/tools/xenstore/../../tools/libs/toolcore/libxentoolcore.so
> > > > 
> > > > [   99s] 
> > > > /home/abuild/rpmbuild/BUILD/xen-4.10.0-testing/tools/xenstore/../../tools/xenstore/libxenstore.so:
> > > > undefined reference to `dlsym'
> > > > 
> > > > I hacked around it by appending '-ldl' to the end of the subsequent
> > > > libxenstore.so rule.
> > > 
> > > Hmm... Maybe I'm a bit dense today. I know the position of -l matters
> > > but I don't quite understand how placing -pthread before xs.opic works
> > > but -ldl doesn't. xs.c uses both after all.
> > 
> > I'm indeed very dense -- -pthread is a special option that sets the
> > proper flags for linking pthread library for both the preprocessor and
> > linker.
> > 
> > But still, Juergen must have tested the change, so I wonder why it
> > doesn't work in your setup. What is your build environment? Gcc version?
> 
> I dropped the patch in a package build on the openSUSE build service, where
> gcc7 was used. But I don't see the problem when building from sources with
> gcc7. Apparently we have a bug in our package build, so ignore this comment.
> Tested-by still stands though :-).
> 

OK, thanks for the reply.

I will commit this patch soon.

Wei.

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