MEDIUM: task: remove TASK_SHARED_WQ and only use t->tid

TASK_SHARED_WQ was set upon task creation and never changed afterwards.
Thus if a task was created to run anywhere (e.g. a check or a Lua task),
all its timers would always pass through the shared timers queue with a
lock. Now we know that tid<0 indicates a shared task, so we can use that
to decide whether or not to use the shared queue. The task might be
migrated using task_set_affinity() but it's always dequeued first so
the check will still be valid.

Not only this removes a flag that's difficult to keep synchronized with
the thread ID, but it should significantly lower the load on systems with
many checks. A quick test with 5000 servers and fast checks that were
saturating the CPU shows that the check rate increased by 20% (hence the
CPU usage dropped by 17%). It's worth noting that run_task_lists() almost
no longer appears in perf top now.

diff --git a/dev/flags/flags.c b/dev/flags/flags.c
index 2f7c76b638c8cee0957b9debcd37054f61ed4cf0..620a669844f52af0d386b2b865a9878d0dc09ccb 100644 (file)
--- a/dev/flags/flags.c
+++ b/dev/flags/flags.c
@@ -283,7 +283,6 @@ void show_task_state(unsigned int f)
        SHOW_FLAG(f, TASK_IN_LIST);
        SHOW_FLAG(f, TASK_KILLED);
        SHOW_FLAG(f, TASK_SELF_WAKING);
-       SHOW_FLAG(f, TASK_SHARED_WQ);
        SHOW_FLAG(f, TASK_QUEUED);
        SHOW_FLAG(f, TASK_GLOBAL);
        SHOW_FLAG(f, TASK_RUNNING);

diff --git a/include/haproxy/task-t.h b/include/haproxy/task-t.h
index 9d74fd91a18e173c3eabce20dd14de5da05865d0..22d7b373cbc58e49bb5581fda221f8b2aacaf38c 100644 (file)
--- a/include/haproxy/task-t.h
+++ b/include/haproxy/task-t.h
@@ -34,8 +34,7 @@
 #define TASK_RUNNING      0x00000001  /* the task is currently running */
 #define TASK_GLOBAL       0x00000002  /* The task is currently in the global runqueue */
 #define TASK_QUEUED       0x00000004  /* The task has been (re-)added to the run queue */
-#define TASK_SHARED_WQ    0x00000008  /* The task's expiration may be updated by other
-                                       * threads, must be set before first queue/wakeup */
+/* unused                 0x00000008 */
 #define TASK_SELF_WAKING  0x00000010  /* task/tasklet found waking itself */
 #define TASK_KILLED       0x00000020  /* task/tasklet killed, may now be freed */
 #define TASK_IN_LIST      0x00000040  /* tasklet is in a tasklet list */
@@ -59,7 +58,7 @@
 /* unused: 0x20000..0x80000000 */
 
 /* These flags are persistent across scheduler calls */
-#define TASK_PERSISTENT   (TASK_SHARED_WQ | TASK_SELF_WAKING | TASK_KILLED | \
+#define TASK_PERSISTENT   (TASK_SELF_WAKING | TASK_KILLED | \
                            TASK_HEAVY | TASK_F_TASKLET | TASK_F_USR1)
 
 struct notification {

diff --git a/include/haproxy/task.h b/include/haproxy/task.h
index 4a794333e8475abc62eb3f79ca50d3fdfa53656c..a753cd90bb17735db9cd97e99593c3071d5b9a2c 100644 (file)
--- a/include/haproxy/task.h
+++ b/include/haproxy/task.h
@@ -278,8 +278,8 @@ static inline struct task *task_unlink_wq(struct task *t)
        unsigned long locked;
 
        if (likely(task_in_wq(t))) {
-               locked = t->state & TASK_SHARED_WQ;
-               BUG_ON(!locked && t->tid != tid);
+               locked = t->tid < 0;
+               BUG_ON(t->tid >= 0 && t->tid != tid);
                if (locked)
                        HA_RWLOCK_WRLOCK(TASK_WQ_LOCK, &wq_lock);
                __task_unlink_wq(t);
@@ -310,7 +310,7 @@ static inline void task_queue(struct task *task)
                return;
 
 #ifdef USE_THREAD
-       if (task->state & TASK_SHARED_WQ) {
+       if (task->tid < 0) {
                HA_RWLOCK_WRLOCK(TASK_WQ_LOCK, &wq_lock);
                if (!task_in_wq(task) || tick_is_lt(task->expire, task->wq.key))
                        __task_queue(task, &timers);
@@ -318,7 +318,7 @@ static inline void task_queue(struct task *task)
        } else
 #endif
        {
-               BUG_ON(task->tid != tid); // should have TASK_SHARED_WQ
+               BUG_ON(task->tid != tid);
                if (!task_in_wq(task) || tick_is_lt(task->expire, task->wq.key))
                        __task_queue(task, &th_ctx->timers);
        }
@@ -547,8 +547,6 @@ static inline struct task *task_init(struct task *t, int tid)
        tid = 0;
 #endif
        t->tid = tid;
-       if (tid < 0)
-               t->state |= TASK_SHARED_WQ;
        t->nice = 0;
        t->calls = 0;
        t->call_date = 0;
@@ -709,7 +707,7 @@ static inline void task_schedule(struct task *task, int when)
                return;
 
 #ifdef USE_THREAD
-       if (task->state & TASK_SHARED_WQ) {
+       if (task->tid < 0) {
                /* FIXME: is it really needed to lock the WQ during the check ? */
                HA_RWLOCK_WRLOCK(TASK_WQ_LOCK, &wq_lock);
                if (task_in_wq(task))

diff --git a/src/task.c b/src/task.c
index c32b9e4db5cec421ae0f1b5837b9e7ec38845474..eac03fe752deee865d761d53a359746718c0d99d 100644 (file)
--- a/src/task.c
+++ b/src/task.c
@@ -302,8 +302,8 @@ void __task_wakeup(struct task *t)
 void __task_queue(struct task *task, struct eb_root *wq)
 {
 #ifdef USE_THREAD
-       BUG_ON((wq == &timers && !(task->state & TASK_SHARED_WQ)) ||
-              (wq == &th_ctx->timers && (task->state & TASK_SHARED_WQ)) ||
+       BUG_ON((wq == &timers && task->tid >= 0) ||
+              (wq == &th_ctx->timers && task->tid < 0) ||
               (wq != &timers && wq != &th_ctx->timers));
 #endif
        /* if this happens the process is doomed anyway, so better catch it now