radioInterface: Atomically fetch and change underrun variable

Otherwise, it could happen that underrun events are lost:
TxLower (isUnderrun):	RxLower (pullBuffer):
read(underrun)
			read(underrun)
			write(underrun, |val) [maybe underrun becomes TRUE]
write(underrun, false)

Similary, it could happen the other direction if atomic was only applied
to isUnderrun:
TxLower (isUnderrun):	RxLower (pullBuffer):
			read(underrun) -> true
read(underrun)-> true
write(underrun, false)
			write(underrun, true|val) where val=false

So in here isUnderrun would return true twice while it should only
return one.

Change-Id: I684e0a5d2a9583a161d5a6593559b3a9e7cd57e3

8 files changed, 62 insertions, 20 deletions

diff --git a/CommonLibs/Threads.cpp b/CommonLibs/Threads.cpp
index dd57d40..020d94e 100644
--- a/CommonLibs/Threads.cpp
+++ b/CommonLibs/Threads.cpp
@@ -40,7 +40,9 @@
 
 using namespace std;
 
-
+#ifndef HAVE_ATOMIC_OPS
+	pthread_mutex_t atomic_ops_mutex = PTHREAD_MUTEX_INITIALIZER;
+#endif
 
 
 Mutex gStreamLock;		///< Global lock to control access to cout and cerr.
diff --git a/CommonLibs/Threads.h b/CommonLibs/Threads.h
index 4cc0884..df61c72 100644
--- a/CommonLibs/Threads.h
+++ b/CommonLibs/Threads.h
@@ -28,6 +28,8 @@
 #ifndef THREADS_H
 #define THREADS_H
 
+#include "config.h"
+
 #include <pthread.h>
 #include <iostream>
 #include <assert.h>
@@ -188,8 +190,30 @@ class Thread {
 	void cancel() { pthread_cancel(mThread); }
 };
 
-
-
+#ifdef HAVE_ATOMIC_OPS
+#define osmo_trx_sync_fetch_and_and(ptr, value) __sync_fetch_and_and((ptr), (value))
+#define osmo_trx_sync_or_and_fetch(ptr, value) __sync_or_and_fetch((ptr), (value))
+#else
+extern pthread_mutex_t atomic_ops_mutex;
+static inline int osmo_trx_sync_fetch_and_and(int *ptr, int value)
+{
+	pthread_mutex_lock(&atomic_ops_mutex);
+	int tmp = *ptr;
+	*ptr &= value;
+	pthread_mutex_unlock(&atomic_ops_mutex);
+	return tmp;
+}
+
+static inline int osmo_trx_sync_or_and_fetch(int *ptr, int value)
+{
+	int tmp;
+	pthread_mutex_lock(&atomic_ops_mutex);
+	*ptr |= value;
+	tmp = *ptr;
+	pthread_mutex_unlock(&atomic_ops_mutex);
+	return tmp;
+}
+#endif
 
 #endif
 // vim: ts=4 sw=4
diff --git a/Transceiver52M/osmo-trx.cpp b/Transceiver52M/osmo-trx.cpp
index ab0b631..9fe6585 100644
--- a/Transceiver52M/osmo-trx.cpp
+++ b/Transceiver52M/osmo-trx.cpp
@@ -571,6 +571,11 @@ int main(int argc, char *argv[])
 #endif
 #endif
 
+#ifndef HAVE_ATOMIC_OPS
+#pragma message ("Built without atomic operation support. Using Mutex, it may affect performance!")
+	printf("Built without atomic operation support. Using Mutex, it may affect performance!\n");
+#endif
+
 	if (!log_mutex_init()) {
 		fprintf(stderr, "Failed to initialize log mutex!\n");
 		exit(2);
diff --git a/Transceiver52M/radioInterface.cpp b/Transceiver52M/radioInterface.cpp
index 6e49a75..fbcacf1 100644
--- a/Transceiver52M/radioInterface.cpp
+++ b/Transceiver52M/radioInterface.cpp
@@ -24,6 +24,7 @@
 #include "radioInterface.h"
 #include "Resampler.h"
 #include <Logger.h>
+#include <Threads.h>
 
 extern "C" {
 #include "convert.h"
@@ -288,9 +289,9 @@ int RadioInterface::driveReceiveRadio()
 
 bool RadioInterface::isUnderrun()
 {
-  bool retVal = underrun;
-  underrun = false;
-
+  bool retVal;
+  /* atomically get previous value of "underrun" and set the var to false */
+  retVal = osmo_trx_sync_fetch_and_and(&underrun, false);
   return retVal;
 }
 
@@ -340,7 +341,7 @@ int RadioInterface::pullBuffer()
 			segmentLen * 2);
   }
 
-  underrun |= local_underrun;
+  osmo_trx_sync_or_and_fetch(&underrun, local_underrun);
   readTimestamp += numRecv;
   return 0;
 }
@@ -366,7 +367,7 @@ bool RadioInterface::pushBuffer()
                                  segmentLen,
                                  &local_underrun,
                                  writeTimestamp);
-  underrun |= local_underrun;
+  osmo_trx_sync_or_and_fetch(&underrun, local_underrun);
   writeTimestamp += numSent;
 
   return true;
diff --git a/Transceiver52M/radioInterface.h b/Transceiver52M/radioInterface.h
index b12c187..d72fb69 100644
--- a/Transceiver52M/radioInterface.h
+++ b/Transceiver52M/radioInterface.h
@@ -48,7 +48,7 @@ protected:
   std::vector<short *> convertRecvBuffer;
   std::vector<short *> convertSendBuffer;
   std::vector<float> powerScaling;
