[ovs-dev] [PATCH] [RFC] lib/ovs-thread: set pthread stack size to 128k

[Ben Pfaff]

On Mon, Jan 25, 2016 at 03:16:22PM +0200, Alexandru Ardelean wrote:
> On Mon, Jan 25, 2016 at 10:13 AM, Alexandru Ardelean <ardeleanalex at gmail.com
> > wrote:
> 
> >
> >
> > On Sun, Jan 24, 2016 at 11:24 PM, Russell Bryant <russell at ovn.org> wrote:
> >
> >> On 01/24/2016 08:56 AM, Alexandru Ardelean wrote:
> >> > Seems that musl libc's default thread stack size is 80k, which
> >> > causes a segfault (stack overflow actually) when trying to add
> >> > a bridge (via the "ovs-vsctl add-br" command).
> >> >
> >> > OpenWRT has been switching to musl libc for a few months now.
> >> > So far, we've been using OVS with uClibc, so I did not catch this
> >> > earlier.
> >> >
> >> > This patch is a RFC:
> >> > - is this thread stack size sufficient ?
> >> > - is this approach acceptable ?
> >> >
> >> > In OpenWRT we'll use this patch since it works.
> >> >
> >> > Issue was found and fixed by Robert McKay.
> >> > I fine tuned his patch and will add it to our OVS package
> >> > in OpenWRT.
> >> >
> >> > Signed-off-by: Robert McKay <robert at mckay.com>
> >> > Signed-off-by: Alexandru Ardelean <ardeleanalex at gmail.com>
> >> > ---
> >> >  lib/ovs-thread.c |    5 ++++-
> >> >  1 file changed, 4 insertions(+), 1 deletion(-)
> >> >
> >> > diff --git a/lib/ovs-thread.c b/lib/ovs-thread.c
> >> > index 7855b3a..e35ddba 100644
> >> > --- a/lib/ovs-thread.c
> >> > +++ b/lib/ovs-thread.c
> >> > @@ -347,6 +347,7 @@ ovs_thread_create(const char *name, void
> >> *(*start)(void *), void *arg)
> >> >  {
> >> >      struct ovsthread_aux *aux;
> >> >      pthread_t thread;
> >> > +    pthread_attr_t attr;
> >> >      int error;
> >> >
> >> >      forbid_forking("multiple threads exist");
> >> > @@ -358,7 +359,9 @@ ovs_thread_create(const char *name, void
> >> *(*start)(void *), void *arg)
> >> >      aux->arg = arg;
> >> >      ovs_strlcpy(aux->name, name, sizeof aux->name);
> >> >
> >> > -    error = pthread_create(&thread, NULL, ovsthread_wrapper, aux);
> >> > +    pthread_attr_init(&attr);
> >> > +    pthread_attr_setstacksize(&attr, 128*1024);
> >> > +    error = pthread_create(&thread, &attr, ovsthread_wrapper, aux);
> >> >      if (error) {
> >> >          ovs_abort(error, "pthread_create failed");
> >> >      }
> >> >
> >>
> >> I don't actually have feedback on the more important question of how
> >> best to deal with your issue, but fwiw, you need a
> >> pthread_attr_destroy() after pthread_create().
> >>
> >> --
> >> Russell Bryant
> >>
> >
> > Thanks for that.
> > Re-spinned patch with your suggestion.
> >
> >
> I've found that 128k is too small under some tests.
> Will try to increase this in increments of 128k (so next one is 256k) and
> run a few more tests.
> RFC for "is this approach acceptable ?" still stands.

My reading of v2 is that it still uses 128 kB.