Skip to content

L
lxc
Commit 95b422fc by S.Çağlar Onur Committed by Stéphane Graber
Options

remove static_lock()/static_unlock() and start to use thread local storage (v2)

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 avatarS.Çağlar Onur <caglar@10ur.org> Acked-by: 's avatarSerge E. Hallyn <serge.hallyn@ubuntu.com>
parent f4d5cc8e
Showing with 3 additions and 36 deletions
src/lxc/lxclock.c
...@@ -44,7 +44,6 @@ lxc_log_define(lxc_lock, lxc); ...@@ -44,7 +44,6 @@ lxc_log_define(lxc_lock, lxc);
#ifdef MUTEX_DEBUGGING #ifdef MUTEX_DEBUGGING
pthread_mutex_t thread_mutex = PTHREAD_ERRORCHECK_MUTEX_INITIALIZER_NP; 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) inline void dump_stacktrace(void)
{ {
...@@ -66,7 +65,6 @@ inline void dump_stacktrace(void) ...@@ -66,7 +65,6 @@ inline void dump_stacktrace(void)
} }
#else #else
pthread_mutex_t thread_mutex = PTHREAD_MUTEX_INITIALIZER; pthread_mutex_t thread_mutex = PTHREAD_MUTEX_INITIALIZER;
pthread_mutex_t static_mutex = PTHREAD_MUTEX_INITIALIZER;
inline void dump_stacktrace(void) {;} inline void dump_stacktrace(void) {;}
#endif #endif
...@@ -324,17 +322,6 @@ void process_unlock(void) ...@@ -324,17 +322,6 @@ void process_unlock(void)
unlock_mutex(&thread_mutex); 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) int container_mem_lock(struct lxc_container *c)
{ {
return lxclock(c->privlock, 0); return lxclock(c->privlock, 0);
......
src/lxc/lxclock.h
...@@ -123,16 +123,6 @@ extern void process_lock(void); ...@@ -123,16 +123,6 @@ extern void process_lock(void);
*/ */
extern void process_unlock(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; struct lxc_container;
/*! /*!
......
src/lxc/utils.c
...@@ -253,8 +253,8 @@ const char *lxc_global_config_value(const char *option_name) ...@@ -253,8 +253,8 @@ const char *lxc_global_config_value(const char *option_name)
{ "cgroup.use", NULL }, { "cgroup.use", NULL },
{ NULL, NULL }, { NULL, NULL },
}; };
/* Protected by a mutex to eliminate conflicting load and store operations */ /* placed in the thread local storage pool */
static const char *values[sizeof(options) / sizeof(options[0])] = { 0 }; static __thread const char *values[sizeof(options) / sizeof(options[0])] = { 0 };
const char *(*ptr)[2]; const char *(*ptr)[2];
const char *value; const char *value;
size_t i; size_t i;
...@@ -270,13 +270,10 @@ const char *lxc_global_config_value(const char *option_name) ...@@ -270,13 +270,10 @@ const char *lxc_global_config_value(const char *option_name)
return NULL; return NULL;
} }
static_lock();
if (values[i]) { if (values[i]) {
value = values[i]; value = values[i];
static_unlock();
return value; return value;
} }
static_unlock();
process_lock(); process_lock();
fin = fopen_cloexec(LXC_GLOBAL_CONF, "r"); fin = fopen_cloexec(LXC_GLOBAL_CONF, "r");
...@@ -313,21 +310,17 @@ const char *lxc_global_config_value(const char *option_name) ...@@ -313,21 +310,17 @@ const char *lxc_global_config_value(const char *option_name)
while (*p && (*p == ' ' || *p == '\t')) p++; while (*p && (*p == ' ' || *p == '\t')) p++;
if (!*p) if (!*p)
continue; continue;
static_lock();
values[i] = copy_global_config_value(p); values[i] = copy_global_config_value(p);
static_unlock();
goto out; goto out;
} }
} }
/* could not find value, use default */ /* could not find value, use default */
static_lock();
values[i] = (*ptr)[1]; values[i] = (*ptr)[1];
/* special case: if default value is NULL, /* special case: if default value is NULL,
* and there is no config, don't view that * and there is no config, don't view that
* as an error... */ * as an error... */
if (!values[i]) if (!values[i])
errno = 0; errno = 0;
static_unlock();
out: out:
process_lock(); process_lock();
...@@ -335,10 +328,7 @@ out: ...@@ -335,10 +328,7 @@ out:
fclose(fin); fclose(fin);
process_unlock(); process_unlock();
static_lock(); return values[i];
value = values[i];
static_unlock();
return value;
} }
const char *default_lvm_vg(void) const char *default_lvm_vg(void)
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment