From 9cf7c4b9df2e050db972fe9c93a9befe980456f0 Mon Sep 17 00:00:00 2001 From: Thierry FOURNIER Date: Mon, 15 Dec 2014 13:26:01 +0100 Subject: [PATCH] MAJOR: poll: only rely on wake_expired_tasks() to compute the wait delay Actually, HAProxy uses the function "process_runnable_tasks" and "wake_expired_tasks" to get the next task which can expires. If a task is added with "task_schedule" or other method during the execution of an other task, the expiration of this new task is not taken into account, and the execution of this task can be too late. Actualy, HAProxy seems to be no sensitive to this bug. This fix moves the call to process_runnable_tasks() before the timeout calculation and ensures that all wakeups are processed together. Only wake_expired_tasks() needs to return a timeout now. --- include/proto/task.h | 4 ++-- src/haproxy.c | 8 ++++---- src/task.c | 22 +++++++--------------- 3 files changed, 13 insertions(+), 21 deletions(-) diff --git a/include/proto/task.h b/include/proto/task.h index 76f77c87e..107c961a5 100644 --- a/include/proto/task.h +++ b/include/proto/task.h @@ -259,13 +259,13 @@ static inline void task_schedule(struct task *task, int when) * - return the date of next event in or eternity. */ -void process_runnable_tasks(int *next); +void process_runnable_tasks(); /* * Extract all expired timers from the timer queue, and wakes up all * associated tasks. Returns the date of next event (or eternity). */ -void wake_expired_tasks(int *next); +int wake_expired_tasks(); /* Perform minimal initializations, report 0 in case of error, 1 if OK. */ int init_task(); diff --git a/src/haproxy.c b/src/haproxy.c index c5ffa0fb9..f50deff95 100644 --- a/src/haproxy.c +++ b/src/haproxy.c @@ -1459,14 +1459,14 @@ void run_poll_loop() tv_update_date(0,1); while (1) { + /* Process a few tasks */ + process_runnable_tasks(); + /* check if we caught some signals and process them */ signal_process_queue(); /* Check if we can expire some tasks */ - wake_expired_tasks(&next); - - /* Process a few tasks */ - process_runnable_tasks(&next); + next = wake_expired_tasks(); /* stop when there's nothing left to do */ if (jobs == 0) diff --git a/src/task.c b/src/task.c index 74ae1c9a7..4985aa22a 100644 --- a/src/task.c +++ b/src/task.c @@ -118,9 +118,9 @@ void __task_queue(struct task *task) /* * Extract all expired timers from the timer queue, and wakes up all - * associated tasks. Returns the date of next event (or eternity) in . + * associated tasks. Returns the date of next event (or eternity). */ -void wake_expired_tasks(int *next) +int wake_expired_tasks() { struct task *task; struct eb32_node *eb; @@ -139,8 +139,7 @@ void wake_expired_tasks(int *next) if (likely(tick_is_lt(now_ms, eb->key))) { /* timer not expired yet, revisit it later */ - *next = eb->key; - return; + return eb->key; } /* timer looks expired, detach it from the queue */ @@ -172,9 +171,8 @@ void wake_expired_tasks(int *next) task_wakeup(task, TASK_WOKEN_TIMER); } - /* We have found no task to expire in any tree */ - *next = TICK_ETERNITY; - return; + /* No task is expired */ + return TICK_ETERNITY; } /* The run queue is chronologically sorted in a tree. An insertion counter is @@ -187,11 +185,10 @@ void wake_expired_tasks(int *next) * * The function adjusts if a new event is closer. */ -void process_runnable_tasks(int *next) +void process_runnable_tasks() { struct task *t; unsigned int max_processed; - int expire; run_queue_cur = run_queue; /* keep a copy for reporting */ nb_tasks_cur = nb_tasks; @@ -206,8 +203,6 @@ void process_runnable_tasks(int *next) if (likely(niced_tasks)) max_processed = (max_processed + 3) / 4; - expire = *next; - while (max_processed--) { /* Note: this loop is one of the fastest code path in * the whole program. It should not be re-arranged @@ -245,13 +240,10 @@ void process_runnable_tasks(int *next) if (likely(t != NULL)) { t->state &= ~TASK_RUNNING; - if (t->expire) { + if (t->expire) task_queue(t); - expire = tick_first_2nz(expire, t->expire); - } } } - *next = expire; } /* perform minimal intializations, report 0 in case of error, 1 if OK. */