From 43ab05b3da3d1ec87b080903545d3f5eec1c96d0 Mon Sep 17 00:00:00 2001 From: Willy Tarreau Date: Tue, 28 Sep 2021 09:43:11 +0200 Subject: [PATCH] MEDIUM: threads: replace ha_set_tid() with ha_set_thread() ha_set_tid() was randomly used either to explicitly set thread 0 or to set any possibly incomplete thread during boot. Let's replace it with a pointer to a valid thread or NULL for any thread. This allows us to check that the designated threads are always valid, and to ignore the thread 0's mapping when setting it to NULL, and always use group 0 with it during boot. The initialization code is also cleaner, as we don't pass ugly casts of a thread ID to a pointer anymore. --- include/haproxy/thread.h | 48 ++++++++++++++++++++++++++++------------ src/haproxy.c | 7 +++--- src/hlua.c | 18 +++++++-------- src/thread.c | 2 +- 4 files changed, 48 insertions(+), 27 deletions(-) diff --git a/include/haproxy/thread.h b/include/haproxy/thread.h index c70108b54..d55c54e08 100644 --- a/include/haproxy/thread.h +++ b/include/haproxy/thread.h @@ -88,11 +88,21 @@ enum { tid = 0 }; #define ha_sigmask(how, set, oldset) sigprocmask(how, set, oldset) -static inline void ha_set_tid(unsigned int tid) +/* Sets the current thread to a valid one described by , or to any thread + * and any group if NULL (e.g. for use during boot where they're not totally + * initialized). + */ +static inline void ha_set_thread(const struct thread_info *thr) { - ti = &ha_thread_info[tid]; - tg = ti->tg ? ti->tg : &ha_tgroup_info[0]; - th_ctx = &ha_thread_ctx[tid]; + if (thr) { + ti = thr; + tg = ti->tg; + th_ctx = &ha_thread_ctx[ti->tid]; + } else { + ti = &ha_thread_info[0]; + tg = &ha_tgroup_info[0]; + th_ctx = &ha_thread_ctx[0]; + } } static inline void thread_idle_now() @@ -203,18 +213,28 @@ extern THREAD_LOCAL unsigned int tid; /* The thread id */ #define ha_sigmask(how, set, oldset) pthread_sigmask(how, set, oldset) -/* sets the thread ID, TID bit and thread cfg/ctx pointers for the current - * thread. Since it may be called during early boot even before threads are - * initialized, we have to be extra careful about some fields which may still - * be null. For example tg may be null during a part of the boot. +/* Sets the current thread to a valid one described by , or to any thread + * and any group if NULL (e.g. for use during boot where they're not totally + * initialized). */ -static inline void ha_set_tid(unsigned int data) +static inline void ha_set_thread(const struct thread_info *thr) { - tid = data; - tid_bit = (1UL << tid); - ti = &ha_thread_info[tid]; - tg = ti->tg ? ti->tg : &ha_tgroup_info[0]; - th_ctx = &ha_thread_ctx[tid]; + if (thr) { + BUG_ON(!thr->tid_bit); + BUG_ON(!thr->tg); + + ti = thr; + tg = thr->tg; + tid = thr->tid; + tid_bit = thr->tid_bit; + th_ctx = &ha_thread_ctx[tid]; + } else { + tid = 0; + tid_bit = 1; + ti = &ha_thread_info[0]; + tg = &ha_tgroup_info[0]; + th_ctx = &ha_thread_ctx[0]; + } } /* Marks the thread as idle, which means that not only it's not doing anything diff --git a/src/haproxy.c b/src/haproxy.c index c8b5ee781..7a4dc790e 100644 --- a/src/haproxy.c +++ b/src/haproxy.c @@ -253,6 +253,7 @@ unsigned int rlim_fd_max_at_boot = 0; /* per-boot randomness */ unsigned char boot_seed[20]; /* per-boot random seed (160 bits initially) */ +/* takes the thread config in argument or NULL for any thread */ static void *run_thread_poll_loop(void *data); /* bitfield of a few warnings to emit just once (WARN_*) */ @@ -836,7 +837,7 @@ static void mworker_loop() leave */ fork_poller(); - run_thread_poll_loop(0); + run_thread_poll_loop(NULL); } /* @@ -2697,7 +2698,7 @@ static void *run_thread_poll_loop(void *data) __decl_thread(static pthread_mutex_t init_mutex = PTHREAD_MUTEX_INITIALIZER); __decl_thread(static pthread_cond_t init_cond = PTHREAD_COND_INITIALIZER); - ha_set_tid((unsigned long)data); + ha_set_thread(data); set_thread_cpu_affinity(); clock_set_local_source(); @@ -3397,7 +3398,7 @@ int main(int argc, char **argv) haproxy_unblock_signals(); /* Finally, start the poll loop for the first thread */ - run_thread_poll_loop(0); + run_thread_poll_loop(&ha_thread_info[0]); /* wait for all threads to terminate */ wait_for_threads_completion(); diff --git a/src/hlua.c b/src/hlua.c index d5d3f599f..b03d8bb46 100644 --- a/src/hlua.c +++ b/src/hlua.c @@ -11060,7 +11060,7 @@ static int hlua_load(char **args, int section_type, struct proxy *curpx, /* loading for global state */ hlua_state_id = 0; - ha_set_tid(0); + ha_set_thread(NULL); return hlua_load_state(args[1], hlua_states[0], err); } @@ -11104,7 +11104,7 @@ static int hlua_load_per_thread(char **args, int section_type, struct proxy *cur /* loading for thread 1 only */ hlua_state_id = 1; - ha_set_tid(0); + ha_set_thread(NULL); return hlua_load_state(args[1], hlua_states[1], err); } @@ -11311,7 +11311,7 @@ int hlua_post_init() /* Perform post init of common thread */ hlua_state_id = 0; - ha_set_tid(0); + ha_set_thread(&ha_thread_info[0]); ret = hlua_post_init_state(hlua_states[hlua_state_id]); if (ret == 0) return 0; @@ -11320,7 +11320,7 @@ int hlua_post_init() for (hlua_state_id = 2; hlua_state_id < global.nbthread + 1; hlua_state_id++) { /* set thread context */ - ha_set_tid(hlua_state_id - 1); + ha_set_thread(&ha_thread_info[hlua_state_id - 1]); /* Init lua state */ hlua_states[hlua_state_id] = hlua_init_state(hlua_state_id); @@ -11336,13 +11336,13 @@ int hlua_post_init() } /* Reset thread context */ - ha_set_tid(0); + ha_set_thread(NULL); /* Execute post init for all states */ for (hlua_state_id = 1; hlua_state_id < global.nbthread + 1; hlua_state_id++) { /* set thread context */ - ha_set_tid(hlua_state_id - 1); + ha_set_thread(&ha_thread_info[hlua_state_id - 1]); /* run post init */ ret = hlua_post_init_state(hlua_states[hlua_state_id]); @@ -11351,7 +11351,7 @@ int hlua_post_init() } /* Reset thread context */ - ha_set_tid(0); + ha_set_thread(NULL); /* control functions registering. Each function must have: * - only the function_ref[0] set positive and all other to -1 @@ -12057,12 +12057,12 @@ void hlua_init(void) { /* Init state for common/shared lua parts */ hlua_state_id = 0; - ha_set_tid(0); + ha_set_thread(NULL); hlua_states[0] = hlua_init_state(0); /* Init state 1 for thread 0. We have at least one thread. */ hlua_state_id = 1; - ha_set_tid(0); + ha_set_thread(NULL); hlua_states[1] = hlua_init_state(1); /* Proxy and server configuration initialisation. */ diff --git a/src/thread.c b/src/thread.c index 2f904deae..ce6a47659 100644 --- a/src/thread.c +++ b/src/thread.c @@ -217,7 +217,7 @@ void setup_extra_threads(void *(*handler)(void *)) /* Create nbthread-1 thread. The first thread is the current process */ ha_pthread[0] = pthread_self(); for (i = 1; i < global.nbthread; i++) - pthread_create(&ha_pthread[i], NULL, handler, (void *)(long)i); + pthread_create(&ha_pthread[i], NULL, handler, &ha_thread_info[i]); } /* waits for all threads to terminate. Does nothing when threads are