[lxc-devel] [PATCH] remove static_lock()/static_unlock() and start to use thread local storage (v2)
Serge Hallyn
serge.hallyn at ubuntu.com
Thu Dec 19 05:22:27 UTC 2013
Quoting S.Çağlar Onur (caglar at 10ur.org):
> 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 at 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 at 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 at 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 at entry=0x7f49200af300) at start.c:783
> #9 0x00007f495548e7a7 in __lxc_start (name=name at entry=0x7f49200b48a0 "lxc-test-concurrent-4", conf=conf at entry=0x7f49200b2030, ops=ops at entry=0x7f49556d3900 <start_ops>, data=data at entry=0x7f495487db90,
> lxcpath=lxcpath at entry=0x7f49200b2010 "/var/lib/lxc") at start.c:951
> #10 0x00007f495548eb9c in lxc_start (name=0x7f49200b48a0 "lxc-test-concurrent-4", argv=argv at entry=0x7f495487dbe0, conf=conf at 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 <caglar at 10ur.org>
only question is, does this work in bionic? So long as it does,
Acked-by: Serge E. Hallyn <serge.hallyn at ubuntu.com>
> ---
> 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
> lxc-devel at lists.linuxcontainers.org
> http://lists.linuxcontainers.org/listinfo/lxc-devel
More information about the lxc-devel
mailing list