From 95b422fccfed6e6f5973c768c0cfdbca65c68e67 Mon Sep 17 00:00:00 2001 From: =?utf8?q?S=2E=C3=87a=C4=9Flar=20Onur?= Date: Thu, 19 Dec 2013 00:08:51 -0500 Subject: [PATCH] remove static_lock()/static_unlock() and start to use thread local storage (v2) MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit 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 ) at pthread_mutex_lock.c:64 #3 0x00007f49554b27a6 in lock_mutex (l=l@entry=0x7f49556d4600 ) 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 , 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=, argv=0x7f495487dbe0) at lxccontainer.c:648 #12 0x0000000000401317 in do_function (arguments=0x1aa80b0) at concurrent.c:94 #13 0x0000000000401499 in concurrent (arguments=) 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 Acked-by: Serge E. Hallyn --- 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 64823d2a5..62e774ff9 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 820e819ac..a02a0320c 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 b60530758..785f3e6b0 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) -- 2.47.2