sched,nr: simplify metrics extraction from MAC

Allow some level of mutex contention between metrics and worker threads in accessing
internal scheduler variables.

srsenb/hdr/stack/mac/nr/mac_nr.h

@@ -48,6 +48,7 @@ public:
             rrc_interface_mac_nr*   rrc_);
   void stop();
 
+  /// Called from metrics thread.
   void get_metrics(srsenb::mac_metrics_t& metrics);
 
   // MAC interface for RRC
@@ -127,11 +128,6 @@ private:
 
   // Number of rach preambles detected for a CC
   std::vector<uint32_t> detected_rachs;
-
-  // Metrics
-  std::mutex              metrics_mutex;
-  std::condition_variable metrics_condvar;
-  srsenb::mac_metrics_t*  metrics_pending = nullptr;
 };
 
 } // namespace srsenb

srsenb/hdr/stack/mac/nr/sched_nr.h

@@ -51,9 +51,11 @@ public:
   void ul_bsr(uint16_t rnti, uint32_t lcg_id, uint32_t bsr) override;
   void dl_buffer_state(uint16_t rnti, uint32_t lcid, uint32_t newtx, uint32_t retx);
 
-  int run_slot(slot_point pdsch_tti, uint32_t cc, dl_sched_res_t& result, mac_metrics_t* metrics = nullptr) override;
+  int run_slot(slot_point pdsch_tti, uint32_t cc, dl_sched_res_t& result) override;
   int get_ul_sched(slot_point pusch_tti, uint32_t cc, ul_sched_t& result) override;
 
+  void get_metrics(mac_metrics_t& metrics);
+
 private:
   void ue_cfg_impl(uint16_t rnti, const ue_cfg_t& cfg);
 

srsenb/hdr/stack/mac/nr/sched_nr_interface.h

@@ -33,8 +33,6 @@ const static size_t   SCHED_NR_MAX_BWP_PER_CELL = 2;
 const static size_t   SCHED_NR_MAX_LCID         = 32;
 const static size_t   SCHED_NR_MAX_LC_GROUP     = 7;
 
-struct mac_metrics_t;
-
 class sched_nr_interface
 {
 public:
@@ -125,7 +123,7 @@ public:
   virtual void ue_cfg(uint16_t rnti, const ue_cfg_t& ue_cfg)                                                   = 0;
   virtual void ue_rem(uint16_t rnti)                                                                           = 0;
   virtual bool ue_exists(uint16_t rnti)                                                                        = 0;
-  virtual int  run_slot(slot_point slot_rx, uint32_t cc, dl_sched_res_t& result, mac_metrics_t* metrics = nullptr) = 0;
+  virtual int  run_slot(slot_point slot_rx, uint32_t cc, dl_sched_res_t& result)                               = 0;
   virtual int  get_ul_sched(slot_point slot_rx, uint32_t cc, ul_sched_t& result)                               = 0;
 
   virtual void dl_ack_info(uint16_t rnti, uint32_t cc, uint32_t pid, uint32_t tb_idx, bool ack) = 0;

srsenb/hdr/stack/mac/nr/sched_nr_worker.h

@@ -95,7 +95,9 @@ public:
   sched_worker_manager(sched_worker_manager&&)      = delete;
   ~sched_worker_manager();
 
-  void run_slot(slot_point slot_tx, uint32_t cc, dl_sched_res_t& dl_res, ul_sched_t& ul_res, mac_metrics_t* metrics);
+  void run_slot(slot_point slot_tx, uint32_t cc, dl_sched_res_t& dl_res, ul_sched_t& ul_res);
+
+  void get_metrics(mac_metrics_t& metrics);
 
   void enqueue_event(uint16_t rnti, srsran::move_callback<void()> ev);
   void enqueue_cc_event(uint32_t cc, srsran::move_callback<void()> ev);
@@ -106,7 +108,7 @@ public:
 
 private:
   void update_ue_db(slot_point slot_tx, bool locked_context);
-  void get_metrics(mac_metrics_t& metrics);
+  void get_metrics_nolocking(mac_metrics_t& metrics);
   bool save_sched_result(slot_point pdcch_slot, uint32_t cc, dl_sched_res_t& dl_res, ul_sched_t& ul_res);
 
   const sched_params&                               cfg;

srsenb/src/stack/mac/nr/mac_nr.cc

@@ -76,15 +76,13 @@ void mac_nr::stop()
   }
 }
 
-/// Called from metrics thread
+/// Called from metrics thread.
+/// Note: This can contend for the same mutexes as the ones used by L1/L2 workers.
+///       However, get_metrics is called infrequently enough to cause major halts in the L1/L2
 void mac_nr::get_metrics(srsenb::mac_metrics_t& metrics)
 {
-  // Requests asynchronously MAC metrics
+  get_metrics_nolock(metrics);
-  std::unique_lock<std::mutex> lock(metrics_mutex);
+  sched.get_metrics(metrics);
-  metrics_pending = &metrics;
-
-  // Blocks waiting for results
-  metrics_condvar.wait(lock, [this]() { return metrics_pending == nullptr; });
 }
 
 void mac_nr::get_metrics_nolock(srsenb::mac_metrics_t& metrics)
