]> git.ipfire.org Git - thirdparty/haproxy.git/commitdiff
MINOR: thread: only use atomic ops to touch the flags
authorWilly Tarreau <w@1wt.eu>
Wed, 22 Jun 2022 07:19:46 +0000 (09:19 +0200)
committerWilly Tarreau <w@1wt.eu>
Fri, 1 Jul 2022 17:15:14 +0000 (19:15 +0200)
The thread flags are touched a little bit by other threads, e.g. the STUCK
flag may be set by other ones, and they're watched a little bit. As such
we need to use atomic ops only to manipulate them. Most places were already
using them, but here we generalize the practice. Only ha_thread_dump() does
not change because it's run under isolation.

include/haproxy/task.h
src/activity.c
src/debug.c
src/fd.c
src/listener.c
src/task.c
src/wdt.c

index da5a6843679eed797a179f8e5324512525b39e8d..a79319eb59f9fdd8512cc888b16a4ae58cb260d2 100644 (file)
@@ -367,7 +367,7 @@ static inline void _tasklet_wakeup_on(struct tasklet *tl, int thr, const char *f
        tl->debug.caller_idx = !tl->debug.caller_idx;
        tl->debug.caller_file[tl->debug.caller_idx] = file;
        tl->debug.caller_line[tl->debug.caller_idx] = line;
-       if (th_ctx->flags & TH_FL_TASK_PROFILING)
+       if (_HA_ATOMIC_LOAD(&th_ctx->flags) & TH_FL_TASK_PROFILING)
                tl->call_date = now_mono_time();
 #endif
        __tasklet_wakeup_on(tl, thr);
@@ -421,7 +421,7 @@ static inline void _task_instant_wakeup(struct task *t, unsigned int f, const ch
        tl->debug.caller_idx = !tl->debug.caller_idx;
        tl->debug.caller_file[tl->debug.caller_idx] = file;
        tl->debug.caller_line[tl->debug.caller_idx] = line;
-       if (th_ctx->flags & TH_FL_TASK_PROFILING)
+       if (_HA_ATOMIC_LOAD(&th_ctx->flags) & TH_FL_TASK_PROFILING)
                tl->call_date = now_mono_time();
 #endif
        __tasklet_wakeup_on(tl, thr);
index 82364dd0a44c6e3e7352f032a929e19c01a7e676..ff9effbde3744583b0b8d73069e8bf2148033b4b 100644 (file)
@@ -383,7 +383,7 @@ void activity_count_runtime(uint32_t run_time)
         * profiling to "on" when automatic, and going back below the "down"
         * threshold switches to off. The forced modes don't check the load.
         */
-       if (!(th_ctx->flags & TH_FL_TASK_PROFILING)) {
+       if (!(_HA_ATOMIC_LOAD(&th_ctx->flags) & TH_FL_TASK_PROFILING)) {
                if (unlikely((profiling & HA_PROF_TASKS_MASK) == HA_PROF_TASKS_ON ||
                             ((profiling & HA_PROF_TASKS_MASK) == HA_PROF_TASKS_AON &&
                             swrate_avg(run_time, TIME_STATS_SAMPLES) >= up)))
index a55d087b058bdfd11aa6ccf5c81e8e1d6ba4624b..4207d13099cbdd09f1e9b492664e23f4c26d2199 100644 (file)
@@ -154,6 +154,7 @@ void ha_backtrace_to_stderr(void)
  * The calling thread ID needs to be passed in <calling_tid> to display a star
  * in front of the calling thread's line (usually it's tid). Any stuck thread
  * is also prefixed with a '>'.
+ * It must be called under thread isolation.
  */
 void ha_thread_dump(struct buffer *buf, int thr, int calling_tid)
 {
@@ -1365,7 +1366,7 @@ void debug_handler(int sig, siginfo_t *si, void *arg)
         * if it didn't move.
         */
        if (!((threads_harmless_mask|sleeping_thread_mask) & tid_bit))
-               th_ctx->flags |= TH_FL_STUCK;
+               _HA_ATOMIC_OR(&th_ctx->flags, TH_FL_STUCK);
 }
 
 static int init_debug_per_thread()
index 96d77bd55fb2ea1ea0c92f025b123a3e99b23508..99c8c4e5b34bfa1cc133473ebc17c1b9c8583039 100644 (file)
--- a/src/fd.c
+++ b/src/fd.c
@@ -472,7 +472,7 @@ int fd_update_events(int fd, uint evts)
        uint new_flags, must_stop;
        ulong rmask, tmask;
 
-       th_ctx->flags &= ~TH_FL_STUCK; // this thread is still running
+       _HA_ATOMIC_AND(&th_ctx->flags, ~TH_FL_STUCK); // this thread is still running
 
        /* do nothing if the FD was taken over under us */
        do {
index 65bb766927873b41933d12f9d49cb1611274bde4..8a033fdc9e4045b7d9d23f5594b24e98054e945b 100644 (file)
@@ -1135,7 +1135,7 @@ void listener_accept(struct listener *l)
                }
 #endif
 
-               th_ctx->flags &= ~TH_FL_STUCK; // this thread is still running
+               _HA_ATOMIC_AND(&th_ctx->flags, ~TH_FL_STUCK); // this thread is still running
        } /* end of for (max_accept--) */
 
  end:
index 76a7432737fcd0754477c1089a109de92de4639d..00b2887c0998a471cafb31658ce90b2c9fe1bf6c 100644 (file)
@@ -241,7 +241,7 @@ void __task_wakeup(struct task *t)
                t->rq.key += offset;
        }
 
-       if (th_ctx->flags & TH_FL_TASK_PROFILING)
+       if (_HA_ATOMIC_LOAD(&th_ctx->flags) & TH_FL_TASK_PROFILING)
                t->call_date = now_mono_time();
 
        eb32_insert(root, &t->rq);
@@ -568,7 +568,7 @@ unsigned int run_tasks_from_lists(unsigned int budgets[])
                process = t->process;
                t->calls++;
                th_ctx->current = t;
-               th_ctx->flags &= ~TH_FL_STUCK; // this thread is still running
+               _HA_ATOMIC_AND(&th_ctx->flags, ~TH_FL_STUCK); // this thread is still running
 
                _HA_ATOMIC_DEC(&th_ctx->rq_total);
 
@@ -578,7 +578,7 @@ unsigned int run_tasks_from_lists(unsigned int budgets[])
                        LIST_DEL_INIT(&((struct tasklet *)t)->list);
                        __ha_barrier_store();
 
-                       if (unlikely(th_ctx->flags & TH_FL_TASK_PROFILING)) {
+                       if (unlikely(_HA_ATOMIC_LOAD(&th_ctx->flags) & TH_FL_TASK_PROFILING)) {
                                profile_entry = sched_activity_entry(sched_activity, t->process);
                                before = now_mono_time();
 #ifdef DEBUG_TASK
@@ -603,7 +603,7 @@ unsigned int run_tasks_from_lists(unsigned int budgets[])
                                continue;
                        }
 
-                       if (unlikely(th_ctx->flags & TH_FL_TASK_PROFILING)) {
+                       if (unlikely(_HA_ATOMIC_LOAD(&th_ctx->flags) & TH_FL_TASK_PROFILING)) {
                                HA_ATOMIC_INC(&profile_entry->calls);
                                HA_ATOMIC_ADD(&profile_entry->cpu_time, now_mono_time() - before);
                        }
@@ -734,7 +734,7 @@ void process_runnable_tasks()
        int heavy_queued = 0;
        int budget;
 
-       th_ctx->flags &= ~TH_FL_STUCK; // this thread is still running
+       _HA_ATOMIC_AND(&th_ctx->flags, ~TH_FL_STUCK); // this thread is still running
 
        if (!thread_has_tasks()) {
                activity[tid].empty_rq++;
index 6bb7d85a6dedfe320bb88c6738762566578ba70b..dc5c1dde5043e63480b58ca5c715f0b2b41cae60 100644 (file)
--- a/src/wdt.c
+++ b/src/wdt.c
@@ -99,7 +99,7 @@ void wdt_handler(int sig, siginfo_t *si, void *arg)
                 * If it's already set, then it's our second call with no
                 * progress and the thread is dead.
                 */
-               if (!(ha_thread_ctx[thr].flags & TH_FL_STUCK)) {
+               if (!(_HA_ATOMIC_LOAD(&ha_thread_ctx[thr].flags) & TH_FL_STUCK)) {
                        _HA_ATOMIC_OR(&ha_thread_ctx[thr].flags, TH_FL_STUCK);
                        goto update_and_leave;
                }