CLEANUP: thread: remove thread_sync_release() and thread_sync_mask

This function was added in 2.0 when reworking the thread isolation
mechanism to make it more reliable. However it if fundamentally
incompatible with the full isolation mechanism provided by
thread_isolate_full() since that one will wait for all threads to
become idle while the former will wait for all threads to finish
waiting, causing a deadlock.

Given that it's not used, let's just drop it entirely before it gets
used by accident.

diff --git a/include/haproxy/thread.h b/include/haproxy/thread.h
index 969e7dc13f5f2c4c461f8d97eb65a3983ded7dfd..d9c280da9c01b83ee47531f6c06756394e32da11 100644 (file)
--- a/include/haproxy/thread.h
+++ b/include/haproxy/thread.h
@@ -59,7 +59,6 @@ enum { all_threads_mask = 1UL };
 enum { all_tgroups_mask = 1UL };
 enum { threads_harmless_mask = 0 };
 enum { threads_idle_mask = 0 };
-enum { threads_sync_mask = 0 };
 enum { threads_want_rdv_mask = 0 };
 enum { tid_bit = 1UL };
 enum { tid = 0 };
@@ -136,10 +135,6 @@ static inline void thread_release()
 {
 }
 
-static inline void thread_sync_release()
-{
-}
-
 static inline unsigned long thread_isolated()
 {
        return 1;
@@ -172,7 +167,6 @@ void thread_harmless_till_end(void);
 void thread_isolate(void);
 void thread_isolate_full(void);
 void thread_release(void);
-void thread_sync_release(void);
 void ha_spin_init(HA_SPINLOCK_T *l);
 void ha_rwlock_init(HA_RWLOCK_T *l);
 void setup_extra_threads(void *(*handler)(void *));
@@ -184,21 +178,16 @@ extern volatile unsigned long all_threads_mask;
 extern volatile unsigned long all_tgroups_mask;
 extern volatile unsigned long threads_harmless_mask;
 extern volatile unsigned long threads_idle_mask;
-extern volatile unsigned long threads_sync_mask;
 extern volatile unsigned long threads_want_rdv_mask;
 extern THREAD_LOCAL unsigned long tid_bit; /* The bit corresponding to the thread id */
 extern THREAD_LOCAL unsigned int tid;      /* The thread id */
 extern THREAD_LOCAL unsigned int tgid;     /* The thread group id (starts at 1) */
 
-/* explanation for threads_want_rdv_mask, threads_harmless_mask, and
- * threads_sync_mask :
+/* explanation for threads_want_rdv_mask, and threads_harmless_mask:
  * - threads_want_rdv_mask is a bit field indicating all threads that have
  *   requested a rendez-vous of other threads using thread_isolate().
  * - threads_harmless_mask is a bit field indicating all threads that are
  *   currently harmless in that they promise not to access a shared resource.
- * - threads_sync_mask is a bit field indicating that a thread waiting for
- *   others to finish wants to leave synchronized with others and as such
- *   promises to do so as well using thread_sync_release().
  *
  * For a given thread, its bits in want_rdv and harmless can be translated like
  * this :
@@ -211,9 +200,6 @@ extern THREAD_LOCAL unsigned int tgid;     /* The thread group id (starts at 1)
  *       1    |     1    | thread interested in RDV and waiting for its turn
  *       1    |     0    | thread currently working isolated from others
  *  ----------+----------+----------------------------------------------------
- *
- * thread_sync_mask only delays the leaving of threads_sync_release() to make
- * sure that each thread's harmless bit is cleared before leaving the function.
  */
 
 #define ha_sigmask(how, set, oldset)  pthread_sigmask(how, set, oldset)

diff --git a/src/thread.c b/src/thread.c
index 7d74858d7db70a092024a0df4d1971cc14b021fc..042ad965edb34ffa78b45eb40798bb2fe6bfdeae 100644 (file)
--- a/src/thread.c
+++ b/src/thread.c
@@ -63,7 +63,6 @@ THREAD_LOCAL struct thread_ctx *th_ctx = &ha_thread_ctx[0];
 volatile unsigned long threads_want_rdv_mask __read_mostly = 0;
 volatile unsigned long threads_harmless_mask = 0;
 volatile unsigned long threads_idle_mask = 0;
-volatile unsigned long threads_sync_mask = 0;
 volatile unsigned long all_threads_mask __read_mostly  = 1; // nbthread 1 assumed by default
 volatile unsigned long all_tgroups_mask __read_mostly  = 1; // nbtgroup 1 assumed by default
 THREAD_LOCAL unsigned int  tgid          = 1; // thread ID starts at 1
@@ -170,34 +169,6 @@ void thread_release()
        _HA_ATOMIC_AND(&threads_want_rdv_mask, ~tid_bit);
 }
 
-/* Cancels the effect of thread_isolate() by releasing the current thread's bit
- * in threads_want_rdv_mask and by marking this thread as harmless until the
- * last worker finishes. The difference with thread_release() is that this one
- * will not leave the function before others are notified to do the same, so it
- * guarantees that the current thread will not pass through a subsequent call
- * to thread_isolate() before others finish.
- */
-void thread_sync_release()
-{
-       _HA_ATOMIC_OR(&threads_sync_mask, tid_bit);
-       __ha_barrier_atomic_store();
-       _HA_ATOMIC_AND(&threads_want_rdv_mask, ~tid_bit);
-
-       while (threads_want_rdv_mask & all_threads_mask) {
-               _HA_ATOMIC_OR(&threads_harmless_mask, tid_bit);
-               while (threads_want_rdv_mask & all_threads_mask)
-                       ha_thread_relax();
-               HA_ATOMIC_AND(&threads_harmless_mask, ~tid_bit);
-       }
-
-       /* the current thread is not harmless anymore, thread_isolate()
-        * is forced to wait till all waiters finish.
-        */
-       _HA_ATOMIC_AND(&threads_sync_mask, ~tid_bit);
-       while (threads_sync_mask & all_threads_mask)
-               ha_thread_relax();
-}
-
 /* Sets up threads, signals and masks, and starts threads 2 and above.
  * Does nothing when threads are disabled.
  */