From 510e9f47a7f9909b5f9d768e3d3fd0d413dd5e73 Mon Sep 17 00:00:00 2001 From: Francisco Date: Tue, 12 Oct 2021 12:09:06 +0100 Subject: [PATCH] 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 | 6 +--- srsenb/hdr/stack/mac/nr/sched_nr.h | 4 ++- srsenb/hdr/stack/mac/nr/sched_nr_interface.h | 14 ++++---- srsenb/hdr/stack/mac/nr/sched_nr_worker.h | 6 ++-- srsenb/src/stack/mac/nr/mac_nr.cc | 34 ++++++-------------- srsenb/src/stack/mac/nr/sched_nr.cc | 9 ++++-- srsenb/src/stack/mac/nr/sched_nr_worker.cc | 19 +++++------ test/phy/dummy_gnb_stack.h | 9 ++++-- 8 files changed, 45 insertions(+), 56 deletions(-) diff --git a/srsenb/hdr/stack/mac/nr/mac_nr.h b/srsenb/hdr/stack/mac/nr/mac_nr.h index 08af3976a..f0687f27e 100644 --- a/srsenb/hdr/stack/mac/nr/mac_nr.h +++ b/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 detected_rachs; - - // Metrics - std::mutex metrics_mutex; - std::condition_variable metrics_condvar; - srsenb::mac_metrics_t* metrics_pending = nullptr; }; } // namespace srsenb diff --git a/srsenb/hdr/stack/mac/nr/sched_nr.h b/srsenb/hdr/stack/mac/nr/sched_nr.h index 659eb90cf..7ec69419e 100644 --- a/srsenb/hdr/stack/mac/nr/sched_nr.h +++ b/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); diff --git a/srsenb/hdr/stack/mac/nr/sched_nr_interface.h b/srsenb/hdr/stack/mac/nr/sched_nr_interface.h index 4b38a8bc5..19943040b 100644 --- a/srsenb/hdr/stack/mac/nr/sched_nr_interface.h +++ b/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: @@ -121,12 +119,12 @@ public: }; virtual ~sched_nr_interface() = default; - virtual int config(const sched_cfg_t& sched_cfg, srsran::const_span ue_cfg) = 0; - 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 get_ul_sched(slot_point slot_rx, uint32_t cc, ul_sched_t& result) = 0; + virtual int config(const sched_cfg_t& sched_cfg, srsran::const_span ue_cfg) = 0; + 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) = 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; virtual void ul_crc_info(uint16_t rnti, uint32_t cc, uint32_t pid, bool crc) = 0; diff --git a/srsenb/hdr/stack/mac/nr/sched_nr_worker.h b/srsenb/hdr/stack/mac/nr/sched_nr_worker.h index 47ba12c8b..7154414db 100644 --- a/srsenb/hdr/stack/mac/nr/sched_nr_worker.h +++ b/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 ev); void enqueue_cc_event(uint32_t cc, srsran::move_callback 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; diff --git a/srsenb/src/stack/mac/nr/mac_nr.cc b/srsenb/src/stack/mac/nr/mac_nr.cc index edeb2ac1c..b0a82fa8d 100644 --- a/srsenb/src/stack/mac/nr/mac_nr.cc +++ b/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 - std::unique_lock lock(metrics_mutex); - metrics_pending = &metrics; - - // Blocks waiting for results - metrics_condvar.wait(lock, [this]() { return metrics_pending == nullptr; }); + get_metrics_nolock(metrics); + sched.get_metrics(metrics); } void mac_nr::get_metrics_nolock(srsenb::mac_metrics_t& metrics) @@ -286,24 +284,10 @@ 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 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); - - // Notify metrics are filled, if requested - if (metrics_pending != nullptr) { - metrics_pending = nullptr; - metrics_condvar.notify_one(); - } - if (ret != SRSRAN_SUCCESS) { - return ret; - } + // Run Scheduler + int ret = sched.run_slot(pdsch_slot, 0, dl_res); + if (ret != SRSRAN_SUCCESS) { + return ret; } dl_sched = dl_res.dl_sched; diff --git a/srsenb/src/stack/mac/nr/sched_nr.cc b/srsenb/src/stack/mac/nr/sched_nr.cc index 827b4bb85..f1a4ffc2e 100644 --- a/srsenb/src/stack/mac/nr/sched_nr.cc +++ b/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); }); diff --git a/srsenb/src/stack/mac/nr/sched_nr_worker.cc b/srsenb/src/stack/mac/nr/sched_nr_worker.cc index 8b533511a..df32ed526 100644 --- a/srsenb/src/stack/mac/nr/sched_nr_worker.cc +++ b/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, - uint32_t cc, - dl_sched_res_t& dl_res, - ul_sched_t& ul_res, - mac_metrics_t* metrics) +void sched_worker_manager::run_slot(slot_point slot_tx, uint32_t cc, dl_sched_res_t& dl_res, ul_sched_t& ul_res) { // 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(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 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) { diff --git a/test/phy/dummy_gnb_stack.h b/test/phy/dummy_gnb_stack.h index c853b5598..c50533e70 100644 --- a/test/phy/dummy_gnb_stack.h +++ b/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 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; } };