From b22891d403ee30eb598bfb50bd6061638d346181 Mon Sep 17 00:00:00 2001 From: Cedric Roux Date: Tue, 6 Aug 2024 16:53:03 +0200 Subject: [PATCH] nr rlc: add a lock to protect access to time variables ThreadSanitizer reported a data race, let's remove it. Here is the warning. WARNING: ThreadSanitizer: data race (pid=7361) Write of size 8 at 0x000003b42770 by thread T22 (mutexes: write M0): #0 nr_rlc_tick /home/richie/openairinterface5g/openair2/LAYER2/nr_rlc/nr_rlc_oai_api.c:1101 (nr-softmodem+0x107d275) (BuildId: 291c846fc591e10c3838e9c53f4738997de8f984) #1 gNB_dlsch_ulsch_scheduler /home/richie/openairinterface5g/openair2/LAYER2/NR_MAC_gNB/gNB_scheduler.c:220 (nr-softmodem+0xaec72a) (BuildId: 291c846fc591e10c3838e9c53f4738997de8f984) #2 run_scheduler /home/richie/openairinterface5g/openair2/NR_PHY_INTERFACE/NR_IF_Module.c:406 (nr-softmodem+0xad8515) (BuildId: 291c846fc591e10c3838e9c53f4738997de8f984) #3 tx_func /home/richie/openairinterface5g/executables/nr-gnb.c:134 (nr-softmodem+0x7002cc) (BuildId: 291c846fc591e10c3838e9c53f4738997de8f984) #4 L1_tx_thread /home/richie/openairinterface5g/executables/nr-gnb.c:197 (nr-softmodem+0x7002cc) Previous read of size 8 at 0x000003b42770 by thread T2 (mutexes: write M1): #0 rlc_data_req /home/richie/openairinterface5g/openair2/LAYER2/nr_rlc/nr_rlc_oai_api.c:348 (nr-softmodem+0x1076f4e) (BuildId: 291c846fc591e10c3838e9c53f4738997de8f984) #1 rlc_data_req_thread /home/richie/openairinterface5g/openair2/LAYER2/nr_pdcp/nr_pdcp_oai_api.c:139 (nr-softmodem+0xa671ce) (BuildId: 291c846fc591e10c3838e9c53f4738997de8f984) --- openair2/LAYER2/nr_rlc/nr_rlc_oai_api.c | 42 ++++++++++++++++++++----- 1 file changed, 35 insertions(+), 7 deletions(-) diff --git a/openair2/LAYER2/nr_rlc/nr_rlc_oai_api.c b/openair2/LAYER2/nr_rlc/nr_rlc_oai_api.c index 46b4dad519..46e0da26cf 100644 --- a/openair2/LAYER2/nr_rlc/nr_rlc_oai_api.c +++ b/openair2/LAYER2/nr_rlc/nr_rlc_oai_api.c @@ -50,10 +50,34 @@ extern RAN_CONTEXT_t RC; static nr_rlc_ue_manager_t *nr_rlc_ue_manager; /* TODO: handle time a bit more properly */ +static pthread_mutex_t nr_rlc_current_time_mutex = PTHREAD_MUTEX_INITIALIZER; static uint64_t nr_rlc_current_time; static int nr_rlc_current_time_last_frame; static int nr_rlc_current_time_last_subframe; +void lock_nr_rlc_current_time(void) +{ + if (pthread_mutex_lock(&nr_rlc_current_time_mutex)) + AssertFatal(0, "error locking mutex"); +} + +void unlock_nr_rlc_current_time(void) +{ + if (pthread_mutex_unlock(&nr_rlc_current_time_mutex)) + AssertFatal(0, "error locking mutex"); +} + +static uint64_t get_nr_rlc_current_time(void) +{ + lock_nr_rlc_current_time(); + + uint64_t ret = nr_rlc_current_time; + + unlock_nr_rlc_current_time(); + + return ret; +} + static void release_rlc_entity_from_lcid(nr_rlc_ue_t *ue, logical_chan_id_t channel_id) { AssertFatal(channel_id != 0, "LCID = 0 shouldn't be handled here\n"); @@ -166,7 +190,7 @@ void mac_rlc_data_ind(const module_id_t module_idP, if (rb != NULL) { LOG_D(RLC, "RB found! (channel ID %d) \n", channel_idP); - rb->set_time(rb, nr_rlc_current_time); + rb->set_time(rb, get_nr_rlc_current_time()); rb->recv_pdu(rb, buffer_pP, tb_sizeP); } else { LOG_E(RLC, "Fatal: no RB found (channel ID %d UE ID %d)\n", channel_idP, ue_id); @@ -197,7 +221,7 @@ tbs_size_t mac_rlc_data_req(const module_id_t module_idP, if (rb != NULL) { LOG_D(RLC, "MAC PDU to get created for channel_idP:%d \n", channel_idP); - rb->set_time(rb, nr_rlc_current_time); + rb->set_time(rb, get_nr_rlc_current_time()); maxsize = tb_sizeP; ret = rb->generate_pdu(rb, buffer_pP, maxsize); } else { @@ -233,7 +257,7 @@ mac_rlc_status_resp_t mac_rlc_status_ind(const module_id_t module_idP, if (rb != NULL) { nr_rlc_entity_buffer_status_t buf_stat; - rb->set_time(rb, nr_rlc_current_time); + rb->set_time(rb, get_nr_rlc_current_time()); /* 38.321 deals with BSR values up to 81338368 bytes, after what it * reports '> 81338368' (table 6.1.3.1-2). Passing 100000000 is thus * more than enough. @@ -275,12 +299,14 @@ rlc_buffer_occupancy_t mac_rlc_get_buffer_occupancy_ind(const module_id_t module } /* TODO: handle time a bit more properly */ + lock_nr_rlc_current_time(); if (nr_rlc_current_time_last_frame != frameP || nr_rlc_current_time_last_subframe != subframeP) { nr_rlc_current_time++; nr_rlc_current_time_last_frame = frameP; nr_rlc_current_time_last_subframe = subframeP; } + unlock_nr_rlc_current_time(); nr_rlc_manager_lock(nr_rlc_ue_manager); nr_rlc_ue_t *ue = nr_rlc_manager_get_ue(nr_rlc_ue_manager, ue_id); @@ -288,7 +314,7 @@ rlc_buffer_occupancy_t mac_rlc_get_buffer_occupancy_ind(const module_id_t module if (rb != NULL) { nr_rlc_entity_buffer_status_t buf_stat; - rb->set_time(rb, nr_rlc_current_time); + rb->set_time(rb, get_nr_rlc_current_time()); /* 38.321 deals with BSR values up to 81338368 bytes, after what it * reports '> 81338368' (table 6.1.3.1-2). Passing 100000000 is thus * more than enough. @@ -345,7 +371,7 @@ rlc_op_status_t rlc_data_req(const protocol_ctxt_t *const ctxt_pP, } if (rb != NULL) { - rb->set_time(rb, nr_rlc_current_time); + rb->set_time(rb, get_nr_rlc_current_time()); rb->recv_sdu(rb, (char *)sdu_pP, sdu_sizeP, muiP); } else { LOG_E(RLC, "%s:%d:%s: fatal: SDU sent to unknown RB\n", __FILE__, __LINE__, __FUNCTION__); @@ -1094,12 +1120,14 @@ void nr_rlc_test_trigger_reestablishment(int ue_id) void nr_rlc_tick(int frame, int subframe) { + lock_nr_rlc_current_time(); if (frame != nr_rlc_current_time_last_frame || subframe != nr_rlc_current_time_last_subframe) { nr_rlc_current_time_last_frame = frame; nr_rlc_current_time_last_subframe = subframe; nr_rlc_current_time++; } + unlock_nr_rlc_current_time(); } /* This is a hack, to compile the gNB. @@ -1158,7 +1186,7 @@ bool nr_rlc_get_statistics(int ue_id, int srb_flag, int rb_id, nr_rlc_statistics ret = true; // Patch buffer status using OAI results (no need to change anything in the RB) - // rb->set_time(rb, nr_rlc_current_time); + // rb->set_time(rb, get_nr_rlc_current_time()); nr_rlc_entity_buffer_status_t oai_stat = rb->buffer_status(rb, 1000*1000); out->rxbuf_occ_bytes = oai_stat.status_size; out->txbuf_occ_bytes = oai_stat.tx_size + oai_stat.retx_size; @@ -1187,7 +1215,7 @@ void nr_rlc_srb_recv_sdu(const int ue_id, const logical_chan_id_t channel_id, un AssertFatal(rb != NULL, "SDU sent to unknown RB UE-ID %d SRB %d\n", ue_id, channel_id); - rb->set_time(rb, nr_rlc_current_time); + rb->set_time(rb, get_nr_rlc_current_time()); rb->recv_sdu(rb, (char *)buf, size, -1); nr_rlc_manager_unlock(nr_rlc_ue_manager);