On Thu, Dec 19, 2013 at 11:25:33AM +0100, Stéphane Graber wrote: > On Wed, Dec 18, 2013 at 11:22:27PM -0600, Serge Hallyn wrote: > > Quoting S.Çağlar Onur ([email protected]): > > > While testing https://github.com/lxc/lxc/pull/106, I found that > > > concurrent starts > > > are hanging time to time. I then reproduced the same problem in master > > > and got following; > > > > > > [caglar@oOo:~] sudo gdb -p 16221 > > > (gdb) bt > > > #0 __lll_lock_wait () at > > > ../nptl/sysdeps/unix/sysv/linux/x86_64/lowlevellock.S:135 > > > #1 0x00007f495526515c in _L_lock_982 () from > > > /lib/x86_64-linux-gnu/libpthread.so.0 > > > #2 0x00007f4955264fab in __GI___pthread_mutex_lock > > > (mutex=0x7f49556d4600 <static_mutex>) at pthread_mutex_lock.c:64 > > > #3 0x00007f49554b27a6 in lock_mutex (l=l@entry=0x7f49556d4600 > > > <static_mutex>) at lxclock.c:78 > > > #4 0x00007f49554b2dac in static_lock () at lxclock.c:330 > > > #5 0x00007f4955498f71 in lxc_global_config_value > > > (option_name=option_name@entry=0x7f49554c02cf "cgroup.use") at utils.c:273 > > > #6 0x00007f495549926c in default_cgroup_use () at utils.c:366 > > > #7 0x00007f49554953bd in lxc_cgroup_load_meta () at cgroup.c:94 > > > #8 0x00007f495548debc in lxc_spawn > > > (handler=handler@entry=0x7f49200af300) at start.c:783 > > > #9 0x00007f495548e7a7 in __lxc_start (name=name@entry=0x7f49200b48a0 > > > "lxc-test-concurrent-4", conf=conf@entry=0x7f49200b2030, > > > ops=ops@entry=0x7f49556d3900 <start_ops>, data=data@entry=0x7f495487db90, > > > lxcpath=lxcpath@entry=0x7f49200b2010 "/var/lib/lxc") at start.c:951 > > > #10 0x00007f495548eb9c in lxc_start (name=0x7f49200b48a0 > > > "lxc-test-concurrent-4", argv=argv@entry=0x7f495487dbe0, > > > conf=conf@entry=0x7f49200b2030, lxcpath=0x7f49200b2010 "/var/lib/lxc") at > > > start.c:1048 > > > #11 0x00007f49554b68f1 in lxcapi_start (c=0x7f49200b1dd0, > > > useinit=<optimized out>, argv=0x7f495487dbe0) at lxccontainer.c:648 > > > #12 0x0000000000401317 in do_function (arguments=0x1aa80b0) at > > > concurrent.c:94 > > > #13 0x0000000000401499 in concurrent (arguments=<optimized out>) at > > > concurrent.c:130 > > > #14 0x00007f4955262f6e in start_thread (arg=0x7f495487e700) at > > > pthread_create.c:311 > > > #15 0x00007f4954f8d9cd in clone () at > > > ../sysdeps/unix/sysv/linux/x86_64/clone.S:113 > > > > > > It looks like both parent and child end up with locked mutex thus > > > deadlocks. > > > > > > I ended up placing values in the thread local storage pool, instead of > > > doing "unlock the lock in the child" dance > > > > > > Signed-off-by: S.Çağlar Onur <[email protected]> > > > > only question is, does this work in bionic? So long as it does, > > > > Acked-by: Serge E. Hallyn <[email protected]> > > I'll do a test build in a minute, if that works, I'll push to master.
Test build passed fine using the Android NDK.
I don't have hardware around me to test the result, but I think the
concerned was more about building/linking than run time.
Commit pushed to master.
>
> >
> > > ---
> > > src/lxc/lxclock.c | 13 -------------
> > > src/lxc/lxclock.h | 10 ----------
> > > src/lxc/utils.c | 16 +++-------------
> > > 3 files changed, 3 insertions(+), 36 deletions(-)
> > >
> > > diff --git a/src/lxc/lxclock.c b/src/lxc/lxclock.c
> > > index 64823d2..62e774f 100644
> > > --- a/src/lxc/lxclock.c
> > > +++ b/src/lxc/lxclock.c
> > > @@ -44,7 +44,6 @@ lxc_log_define(lxc_lock, lxc);
> > >
> > > #ifdef MUTEX_DEBUGGING
> > > pthread_mutex_t thread_mutex = PTHREAD_ERRORCHECK_MUTEX_INITIALIZER_NP;
> > > -pthread_mutex_t static_mutex = PTHREAD_ERRORCHECK_MUTEX_INITIALIZER_NP;
> > >
> > > inline void dump_stacktrace(void)
> > > {
> > > @@ -66,7 +65,6 @@ inline void dump_stacktrace(void)
> > > }
> > > #else
> > > pthread_mutex_t thread_mutex = PTHREAD_MUTEX_INITIALIZER;
> > > -pthread_mutex_t static_mutex = PTHREAD_MUTEX_INITIALIZER;
> > >
> > > inline void dump_stacktrace(void) {;}
> > > #endif
> > > @@ -324,17 +322,6 @@ void process_unlock(void)
> > > unlock_mutex(&thread_mutex);
> > > }
> > >
> > > -/* Protects static const values inside the lxc_global_config_value
> > > funtion */
> > > -void static_lock(void)
> > > -{
> > > - lock_mutex(&static_mutex);
> > > -}
> > > -
> > > -void static_unlock(void)
> > > -{
> > > - unlock_mutex(&static_mutex);
> > > -}
> > > -
> > > int container_mem_lock(struct lxc_container *c)
> > > {
> > > return lxclock(c->privlock, 0);
> > > diff --git a/src/lxc/lxclock.h b/src/lxc/lxclock.h
> > > index 820e819..a02a032 100644
> > > --- a/src/lxc/lxclock.h
> > > +++ b/src/lxc/lxclock.h
> > > @@ -123,16 +123,6 @@ extern void process_lock(void);
> > > */
> > > extern void process_unlock(void);
> > >
> > > -/*!
> > > - * \brief Lock global data.
> > > - */
> > > -extern void static_lock(void);
> > > -
> > > -/*!
> > > - * \brief Unlock global data.
> > > - */
> > > -extern void static_unlock(void);
> > > -
> > > struct lxc_container;
> > >
> > > /*!
> > > diff --git a/src/lxc/utils.c b/src/lxc/utils.c
> > > index b605307..785f3e6 100644
> > > --- a/src/lxc/utils.c
> > > +++ b/src/lxc/utils.c
> > > @@ -253,8 +253,8 @@ const char *lxc_global_config_value(const char
> > > *option_name)
> > > { "cgroup.use", NULL },
> > > { NULL, NULL },
> > > };
> > > - /* Protected by a mutex to eliminate conflicting load and store
> > > operations */
> > > - static const char *values[sizeof(options) / sizeof(options[0])] = { 0 };
> > > + /* placed in the thread local storage pool */
> > > + static __thread const char *values[sizeof(options) /
> > > sizeof(options[0])] = { 0 };
> > > const char *(*ptr)[2];
> > > const char *value;
> > > size_t i;
> > > @@ -270,13 +270,10 @@ const char *lxc_global_config_value(const char
> > > *option_name)
> > > return NULL;
> > > }
> > >
> > > - static_lock();
> > > if (values[i]) {
> > > value = values[i];
> > > - static_unlock();
> > > return value;
> > > }
> > > - static_unlock();
> > >
> > > process_lock();
> > > fin = fopen_cloexec(LXC_GLOBAL_CONF, "r");
> > > @@ -313,21 +310,17 @@ const char *lxc_global_config_value(const char
> > > *option_name)
> > > while (*p && (*p == ' ' || *p == '\t')) p++;
> > > if (!*p)
> > > continue;
> > > - static_lock();
> > > values[i] = copy_global_config_value(p);
> > > - static_unlock();
> > > goto out;
> > > }
> > > }
> > > /* could not find value, use default */
> > > - static_lock();
> > > values[i] = (*ptr)[1];
> > > /* special case: if default value is NULL,
> > > * and there is no config, don't view that
> > > * as an error... */
> > > if (!values[i])
> > > errno = 0;
> > > - static_unlock();
> > >
> > > out:
> > > process_lock();
> > > @@ -335,10 +328,7 @@ out:
> > > fclose(fin);
> > > process_unlock();
> > >
> > > - static_lock();
> > > - value = values[i];
> > > - static_unlock();
> > > - return value;
> > > + return values[i];
> > > }
> > >
> > > const char *default_lvm_vg(void)
> > > --
> > > 1.8.3.2
> > >
> > > _______________________________________________
> > > lxc-devel mailing list
> > > [email protected]
> > > http://lists.linuxcontainers.org/listinfo/lxc-devel
> > _______________________________________________
> > lxc-devel mailing list
> > [email protected]
> > http://lists.linuxcontainers.org/listinfo/lxc-devel
>
> --
> Stéphane Graber
> Ubuntu developer
> http://www.ubuntu.com
> _______________________________________________
> lxc-devel mailing list
> [email protected]
> http://lists.linuxcontainers.org/listinfo/lxc-devel
--
Stéphane Graber
Ubuntu developer
http://www.ubuntu.com
signature.asc
Description: Digital signature
_______________________________________________ lxc-devel mailing list [email protected] http://lists.linuxcontainers.org/listinfo/lxc-devel
