On Sat, Jan 04, 2014 at 12:00:04AM -0500, S.Çağlar Onur wrote: > This patch; > > Adds pthread_atfork check to configure.ac and uses it when necessary, > Introduces tls.m4 macro for checking thread-local storage support, > Puts values array into thread-local storage > (lxc_global_config_value@src/lxc/utils.c), > Removes static_lock/static_unlock from LXC code, > > Lastly, it introduces a warning for bionic users about multithreaded usage of > LXC. > > (requires 64b1be2903078ef9e9ba3ffcbc30a4dc9bc5cc6c to be reverted first) > > Signed-off-by: S.Çağlar Onur <cag...@10ur.org>
Confirmed to restore the bionic build to a working state, thanks!
Acked-by: Stéphane Graber <stgra...@ubuntu.com>
> ---
> config/tls.m4 | 14 ++++++++++++++
> configure.ac | 15 +++++++++++++++
> src/lxc/lxclock.c | 21 ++-------------------
> src/lxc/lxclock.h | 10 ----------
> src/lxc/utils.c | 23 +++++++----------------
> 5 files changed, 38 insertions(+), 45 deletions(-)
> create mode 100644 config/tls.m4
>
> diff --git a/config/tls.m4 b/config/tls.m4
> new file mode 100644
> index 0000000..5d1ac59
> --- /dev/null
> +++ b/config/tls.m4
> @@ -0,0 +1,14 @@
> +# See if we have working TLS. We only check to see if it compiles, and that
> +# the resulting program actually runs, not whether the resulting TLS
> variables
> +# work properly; that check is done at runtime, since we can run binaries
> +# compiled with __thread on systems without TLS.
> +AC_DEFUN([LXC_CHECK_TLS],
> +[
> + AC_MSG_CHECKING(for TLS)
> + AC_RUN_IFELSE([AC_LANG_SOURCE([[ static __thread int val; int main() {
> return 0; } ]])],[have_tls=yes],[have_tls=no],[have_tls=no ])
> + AC_MSG_RESULT($have_tls)
> + if test "$have_tls" = "yes"; then
> + AC_DEFINE([HAVE_TLS],[1],[Define if the compiler supports __thread])
> + AC_DEFINE([thread_local],[__thread],[Define to the compiler TLS
> keyword])
> + fi
> +])
> diff --git a/configure.ac b/configure.ac
> index 2d24937..af0991d 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -483,6 +483,8 @@ AC_CHECK_HEADERS([sys/signalfd.h pty.h ifaddrs.h
> sys/capability.h sys/personalit
> AC_CHECK_FUNCS([setns pivot_root sethostname unshare rand_r confstr])
>
> # Check for some functions
> +AC_CHECK_LIB(pthread, main)
> +AC_CHECK_FUNCS(pthread_atfork)
> AC_CHECK_LIB(util, openpty)
> AC_CHECK_FUNCS([openpty hasmntopt setmntent endmntent])
> AC_CHECK_FUNCS([getline],
> @@ -502,6 +504,9 @@ AC_SEARCH_LIBS(clock_gettime, [rt])
> AC_PROG_GCC_TRADITIONAL
> AC_PROG_SED
>
> +# See if we support thread-local storage.
> +LXC_CHECK_TLS
> +
> if test "x$GCC" = "xyes"; then
> CFLAGS="$CFLAGS -Wall -Werror"
> fi
> @@ -680,3 +685,13 @@ Debugging:
> Paths:
> - Logs in configpath: $enable_configpath_log
> EOF
> +
> +if test "x$ac_cv_func_pthread_atfork" = "xno" ; then
> +cat << EOF
> +
> +WARNING: Threading not supported on your platform
> +
> + You are compiling LXC for bionic target which lacks certain threading
> related functionality used by LXC API (like pthread_atfork).
> + Please note that, because of the missing functionality, multithreaded
> usage of LXC API cause some problems.
> +EOF
> +fi
> diff --git a/src/lxc/lxclock.c b/src/lxc/lxclock.c
> index b0420bb..3e1b054 100644
> --- a/src/lxc/lxclock.c
> +++ b/src/lxc/lxclock.c
> @@ -46,7 +46,6 @@ lxc_log_define(lxc_lock, lxc);
>
> #ifdef MUTEX_DEBUGGING
> static pthread_mutex_t thread_mutex =
> PTHREAD_ERRORCHECK_MUTEX_INITIALIZER_NP;
> -static pthread_mutex_t static_mutex =
> PTHREAD_ERRORCHECK_MUTEX_INITIALIZER_NP;
>
> static inline void dump_stacktrace(void)
> {
> @@ -68,7 +67,6 @@ static inline void dump_stacktrace(void)
> }
> #else
> static pthread_mutex_t thread_mutex = PTHREAD_MUTEX_INITIALIZER;
> -static pthread_mutex_t static_mutex = PTHREAD_MUTEX_INITIALIZER;
>
> static inline void dump_stacktrace(void) {;}
> #endif
> @@ -326,28 +324,13 @@ void process_unlock(void)
> * to unlock the mutex.
> * This forbids doing fork() while explicitly holding the lock.
> */
> +#ifdef HAVE_PTHREAD_ATFORK
> __attribute__((constructor))
> static void process_lock_setup_atfork(void)
> {
> pthread_atfork(process_lock, process_unlock, process_unlock);
> }
> -
> -/* 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);
> -}
> -
> -__attribute__((constructor))
> -static void static_lock_setup_atfork(void)
> -{
> - pthread_atfork(static_lock, static_unlock, static_unlock);
> -}
> +#endif
>
> int container_mem_lock(struct lxc_container *c)
> {
> 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 1f9ceea..5f2c8b2 100644
> --- a/src/lxc/utils.c
> +++ b/src/lxc/utils.c
> @@ -250,9 +250,12 @@ static const char *lxc_global_config_value(const char
> *option_name)
> { NULL, NULL },
> };
>
> - /* Protected by a mutex to eliminate conflicting load and store
> operations */
> + /* placed in the thread local storage pool for non-bionic targets */
> +#ifdef HAVE_TLS
> + static __thread const char *values[sizeof(options) /
> sizeof(options[0])] = { 0 };
> +#else
> static const char *values[sizeof(options) / sizeof(options[0])] = { 0 };
> -
> +#endif
> char *user_config_path = NULL;
> char *user_lxc_path = NULL;
>
> @@ -273,7 +276,6 @@ static const char *lxc_global_config_value(const char
> *option_name)
> }
>
> const char * const (*ptr)[2];
> - const char *value;
> size_t i;
> char buf[1024], *p, *p2;
> FILE *fin = NULL;
> @@ -289,15 +291,11 @@ static const char *lxc_global_config_value(const char
> *option_name)
> return NULL;
> }
>
> - static_lock();
> if (values[i]) {
> free(user_config_path);
> free(user_lxc_path);
> - value = values[i];
> - static_unlock();
> - return value;
> + return values[i];
> }
> - static_unlock();
>
> fin = fopen_cloexec(user_config_path, "r");
> free(user_config_path);
> @@ -333,15 +331,12 @@ static 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();
> free(user_lxc_path);
> goto out;
> }
> }
> /* could not find value, use default */
> - static_lock();
> if (strcmp(option_name, "lxcpath") == 0)
> values[i] = user_lxc_path;
> else {
> @@ -353,16 +348,12 @@ static const char *lxc_global_config_value(const char
> *option_name)
> * as an error... */
> if (!values[i])
> errno = 0;
> - static_unlock();
>
> out:
> if (fin)
> fclose(fin);
>
> - 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@lists.linuxcontainers.org
> 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 lxc-devel@lists.linuxcontainers.org http://lists.linuxcontainers.org/listinfo/lxc-devel
