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

[Alexandru Ardelean]

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.