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 35b6e6eac2fdc95b8db21c37d7cc3b68787c2926..068b2a1530a7b9bb3b3fed1996010bb6b84714d0 100644 (file)
--- 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 9841e2dc6a3f1d38a3e3057700fc8d7b39625780..803a9189bd8531430d5764c183031a491fc0551e 100644 (file)
--- 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 8525593e2e38ceeb2f4079e2932262aa6cdaed3d..810d5983890f221a227114be333352a0551490ad 100644 (file)
--- 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 c0c5956f504922efb93f2241089a827aa2130164..bfd055aa9b9c56c2078a8175fe117d6272b2c9d4 100644 (file)
--- a/src/hathreads.c
+++ b/src/hathreads.c
@@ -11,6 +11,7 @@
  */
 
 #include <unistd.h>
+#include <stdlib.h>
 #include <fcntl.h>
 
 #include <common/cfgparse.h>
@@ -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 <arg>, returns it and adjusts a few
+ * internal variables accordingly, or fails and returns zero with an error
+ * reason in <errmsg>. 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;
+}