[lxc-devel] [PATCH] remove static_lock()/static_unlock() and start to use thread local storage

[S.Çağlar Onur]

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
    lxcpath=lxcpath at entry=0x7f49200b2010 "/var/lib/lxc") at start.c:951
(gdb)

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>
---
 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