BUG/MEDIUM: task/h2: add an idempotent task removal fucntion

Previous commit 3ea351368 ("BUG/MEDIUM: h2: Remove the tasklet from the
task list if unsubscribing.") uncovered an issue which needs to be
addressed in the scheduler's API. The function task_remove_from_task_list()
was initially designed to remove a task from the running tasklet list from
within the scheduler, and had to be used in h2 to abort pending I/O events.
However this function was not designed to be idempotent, occasionally
causing a double removal from the tasklet list, with the second doing
nothing but affecting the apparent tasks count and making haproxy use
100% CPU on some tests consisting in stopping the client during some
transfers. The h2_unsubscribe() function can sometimes be called upon
stream exit after an error where the tasklet was possibly already
removed, so it.

This patch does 2 things :
  - it renames task_remove_from_task_list() to
    __task_remove_from_tasklet_list() to discourage users from calling
    it. Also note the fix in the naming since it's a tasklet list and
    not a task list. This function is still uesd from the scheduler.
  - it adds a new, idempotent, task_remove_from_tasklet_list() function
    which does nothing if the task is already not in the tasklet list.

This patch will need to be backported where the commit above is backported.
This commit is contained in:
Willy Tarreau 2019-03-25 18:02:54 +01:00
parent 3ea3513689
commit e73256fd2a
3 changed files with 13 additions and 4 deletions

View File

@ -271,7 +271,10 @@ static inline void task_insert_into_tasklet_list(struct task *t)
LIST_ADDQ(&task_per_thread[tid].task_list, &tl->list);
}
static inline void task_remove_from_task_list(struct task *t)
/* remove the task from the tasklet list. The task MUST already be there. If
* unsure, use task_remove_from_task_list() instead.
*/
static inline void __task_remove_from_tasklet_list(struct task *t)
{
LIST_DEL_INIT(&((struct tasklet *)t)->list);
task_per_thread[tid].task_list_size--;
@ -280,6 +283,12 @@ static inline void task_remove_from_task_list(struct task *t)
_HA_ATOMIC_SUB(&tasks_run_queue, 1);
}
static inline void task_remove_from_tasklet_list(struct task *t)
{
if (likely(!LIST_ISEMPTY(&((struct tasklet *)t)->list)))
__task_remove_from_tasklet_list(t);
}
/*
* Unlinks the task and adjusts run queue stats.
* A pointer to the task itself is returned.

View File

@ -5182,7 +5182,7 @@ static int h2_unsubscribe(struct conn_stream *cs, int event_type, void *param)
sw = param;
if (h2s->send_wait == sw) {
sw->events &= ~SUB_CALL_UNSUBSCRIBE;
task_remove_from_task_list((struct task *)h2s->send_wait->task);
task_remove_from_tasklet_list((struct task *)h2s->send_wait->task);
h2s->send_wait = NULL;
LIST_DEL(&h2s->list);
LIST_INIT(&h2s->list);
@ -5270,7 +5270,7 @@ static void h2_stop_senders(struct h2c *h2c)
list_for_each_entry_safe(h2s, h2s_back, &h2c->sending_list, sending_list) {
LIST_DEL_INIT(&h2s->sending_list);
task_remove_from_task_list((struct task *)h2s->send_wait->task);
task_remove_from_tasklet_list((struct task *)h2s->send_wait->task);
h2s->send_wait->events |= SUB_RETRY_SEND;
h2s->send_wait->events &= ~SUB_CALL_UNSUBSCRIBE;
}

View File

@ -416,7 +416,7 @@ void process_runnable_tasks()
t = (struct task *)LIST_ELEM(task_per_thread[tid].task_list.n, struct tasklet *, list);
state = _HA_ATOMIC_XCHG(&t->state, TASK_RUNNING);
__ha_barrier_atomic_store();
task_remove_from_task_list(t);
__task_remove_from_tasklet_list(t);
ctx = t->context;
process = t->process;