@@ -286,25 +284,11 @@ int mac_nr::get_dl_sched(const srsran_slot_cfg_t& slot_cfg, dl_sched_t& dl_sched
   slot_point                         pdsch_slot = srsran::slot_point{NUMEROLOGY_IDX, slot_cfg.idx};
   sched_nr_interface::dl_sched_res_t dl_res;
 
-  // Get metrics if requested
-  {
-    std::unique_lock<std::mutex> metrics_lock(metrics_mutex);
-    if (metrics_pending != nullptr) {
-      get_metrics_nolock(*metrics_pending);
-    }
-
   // Run Scheduler
-    int ret = sched.run_slot(pdsch_slot, 0, dl_res, metrics_pending);
+  int ret = sched.run_slot(pdsch_slot, 0, dl_res);
-
-    // Notify metrics are filled, if requested
-    if (metrics_pending != nullptr) {
-      metrics_pending = nullptr;
-      metrics_condvar.notify_one();
-    }
   if (ret != SRSRAN_SUCCESS) {
     return ret;
   }
-  }
   dl_sched = dl_res.dl_sched;
 
   uint32_t                  rar_count = 0;

srsenb/src/stack/mac/nr/sched_nr.cc

@@ -124,13 +124,13 @@ void sched_nr::ue_cfg_impl(uint16_t rnti, const ue_cfg_t& uecfg)
 }
 
 /// Generate {pdcch_slot,cc} scheduling decision
-int sched_nr::run_slot(slot_point slot_dl, uint32_t cc, dl_sched_res_t& result, mac_metrics_t* metrics)
+int sched_nr::run_slot(slot_point slot_dl, uint32_t cc, dl_sched_res_t& result)
 {
   // Copy UL results to intermediate buffer
   ul_sched_t& ul_res = pending_results->add_ul_result(slot_dl, cc);
 
   // Generate {slot_idx,cc} result
-  sched_workers->run_slot(slot_dl, cc, result, ul_res, metrics);
+  sched_workers->run_slot(slot_dl, cc, result, ul_res);
 
   return SRSRAN_SUCCESS;
 }
@@ -148,6 +148,11 @@ int sched_nr::get_ul_sched(slot_point slot_ul, uint32_t cc, ul_sched_t& result)
   return SRSRAN_SUCCESS;
 }
 
+void sched_nr::get_metrics(mac_metrics_t& metrics)
+{
+  sched_workers->get_metrics(metrics);
+}
+
 int sched_nr::dl_rach_info(uint32_t cc, const dl_sched_rar_info_t& rar_info)
 {
   sched_workers->enqueue_cc_event(cc, [this, cc, rar_info]() { cells[cc]->bwps[0].ra.dl_rach_info(rar_info); });

srsenb/src/stack/mac/nr/sched_nr_worker.cc

@@ -256,11 +256,7 @@ void sched_worker_manager::update_ue_db(slot_point slot_tx, bool locked_context)
   }
 }
 
-void sched_worker_manager::run_slot(slot_point      slot_tx,
+void sched_worker_manager::run_slot(slot_point slot_tx, uint32_t cc, dl_sched_res_t& dl_res, ul_sched_t& ul_res)
-                                    uint32_t        cc,
-                                    dl_sched_res_t& dl_res,
-                                    ul_sched_t&     ul_res,
-                                    mac_metrics_t*  metrics)
 {
   // Fill DL signalling messages that do not depend on UEs state
   serv_cell_manager& serv_cell = *cells[cc];
@@ -289,11 +285,6 @@ void sched_worker_manager::run_slot(slot_point      slot_tx,
       }
       update_ue_db(slot_tx, true);
 
-      // Obtain MAC metrics if requested
-      if (metrics != nullptr) {
-        get_metrics(*metrics);
-      }
-
       // mark the start of slot. awake remaining workers if locking on the mutex
       current_slot = slot_tx;
       worker_count.store(static_cast<int>(cc_worker_list.size()), std::memory_order_relaxed);
@@ -346,6 +337,12 @@ void sched_worker_manager::run_slot(slot_point      slot_tx,
   save_sched_result(slot_tx, cc, dl_res, ul_res);
 }
 
+void sched_worker_manager::get_metrics(mac_metrics_t& metrics)
+{
+  std::unique_lock<std::mutex> lock(slot_mutex);
+  get_metrics_nolocking(metrics);
+}
+
 bool sched_worker_manager::save_sched_result(slot_point      pdcch_slot,
                                              uint32_t        cc,
                                              dl_sched_res_t& dl_res,
@@ -369,7 +366,7 @@ bool sched_worker_manager::save_sched_result(slot_point      pdcch_slot,
   return true;
 }
 
-void sched_worker_manager::get_metrics(mac_metrics_t& metrics)
+void sched_worker_manager::get_metrics_nolocking(mac_metrics_t& metrics)
 {
   for (mac_ue_metrics_t& ue_metric : metrics.ues) {
     if (ue_db.contains(ue_metric.rnti) and ue_db[ue_metric.rnti]->carriers[0] != nullptr) {

test/phy/dummy_gnb_stack.h

@@ -451,7 +451,8 @@ public:
     valid = true;
   }
 
-  ~gnb_dummy_stack() {}
+  ~gnb_dummy_stack() = default;
+
   bool is_valid() const { return valid; }
 
   int slot_indication(const srsran_slot_cfg_t& slot_cfg) override { return 0; }
@@ -712,7 +713,11 @@ public:
   metrics_t get_metrics()
   {
     std::unique_lock<std::mutex> lock(metrics_mutex);
-
+    if (not use_dummy_mac) {
+      srsenb::mac_metrics_t mac_metrics;
+      mac->get_metrics(mac_metrics);
+      metrics.mac = mac_metrics.ues[0];
+    }
     return metrics;
   }
 };