From 599588ff519f5f447423d75f4d7adbed4c9c36bf Mon Sep 17 00:00:00 2001
From: Francisco Paisana <paisanaf@tcd.ie>
Date: Tue, 24 Mar 2020 18:41:56 +0000
Subject: [PATCH] removed uneeded locks in the rlc, moved ue stack get_metrics
 to stack thread, solved the idle procedure double-locking

---
 lib/src/upper/rlc.cc                  | 14 --------------
 srsue/hdr/stack/ue_stack_lte.h        |  3 ++-
 srsue/src/stack/rrc/rrc_procedures.cc |  3 ++-
 srsue/src/stack/ue_stack_lte.cc       | 14 ++++++++++----
 4 files changed, 14 insertions(+), 20 deletions(-)

diff --git a/lib/src/upper/rlc.cc b/lib/src/upper/rlc.cc
index 629690698..e92c0c529 100644
--- a/lib/src/upper/rlc.cc
+++ b/lib/src/upper/rlc.cc
@@ -82,20 +82,16 @@ void rlc::reset_metrics()
 
 void rlc::stop()
 {
-  pthread_rwlock_rdlock(&rwlock);
   for (rlc_map_t::iterator it = rlc_array.begin(); it != rlc_array.end(); ++it) {
     it->second->stop();
   }
   for (rlc_map_t::iterator it = rlc_array_mrb.begin(); it != rlc_array_mrb.end(); ++it) {
     it->second->stop();
   }
-  pthread_rwlock_unlock(&rwlock);
 }
 
 void rlc::get_metrics(rlc_metrics_t& m)
 {
-  pthread_rwlock_rdlock(&rwlock);
-
   gettimeofday(&metrics_time[2], NULL);
   get_time_interval(metrics_time);
   double secs = (double)metrics_time[0].tv_sec + metrics_time[0].tv_usec * 1e-6;
@@ -120,15 +116,11 @@ void rlc::get_metrics(rlc_metrics_t& m)
 
   memcpy(&metrics_time[1], &metrics_time[2], sizeof(struct timeval));
   reset_metrics();
-
-  pthread_rwlock_unlock(&rwlock);
 }
 
 // Reestablish all RLC bearer
 void rlc::reestablish()
 {
-  pthread_rwlock_rdlock(&rwlock);
-
   for (rlc_map_t::iterator it = rlc_array.begin(); it != rlc_array.end(); ++it) {
     it->second->reestablish();
   }
@@ -136,21 +128,17 @@ void rlc::reestablish()
   for (rlc_map_t::iterator it = rlc_array_mrb.begin(); it != rlc_array_mrb.end(); ++it) {
     it->second->reestablish();
   }
-
-  pthread_rwlock_unlock(&rwlock);
 }
 
 // Reestablish a specific RLC bearer
 void rlc::reestablish(uint32_t lcid)
 {
-  pthread_rwlock_rdlock(&rwlock);
   if (valid_lcid(lcid)) {
     rlc_log->info("Reestablishing LCID %d\n", lcid);
     rlc_array.at(lcid)->reestablish();
   } else {
     rlc_log->warning("RLC LCID %d doesn't exist. Deallocating SDU\n", lcid);
   }
-  pthread_rwlock_unlock(&rwlock);
 }
 
 // Resetting the RLC layer returns the object to the state after the call to init():
@@ -180,11 +168,9 @@ void rlc::reset()
 void rlc::empty_queue()
 {
   // Empty Tx queue, not needed for MCH bearers
-  pthread_rwlock_rdlock(&rwlock);
   for (rlc_map_t::iterator it = rlc_array.begin(); it != rlc_array.end(); ++it) {
     it->second->empty_queue();
   }
-  pthread_rwlock_unlock(&rwlock);
 }
 
 /*******************************************************************************
diff --git a/srsue/hdr/stack/ue_stack_lte.h b/srsue/hdr/stack/ue_stack_lte.h
index f84d5cf60..4bd9393ff 100644
--- a/srsue/hdr/stack/ue_stack_lte.h
+++ b/srsue/hdr/stack/ue_stack_lte.h
@@ -176,7 +176,8 @@ private:
   static const int        STACK_MAIN_THREAD_PRIO = -1; // Use default high-priority below UHD
   srslte::task_multiqueue pending_tasks;
   int sync_queue_id = -1, ue_queue_id = -1, gw_queue_id = -1, mac_queue_id = -1, background_queue_id = -1;
-  srslte::task_thread_pool background_tasks; ///< Thread pool used for long, low-priority tasks
+  srslte::task_thread_pool             background_tasks; ///< Thread pool used for long, low-priority tasks
+  srslte::block_queue<stack_metrics_t> pending_stack_metrics;
 
   // TTI stats
   srslte::tprof<srslte::sliding_window_stats_ms> tti_tprof;
diff --git a/srsue/src/stack/rrc/rrc_procedures.cc b/srsue/src/stack/rrc/rrc_procedures.cc
index 66438b9c2..8a9ccbfbe 100644
--- a/srsue/src/stack/rrc/rrc_procedures.cc
+++ b/srsue/src/stack/rrc/rrc_procedures.cc
@@ -1003,7 +1003,8 @@ proc_outcome_t rrc::go_idle_proc::init()
 {
   Info("Starting...\n");
   rlc_flush_timer.run();
-  return step();
+  // Do not call step() directly. Instead we defer for one TTI to avoid double-locking the RLC mutex
+  return proc_outcome_t::yield;
 }
 
 srslte::proc_outcome_t rrc::go_idle_proc::react(bool timeout)
diff --git a/srsue/src/stack/ue_stack_lte.cc b/srsue/src/stack/ue_stack_lte.cc
index 7c095f657..0b44fa207 100644
--- a/srsue/src/stack/ue_stack_lte.cc
+++ b/srsue/src/stack/ue_stack_lte.cc
@@ -210,10 +210,16 @@ bool ue_stack_lte::disable_data()
 
 bool ue_stack_lte::get_metrics(stack_metrics_t* metrics)
 {
-  mac.get_metrics(metrics->mac);
-  rlc.get_metrics(metrics->rlc);
-  nas.get_metrics(&metrics->nas);
-  rrc.get_metrics(metrics->rrc);
+  // use stack thread to query metrics
+  pending_tasks.try_push(ue_queue_id, [this]() {
+    stack_metrics_t metrics{};
+    mac.get_metrics(metrics.mac);
+    rlc.get_metrics(metrics.rlc);
+    nas.get_metrics(&metrics.nas);
+    rrc.get_metrics(metrics.rrc);
+  });
+  // wait for result
+  *metrics = pending_stack_metrics.wait_pop();
   return (metrics->nas.state == EMM_STATE_REGISTERED && metrics->rrc.state == RRC_STATE_CONNECTED);
 }