From f53e01cfa366d43f6296726617ec64cda0059ca8 Mon Sep 17 00:00:00 2001 From: Carlo Galiotto Date: Fri, 8 Oct 2021 18:33:05 +0200 Subject: [PATCH] mac: apply changes by PR reviewers - reinstate write_lock on ue metrics - change "rwlock" variable name in mac_nr.h Signed-off-by: Carlo Galiotto --- lib/include/srsran/common/rwlock_guard.h | 8 ++++---- srsenb/hdr/stack/mac/mac.h | 2 +- srsenb/hdr/stack/mac/nr/mac_nr.h | 2 +- srsenb/src/stack/mac/mac.cc | 2 +- srsenb/src/stack/mac/nr/mac_nr.cc | 18 +++++++++--------- srsenb/src/stack/mac/nr/ue_nr.cc | 3 +-- 6 files changed, 17 insertions(+), 18 deletions(-) diff --git a/lib/include/srsran/common/rwlock_guard.h b/lib/include/srsran/common/rwlock_guard.h index 1eb32fad9..7fe87e763 100644 --- a/lib/include/srsran/common/rwlock_guard.h +++ b/lib/include/srsran/common/rwlock_guard.h @@ -31,19 +31,19 @@ private: pthread_rwlock_t* rwlock; }; -// Shared lock guard that automatically unlocks rwmutex on exit +// Shared lock guard that automatically unlocks rwlock on exit class rwlock_read_guard { public: - rwlock_read_guard(pthread_rwlock_t& rwlock_) : rwmutex(&rwlock_) { pthread_rwlock_rdlock(rwmutex); } + rwlock_read_guard(pthread_rwlock_t& rwlock_) : rwlock(&rwlock_) { pthread_rwlock_rdlock(rwlock); } rwlock_read_guard(const rwlock_read_guard&) = delete; rwlock_read_guard(rwlock_read_guard&&) = delete; rwlock_read_guard& operator=(const rwlock_read_guard&) = delete; rwlock_read_guard& operator=(rwlock_read_guard&&) = delete; - ~rwlock_read_guard() { pthread_rwlock_unlock(rwmutex); } + ~rwlock_read_guard() { pthread_rwlock_unlock(rwlock); } private: - pthread_rwlock_t* rwmutex; + pthread_rwlock_t* rwlock; }; } // namespace srsran diff --git a/srsenb/hdr/stack/mac/mac.h b/srsenb/hdr/stack/mac/mac.h index 8790d42a6..cbb7e4af3 100644 --- a/srsenb/hdr/stack/mac/mac.h +++ b/srsenb/hdr/stack/mac/mac.h @@ -112,7 +112,7 @@ private: srslog::basic_logger& logger; - // We use a rwmutex in MAC to allow multiple workers to access MAC simultaneously. No conflicts will happen since + // We use a rwlock in MAC to allow multiple workers to access MAC simultaneously. No conflicts will happen since // access for different TTIs pthread_rwlock_t rwlock = {}; diff --git a/srsenb/hdr/stack/mac/nr/mac_nr.h b/srsenb/hdr/stack/mac/nr/mac_nr.h index 3b44d5bba..e11a0e32f 100644 --- a/srsenb/hdr/stack/mac/nr/mac_nr.h +++ b/srsenb/hdr/stack/mac/nr/mac_nr.h @@ -111,7 +111,7 @@ private: std::vector cell_config; // Map of active UEs - pthread_rwlock_t rwlock = {}; + pthread_rwlock_t rwmutex = {}; static const uint16_t FIRST_RNTI = 0x4601; srsran::static_circular_map, SRSENB_MAX_UES> ue_db; diff --git a/srsenb/src/stack/mac/mac.cc b/srsenb/src/stack/mac/mac.cc index 2136c9ea7..88c919f8f 100644 --- a/srsenb/src/stack/mac/mac.cc +++ b/srsenb/src/stack/mac/mac.cc @@ -996,7 +996,7 @@ void mac::write_mcch(const srsran::sib2_mbms_t* sib2_, rrc_h->add_user(SRSRAN_MRNTI, {}); } -// Internal helper function, caller must hold UE DB rwmutex +// Internal helper function, caller must hold UE DB rwlock bool mac::check_ue_active(uint16_t rnti) { if (not ue_db.contains(rnti)) { diff --git a/srsenb/src/stack/mac/nr/mac_nr.cc b/srsenb/src/stack/mac/nr/mac_nr.cc index a0319dac6..9ee998905 100644 --- a/srsenb/src/stack/mac/nr/mac_nr.cc +++ b/srsenb/src/stack/mac/nr/mac_nr.cc @@ -200,7 +200,7 @@ uint16_t mac_nr::alloc_ue(uint32_t enb_cc_idx) // Pre-check if rnti is valid { - srsran::rwlock_read_guard read_lock(rwlock); + srsran::rwlock_read_guard read_lock(rwmutex); if (not is_rnti_valid_nolock(rnti)) { continue; } @@ -210,7 +210,7 @@ uint16_t mac_nr::alloc_ue(uint32_t enb_cc_idx) std::unique_ptr ue_ptr = std::unique_ptr(new ue_nr(rnti, enb_cc_idx, &sched, rrc, rlc, phy, logger)); // Add UE to rnti map - srsran::rwlock_write_guard rw_lock(rwlock); + srsran::rwlock_write_guard rw_lock(rwmutex); if (not is_rnti_valid_nolock(rnti)) { continue; } @@ -228,7 +228,7 @@ uint16_t mac_nr::alloc_ue(uint32_t enb_cc_idx) // Remove UE from the perspective of L2/L3 int mac_nr::remove_ue(uint16_t rnti) { - srsran::rwlock_write_guard lock(rwlock); + srsran::rwlock_write_guard lock(rwmutex); if (is_rnti_active_nolock(rnti)) { sched.ue_rem(rnti); ue_db.erase(rnti); @@ -292,7 +292,7 @@ int mac_nr::get_dl_sched(const srsran_slot_cfg_t& slot_cfg, dl_sched_t& dl_sched dl_sched = dl_res.dl_sched; uint32_t rar_count = 0; - srsran::rwlock_read_guard rw_lock(rwlock); + srsran::rwlock_read_guard rw_lock(rwmutex); for (pdsch_t& pdsch : dl_sched.pdsch) { if (pdsch.sch.grant.rnti_type == srsran_rnti_type_c) { uint16_t rnti = pdsch.sch.grant.rnti; @@ -330,7 +330,7 @@ int mac_nr::get_ul_sched(const srsran_slot_cfg_t& slot_cfg, ul_sched_t& ul_sched slot_point pusch_slot = srsran::slot_point{NUMEROLOGY_IDX, slot_cfg.idx}; ret = sched.get_ul_sched(pusch_slot, 0, ul_sched); - srsran::rwlock_read_guard rw_lock(rwlock); + srsran::rwlock_read_guard rw_lock(rwmutex); for (auto& pusch : ul_sched.pusch) { if (ue_db.contains(pusch.sch.grant.rnti)) { ue_db[pusch.sch.grant.rnti]->metrics_ul_mcs(pusch.sch.grant.tb->mcs); @@ -355,7 +355,7 @@ bool mac_nr::handle_uci_data(const uint16_t rnti, const srsran_uci_cfg_nr_t& cfg const srsran_harq_ack_bit_t* ack_bit = &cfg_.ack.bits[i]; bool is_ok = (value.ack[i] == 1) and value.valid; sched.dl_ack_info(rnti, 0, ack_bit->pid, 0, is_ok); - srsran::rwlock_read_guard rw_lock(rwlock); + srsran::rwlock_read_guard rw_lock(rwmutex); if (ue_db.contains(rnti)) { ue_db[rnti]->metrics_tx(is_ok, 0 /*TODO get size of packet from scheduler somehow*/); } @@ -367,7 +367,7 @@ bool mac_nr::handle_uci_data(const uint16_t rnti, const srsran_uci_cfg_nr_t& cfg } // Process CQI - srsran::rwlock_read_guard rw_lock(rwlock); + srsran::rwlock_read_guard rw_lock(rwmutex); ue_db[rnti]->metrics_dl_cqi(cfg_, value.csi->wideband_cri_ri_pmi_cqi.cqi, value.valid); return true; @@ -394,7 +394,7 @@ int mac_nr::pusch_info(const srsran_slot_cfg_t& slot_cfg, mac_interface_phy_nr:: } auto process_pdu_task = [this, rnti](srsran::unique_byte_buffer_t& pdu) { - srsran::rwlock_read_guard lock(rwlock); + srsran::rwlock_read_guard lock(rwmutex); if (is_rnti_active_nolock(rnti)) { ue_db[rnti]->process_pdu(std::move(pdu)); } else { @@ -403,7 +403,7 @@ int mac_nr::pusch_info(const srsran_slot_cfg_t& slot_cfg, mac_interface_phy_nr:: }; stack_task_queue.try_push(std::bind(process_pdu_task, std::move(pusch_info.pdu))); } - srsran::rwlock_read_guard rw_lock(rwlock); + srsran::rwlock_read_guard rw_lock(rwmutex); if (ue_db.contains(rnti)) { ue_db[rnti]->metrics_rx(pusch_info.pusch_data.tb[0].crc, nof_bytes); } diff --git a/srsenb/src/stack/mac/nr/ue_nr.cc b/srsenb/src/stack/mac/nr/ue_nr.cc index e0e072b09..a4908ba3f 100644 --- a/srsenb/src/stack/mac/nr/ue_nr.cc +++ b/srsenb/src/stack/mac/nr/ue_nr.cc @@ -196,8 +196,7 @@ void ue_nr::metrics_read(mac_ue_metrics_t* metrics_) void ue_nr::metrics_dl_cqi(const srsran_uci_cfg_nr_t& cfg_, uint32_t dl_cqi, bool valid_cqi) { - // I think this is not necessary, as we locked from the calling function - // std::lock_guard lock(metrics_mutex); + std::lock_guard lock(metrics_mutex); // Process CQI for (uint32_t i = 0; i < cfg_.nof_csi; i++) {