From: Willy Tarreau Date: Mon, 30 Jul 2018 08:34:35 +0000 (+0200) Subject: MINOR: threads: move "nbthread" parsing to hathreads.c X-Git-Tag: v1.9-dev1~8 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=0ccd32285fbe8a1a394f7b8fb9f7e6282ec9a32d;p=thirdparty%2Fhaproxy.git MINOR: threads: move "nbthread" parsing to hathreads.c The purpose is to make sure that all variables which directly depend on this nbthread argument are set at the right moment. For now only all_threads_mask needs to be set. It used to be set while calling thread_sync_init() which is called too late for certain checks. The same function handles threads and non-threads, which removes the need for some thread-specific knowledge from cfgparse.c. --- diff --git a/include/common/hathreads.h b/include/common/hathreads.h index 35b6e6eac2..068b2a1530 100644 --- a/include/common/hathreads.h +++ b/include/common/hathreads.h @@ -28,6 +28,12 @@ extern THREAD_LOCAL unsigned int tid; /* The thread id */ extern THREAD_LOCAL unsigned long tid_bit; /* The bit corresponding to the thread id */ +/* Note about all_threads_mask : + * - with threads support disabled, this symbol is defined as zero (0UL). + * - with threads enabled, this variable is never zero, it contains the mask + * of enabled threads. Thus if only one thread is enabled, it equals 1. + */ + #ifndef USE_THREAD #define MAX_THREADS 1 @@ -85,7 +91,7 @@ extern THREAD_LOCAL unsigned long tid_bit; /* The bit corresponding to the threa #define HA_BARRIER() do { } while (0) -#define THREAD_SYNC_INIT(m) do { /* do nothing */ } while(0) +#define THREAD_SYNC_INIT() do { /* do nothing */ } while(0) #define THREAD_SYNC_ENABLE() do { /* do nothing */ } while(0) #define THREAD_WANT_SYNC() do { /* do nothing */ } while(0) #define THREAD_ENTER_SYNC() do { /* do nothing */ } while(0) @@ -244,7 +250,7 @@ static inline void __ha_barrier_full(void) #define HA_BARRIER() pl_barrier() -#define THREAD_SYNC_INIT(m) thread_sync_init(m) +#define THREAD_SYNC_INIT() thread_sync_init() #define THREAD_SYNC_ENABLE() thread_sync_enable() #define THREAD_WANT_SYNC() thread_want_sync() #define THREAD_ENTER_SYNC() thread_enter_sync() @@ -252,7 +258,7 @@ static inline void __ha_barrier_full(void) #define THREAD_NO_SYNC() thread_no_sync() #define THREAD_NEED_SYNC() thread_need_sync() -int thread_sync_init(int nbthread); +int thread_sync_init(); void thread_sync_enable(void); void thread_want_sync(void); void thread_enter_sync(void); @@ -877,5 +883,6 @@ static inline void __ha_compiler_barrier(void) /* Dummy I/O handler used by the sync pipe.*/ void thread_sync_io_handler(int fd); +int parse_nbthread(const char *arg, char **err); #endif /* _COMMON_HATHREADS_H */ diff --git a/src/cfgparse.c b/src/cfgparse.c index 9841e2dc6a..803a9189bd 100644 --- a/src/cfgparse.c +++ b/src/cfgparse.c @@ -1190,18 +1190,10 @@ int cfg_parse_global(const char *file, int linenum, char **args, int kwm) err_code |= ERR_ALERT | ERR_FATAL; goto out; } - global.nbthread = atol(args[1]); -#ifndef USE_THREAD - if (global.nbthread > 1) { - ha_alert("HAProxy is not compiled with threads support, please check build options for USE_THREAD.\n"); - global.nbthread = 1; - err_code |= ERR_ALERT | ERR_FATAL; - goto out; - } -#endif - if (global.nbthread < 1 || global.nbthread > MAX_THREADS) { - ha_alert("parsing [%s:%d] : '%s' must be between 1 and %d (was %d).\n", - file, linenum, args[0], MAX_THREADS, global.nbthread); + global.nbthread = parse_nbthread(args[1], &errmsg); + if (!global.nbthread) { + ha_alert("parsing [%s:%d] : '%s' %s.\n", + file, linenum, args[0], errmsg); err_code |= ERR_ALERT | ERR_FATAL; goto out; } diff --git a/src/haproxy.c b/src/haproxy.c index 8525593e2e..810d598389 100644 --- a/src/haproxy.c +++ b/src/haproxy.c @@ -3030,7 +3030,7 @@ int main(int argc, char **argv) int i; sigset_t blocked_sig, old_sig; - THREAD_SYNC_INIT(global.nbthread); + THREAD_SYNC_INIT(); /* Init tids array */ for (i = 0; i < global.nbthread; i++) diff --git a/src/hathreads.c b/src/hathreads.c index c0c5956f50..bfd055aa9b 100644 --- a/src/hathreads.c +++ b/src/hathreads.c @@ -11,6 +11,7 @@ */ #include +#include #include #include @@ -31,7 +32,7 @@ void thread_sync_io_handler(int fd) static HA_SPINLOCK_T sync_lock; static int threads_sync_pipe[2]; static unsigned long threads_want_sync = 0; -volatile unsigned long all_threads_mask = 0; +volatile unsigned long all_threads_mask = 1; // nbthread 1 assumed by default #if defined(DEBUG_THREAD) || defined(DEBUG_FULL) struct lock_stat lock_stats[LOCK_LABELS]; @@ -41,7 +42,7 @@ struct lock_stat lock_stats[LOCK_LABELS]; * others when a sync is requested. It also initializes the mask of all created * threads. It returns 0 on success and -1 if an error occurred. */ -int thread_sync_init(int nbthread) +int thread_sync_init() { int rfd; @@ -51,10 +52,6 @@ int thread_sync_init(int nbthread) rfd = threads_sync_pipe[0]; fcntl(rfd, F_SETFL, O_NONBLOCK); fd_insert(rfd, thread_sync_io_handler, thread_sync_io_handler, MAX_THREADS_MASK); - - /* we proceed like this to be sure never to overflow the left shift */ - all_threads_mask = 1UL << (nbthread - 1); - all_threads_mask |= all_threads_mask - 1; return 0; } @@ -173,4 +170,38 @@ static void __hathreads_init(void) hap_register_build_opts("Built with multi-threading support.", 0); } +#endif // USE_THREAD + + +/* Parse the number of threads in argument , returns it and adjusts a few + * internal variables accordingly, or fails and returns zero with an error + * reason in . May be called multiple times while parsing. + */ +int parse_nbthread(const char *arg, char **err) +{ + long nbthread; + char *errptr; + + nbthread = strtol(arg, &errptr, 10); + if (!*arg || *errptr) { + memprintf(err, "passed a missing or unparsable integer value in '%s'", arg); + return 0; + } + +#ifndef USE_THREAD + if (nbthread != 1) { + memprintf(err, "specified with a value other than 1 while HAProxy is not compiled with threads support. Please check build options for USE_THREAD"); + return 0; + } +#else + if (nbthread < 1 || nbthread > MAX_THREADS) { + memprintf(err, "value must be between 1 and %d (was %ld)", MAX_THREADS, nbthread); + return 0; + } + + /* we proceed like this to be sure never to overflow the left shift */ + all_threads_mask = 1UL << (nbthread - 1); + all_threads_mask |= all_threads_mask - 1; #endif + return nbthread; +}