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.

diff --git a/include/proto/task.h b/include/proto/task.h
index 6ec2846cdf00b2eb42d5c9bf3919087ca285c5e4..5f2b144a7a0ae73420bd0906748faec3a107771d 100644 (file)
--- a/include/proto/task.h
+++ b/include/proto/task.h
@@ -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);

diff --git a/include/types/task.h b/include/types/task.h
index fbefeef426c42b4729333d990b1ffeceae8c95fe..f5823fe69c852d55fc6793816f09937518c128b9 100644 (file)
--- a/include/types/task.h
+++ b/include/types/task.h
@@ -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)