BUG/MEDIUM: task: make tasklets either local or shared but not both at once

Tasklets may be woken up to run on the calling thread or by a specific thread
(the owner). But since we use a non-thread safe mechanism when the calling
thread is also the for the owner, there may sometimes be collisions when two
threads decide to wake the same tasklet up at the same time and one of them
is the owner.

This is more of a matter of usage than code, in that a tasklet usually is
designed to be woken up and executed on the calling thread only (most cases)
or on a specific thread. Thus it is a property of the tasklet itself as this
solely depends how the code is constructed around it.

This patch performs a small change to address this. By default tasklet_new()
creates a "local" tasklet, which will run on the calling thread, like in 2.0.
This is done by setting tl->tid to a negative value. If the caller wants the
tasklet to run exclusively on a specific thread, it just has to set tl->tid,
which is already what shared tasklet callers do anyway.

No backport is needed.
This commit is contained in:
Willy Tarreau 2019-10-18 06:43:53 +02:00
parent bbb5f1d6d2
commit 8cdc167df8
2 changed files with 13 additions and 4 deletions

View File

@ -228,12 +228,14 @@ static inline struct task *task_unlink_rq(struct task *t)
static inline void tasklet_wakeup(struct tasklet *tl)
{
if (tl->tid == tid) {
if (likely(tl->tid < 0)) {
/* this tasklet runs on the caller thread */
if (LIST_ISEMPTY(&tl->list)) {
LIST_ADDQ(&task_per_thread[tl->tid].task_list, &tl->list);
LIST_ADDQ(&task_per_thread[tid].task_list, &tl->list);
_HA_ATOMIC_ADD(&tasks_run_queue, 1);
}
} else {
/* this tasklet runs on a specific thread */
if (MT_LIST_ADDQ(&task_per_thread[tl->tid].shared_tasklet_list, (struct mt_list *)&tl->list) == 1) {
_HA_ATOMIC_ADD(&tasks_run_queue, 1);
if (sleeping_thread_mask & (1UL << tl->tid)) {
@ -292,16 +294,23 @@ static inline struct task *task_init(struct task *t, unsigned long thread_mask)
return t;
}
/* Initialize a new tasklet. It's identified as a tasklet by ->nice=-32768. It
* is expected to run on the calling thread by default, it's up to the caller
* to change ->tid if it wants to own it.
*/
static inline void tasklet_init(struct tasklet *t)
{
t->nice = -32768;
t->calls = 0;
t->state = 0;
t->process = NULL;
t->tid = tid;
t->tid = -1;
LIST_INIT(&t->list);
}
/* Allocate and initialize a new tasklet, local to the thread by default. The
* caller may assing its tid if it wants to own the tasklet.
*/
static inline struct tasklet *tasklet_new(void)
{
struct tasklet *t = pool_alloc(pool_head_tasklet);

View File

@ -97,7 +97,7 @@ struct task {
struct tasklet {
TASK_COMMON; /* must be at the beginning! */
struct list list;
int tid; /* TID of the tasklet owner */
int tid; /* TID of the tasklet owner, <0 if local */
};
#define TASK_IS_TASKLET(t) ((t)->nice == -32768)