-  bool underrun;			      ///< indicates writes to USRP are too slow
+  int underrun;			              ///< indicates writes to USRP are too slow
   bool overrun;				      ///< indicates reads from USRP are too slow
   TIMESTAMP writeTimestamp;		      ///< sample timestamp of next packet written to USRP
   TIMESTAMP readTimestamp;		      ///< sample timestamp of next packet read from USRP
diff --git a/Transceiver52M/radioInterfaceMulti.cpp b/Transceiver52M/radioInterfaceMulti.cpp
index eec426e..92e31e1 100644
--- a/Transceiver52M/radioInterfaceMulti.cpp
+++ b/Transceiver52M/radioInterfaceMulti.cpp
@@ -251,7 +251,7 @@ int RadioInterfaceMulti::pullBuffer()
 	convert_short_float((float *) outerRecvBuffer->begin(),
 			    convertRecvBuffer[0], 2 * outerRecvBuffer->size());
 
-	underrun |= local_underrun;
+	osmo_trx_sync_or_and_fetch(&underrun, local_underrun);
 	readTimestamp += num;
 
 	channelizer->rotate((float *) outerRecvBuffer->begin(),
@@ -348,7 +348,7 @@ bool RadioInterfaceMulti::pushBuffer()
 		LOG(ALERT) << "Transmit error " << num;
 	}
 
-	underrun |= local_underrun;
+	osmo_trx_sync_or_and_fetch(&underrun, local_underrun);
 	writeTimestamp += num;
 
 	return true;
diff --git a/Transceiver52M/radioInterfaceResamp.cpp b/Transceiver52M/radioInterfaceResamp.cpp
index 864cdee..b92432f 100644
--- a/Transceiver52M/radioInterfaceResamp.cpp
+++ b/Transceiver52M/radioInterfaceResamp.cpp
@@ -184,7 +184,7 @@ int RadioInterfaceResamp::pullBuffer()
 	convert_short_float((float *) outerRecvBuffer->begin(),
 			    convertRecvBuffer[0], 2 * resamp_outchunk);
 
-	underrun |= local_underrun;
+	osmo_trx_sync_or_and_fetch(&underrun, local_underrun);
 	readTimestamp += (TIMESTAMP) resamp_outchunk;
 
 	/* Write to the end of the inner receive buffer */
@@ -232,7 +232,7 @@ bool RadioInterfaceResamp::pushBuffer()
 		LOG(ALERT) << "Transmit error " << numSent;
 	}
 
-	underrun |= local_underrun;
+	osmo_trx_sync_or_and_fetch(&underrun, local_underrun);
 	writeTimestamp += resamp_outchunk;
 
 	return true;
diff --git a/configure.ac b/configure.ac
index fa9cf8d..350c77c 100644
--- a/configure.ac
+++ b/configure.ac
@@ -190,29 +190,39 @@ dnl Check if the compiler supports specified GCC's built-in function
 AC_DEFUN([CHECK_BUILTIN_SUPPORT], [
   AC_CACHE_CHECK(
     [whether ${CC} has $1 built-in],
-    [osmo_cv_cc_has_builtin], [
+    [osmo_cv_cc_has_$1], [
       AC_LINK_IFELSE([
         AC_LANG_PROGRAM([], [
-          __builtin_cpu_supports("sse");
+          $2
         ])
       ],
-      [AS_VAR_SET([osmo_cv_cc_has_builtin], [yes])],
-      [AS_VAR_SET([osmo_cv_cc_has_builtin], [no])])
+      [AS_VAR_SET([osmo_cv_cc_has_$1], [yes])],
+      [AS_VAR_SET([osmo_cv_cc_has_$1], [no])])
     ]
   )
 
-  AS_IF([test yes = AS_VAR_GET([osmo_cv_cc_has_builtin])], [
+  AS_IF([test yes = AS_VAR_GET([osmo_cv_cc_has_$1])], [
     AC_DEFINE_UNQUOTED(AS_TR_CPP(HAVE_$1), 1,
       [Define to 1 if compiler has the '$1' built-in function])
   ], [
-    AC_MSG_WARN($2)
+    AC_MSG_WARN($3)
   ])
 ])
 
 dnl Check if the compiler supports runtime SIMD detection
-CHECK_BUILTIN_SUPPORT([__builtin_cpu_supports],
+CHECK_BUILTIN_SUPPORT([__builtin_cpu_supports], [__builtin_cpu_supports("sse");],
   [Runtime SIMD detection will be disabled])
 
+dnl Check for __sync_fetch_and_add().
+CHECK_BUILTIN_SUPPORT([__sync_fetch_and_and], [int x;__sync_fetch_and_and(&x,1);],
+  [Atomic operation not available, will use mutex])
+dnl Check for __sync_or_and_fetch().
+CHECK_BUILTIN_SUPPORT([__sync_or_and_fetch], [int x;__sync_or_and_fetch(&x,1);],
+  [Atomic operation not available, will use mutex])
+AS_IF([test "x$osmo_cv_cc_has___sync_fetch_and_and" = "xyes" && test "x$osmo_cv_cc_has___sync_or_and_fetch" = "xyes"], [
+    AC_DEFINE(HAVE_ATOMIC_OPS, 1, [Support all required atomic operations], [AC_MSG_WARN("At least one aotmic operation missing, will use mutex")])
+])
+
 AM_CONDITIONAL(DEVICE_UHD, [test "x$with_uhd" != "xno"])
 AM_CONDITIONAL(DEVICE_USRP1, [test "x$with_usrp1" = "xyes"])
 AM_CONDITIONAL(DEVICE_LMS, [test "x$with_lms" = "xyes"])