From 997e3344d206198aa55e6c2b4dcbd7748e75c8cb Mon Sep 17 00:00:00 2001
From: Pedro Gonnet <gonnet@google.com>
Date: Tue, 7 Jun 2016 21:39:31 +0200
Subject: [PATCH] update links atomically, makes this thread-safe without
 explicit locking.

---
 src/atomic.h |  1 +
 src/engine.c | 43 ++++++++++++++++---------------------------
 src/engine.h |  1 -
 3 files changed, 17 insertions(+), 28 deletions(-)

diff --git a/src/atomic.h b/src/atomic.h
index 855cd25867..be24f96e5a 100644
--- a/src/atomic.h
+++ b/src/atomic.h
@@ -27,5 +27,6 @@
 #define atomic_inc(v) atomic_add(v, 1)
 #define atomic_dec(v) atomic_add(v, -1)
 #define atomic_cas(v, o, n) __sync_val_compare_and_swap(v, o, n)
+#define atomic_swap(v, n) __sync_lock_test_and_set(v, n)
 
 #endif /* SWIFT_ATOMIC_H */
diff --git a/src/engine.c b/src/engine.c
index d3213cdfe6..5d8962c29a 100644
--- a/src/engine.c
+++ b/src/engine.c
@@ -89,22 +89,24 @@ static cpu_set_t entry_affinity;
  * @brief Link a density/force task to a cell.
  *
  * @param e The #engine.
- * @param l The #link.
+ * @param l A pointer to the #link, will be modified atomically.
  * @param t The #task.
  *
  * @return The new #link pointer.
  */
 
-struct link *engine_addlink(struct engine *e, struct link *l, struct task *t) {
+void engine_addlink(struct engine *e, struct link **l, struct task *t) {
 
+  /* Get the next free link. */
   const int ind = atomic_inc(&e->nr_links);
   if (ind >= e->size_links) {
     error("Link table overflow.");
   }
   struct link *res = &e->links[ind];
-  res->next = l;
+  
+  /* Set it atomically. */
   res->t = t;
-  return res;
+  res->next = atomic_swap(l, res);
 }
 
 /**
@@ -1188,43 +1190,30 @@ void engine_count_and_link_tasks_mapper(void *map_data, void *extra_data) {
       }
 
   /* Link density tasks to cells. */
-  /* TODO(pedro): Instead of locking the cell, re-write engine_addlink to
-     directly modify the second argument using the __atomic_exchange_n
-     intrinsic. */
   if (t->type == task_type_self) {
     atomic_inc(&t->ci->nr_tasks);
     if (t->subtype == task_subtype_density) {
-      lock_lock(&t->ci->lock);
-      t->ci->density = engine_addlink(e, t->ci->density, t);
+      engine_addlink(e, &t->ci->density, t);
       atomic_inc(&t->ci->nr_density);
-      lock_unlock_blind(&t->ci->lock);
     }
   } else if (t->type == task_type_pair) {
     atomic_inc(&t->ci->nr_tasks);
     atomic_inc(&t->cj->nr_tasks);
     if (t->subtype == task_subtype_density) {
-      lock_lock(&t->ci->lock);
-      t->ci->density = engine_addlink(e, t->ci->density, t);
+      engine_addlink(e, &t->ci->density, t);
       atomic_inc(&t->ci->nr_density);
-      lock_unlock_blind(&t->ci->lock);
-      lock_lock(&t->cj->lock);
-      t->cj->density = engine_addlink(e, t->cj->density, t);
+      engine_addlink(e, &t->cj->density, t);
       atomic_inc(&t->cj->nr_density);
-      lock_unlock_blind(&t->cj->lock);
     }
   } else if (t->type == task_type_sub) {
     atomic_inc(&t->ci->nr_tasks);
     if (t->cj != NULL) atomic_inc(&t->cj->nr_tasks);
     if (t->subtype == task_subtype_density) {
-      lock_lock(&t->ci->lock);
-      t->ci->density = engine_addlink(e, t->ci->density, t);
+      engine_addlink(e, &t->ci->density, t);
       atomic_inc(&t->ci->nr_density);
-      lock_unlock_blind(&t->ci->lock);
       if (t->cj != NULL) {
-        lock_lock(&t->cj->lock);
-        t->cj->density = engine_addlink(e, t->cj->density, t);
+        engine_addlink(e, &t->cj->density, t);
         atomic_inc(&t->cj->nr_density);
-        lock_unlock_blind(&t->cj->lock);
       }
     }
   }
@@ -1284,7 +1273,7 @@ void engine_make_extra_hydroloop_tasks_mapper(void *map_data,
         sched, task_type_self, task_subtype_force, 0, 0, t->ci, NULL, 0);
 
     /* Add the link between the new loop and the cell */
-    t->ci->force = engine_addlink(e, t->ci->force, t2);
+    engine_addlink(e, &t->ci->force, t2);
     atomic_inc(&t->ci->nr_force);
 
     /* Now, build all the dependencies for the hydro */
@@ -1299,9 +1288,9 @@ void engine_make_extra_hydroloop_tasks_mapper(void *map_data,
         sched, task_type_pair, task_subtype_force, 0, 0, t->ci, t->cj, 0);
 
     /* Add the link between the new loop and both cells */
-    t->ci->force = engine_addlink(e, t->ci->force, t2);
+    engine_addlink(e, &t->ci->force, t2);
     atomic_inc(&t->ci->nr_force);
-    t->cj->force = engine_addlink(e, t->cj->force, t2);
+    engine_addlink(e, &t->cj->force, t2);
     atomic_inc(&t->cj->nr_force);
 
     /* Now, build all the dependencies for the hydro for the cells */
@@ -1322,10 +1311,10 @@ void engine_make_extra_hydroloop_tasks_mapper(void *map_data,
         sched, task_type_sub, task_subtype_force, t->flags, 0, t->ci, t->cj, 0);
 
     /* Add the link between the new loop and both cells */
-    t->ci->force = engine_addlink(e, t->ci->force, t2);
+    engine_addlink(e, &t->ci->force, t2);
     atomic_inc(&t->ci->nr_force);
     if (t->cj != NULL) {
-      t->cj->force = engine_addlink(e, t->cj->force, t2);
+      engine_addlink(e, &t->cj->force, t2);
       atomic_inc(&t->cj->nr_force);
     }
 
diff --git a/src/engine.h b/src/engine.h
index 392cd9f25c..f1fea143e6 100644
--- a/src/engine.h
+++ b/src/engine.h
@@ -234,7 +234,6 @@ void engine_rebuild(struct engine *e);
 void engine_repartition(struct engine *e);
 void engine_makeproxies(struct engine *e);
 void engine_redistribute(struct engine *e);
-struct link *engine_addlink(struct engine *e, struct link *l, struct task *t);
 void engine_print_policy(struct engine *e);
 int engine_is_done(struct engine *e);
 void engine_pin();
-- 
GitLab