]> git.ipfire.org Git - thirdparty/haproxy.git/commitdiff
BUG/MINOR: task: fix possibly missed event in inter-thread wakeups
authorWilly Tarreau <w@1wt.eu>
Sun, 27 Jan 2019 16:41:27 +0000 (17:41 +0100)
committerWilly Tarreau <w@1wt.eu>
Mon, 28 Jan 2019 14:03:04 +0000 (15:03 +0100)
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
include/types/task.h

index bd2fe4cbb6a24a9a77d8055cc6bf02237da51103..0177c52bab84ddd5135bad893ecfecf0458c27e9 100644 (file)
@@ -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);
 }
 
index e8ade67d6ee9ebd585b1a47085504b10e4403a72..508667c7b4c67e0e2381c61ad57373043da11a3a 100644 (file)
@@ -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 */