From 13afcb7ab31a8f4e82ea943128706b5bd8df9e51 Mon Sep 17 00:00:00 2001 From: Willy Tarreau Date: Sun, 27 Jan 2019 17:41:27 +0100 Subject: [PATCH] BUG/MINOR: task: fix possibly missed event in inter-thread wakeups There's a very small but existing uncertainty window when waking another thread up where it is possible for task_wakeup() not to wake the other task up because it's still running while this once is in the process of finishing and loses its TASK_RUNNING flag. In this case the wakeup will be missed. The problem is that we have a single flag to store 3 states, since the transition from running to sleeping isn't atomic. Thus we need to have another flag to cover this part. This patch introduces TASK_QUEUED to mention that the task is already in the run queue, running or not. This bit will be removed while TASK_RUNNING is kept once dequeued, and will be used when removing TASK_RUNNING to check if the task has been requeued. It might be possible to slightly improve this but the occurrence rate is quite low and we don't really need to complexify the scheduler to optimize for a rare case. The impact with the current code is very low since we have few inter- thread wakeups. Most of them are caused by checks killing sessions. This must be backported to 1.9. --- include/proto/task.h | 7 +++++-- include/types/task.h | 1 + 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/include/proto/task.h b/include/proto/task.h index bd2fe4cbb..0177c52ba 100644 --- a/include/proto/task.h +++ b/include/proto/task.h @@ -152,8 +152,11 @@ static inline void task_wakeup(struct task *t, unsigned int f) struct eb_root *root = &task_per_thread[tid].rqueue; #endif - state = HA_ATOMIC_OR(&t->state, f); - if (!(state & TASK_RUNNING)) + f |= TASK_QUEUED; + state = t->state; + while (!HA_ATOMIC_CAS(&t->state, &state, state | f)) + ; + if (!(state & TASK_QUEUED)) __task_wakeup(t, root); } diff --git a/include/types/task.h b/include/types/task.h index e8ade67d6..508667c7b 100644 --- a/include/types/task.h +++ b/include/types/task.h @@ -33,6 +33,7 @@ #define TASK_SLEEPING 0x0000 /* task sleeping */ #define TASK_RUNNING 0x0001 /* the task is currently running */ #define TASK_GLOBAL 0x0002 /* The task is currently in the global runqueue */ +#define TASK_QUEUED 0x0004 /* The task has been (re-)added to the run queue */ #define TASK_WOKEN_INIT 0x0100 /* woken up for initialisation purposes */ #define TASK_WOKEN_TIMER 0x0200 /* woken up because of expired timer */