fixeria has submitted this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/31672 ) Change subject: osmo-bts-trx: alloc/free burst buffers in trx_sched_set_lchan() ...................................................................... osmo-bts-trx: alloc/free burst buffers in trx_sched_set_lchan() It's a lot cleaner if the Rx/Tx burst buffers are allocated and free()d in one place rather than in each lchan handler individually. Allocate the buffer(s) during the lchan activation, free() on deactivation. Regardless of lchan type, allocate the maximim size of 4 * (3 * 2 * 58) bytes, which is sufficient to store 4 8PSK modulated bursts. This way we reduce the load on the memory allocator at the cost of memory consumption, what should not be a big problem. Change-Id: I6a5f76023fc492786076a63016f81285b3576c33 Depends: libosmocore.git I1c38ccc2b64ba9046bb3b1221d99cc55ec493802 Related: OS#1572 --- M src/common/scheduler.c M src/osmo-bts-trx/sched_lchan_pdtch.c M src/osmo-bts-trx/sched_lchan_tchf.c M src/osmo-bts-trx/sched_lchan_tchh.c M src/osmo-bts-trx/sched_lchan_xcch.c 5 files changed, 58 insertions(+), 129 deletions(-) Approvals: lynxis lazus: Looks good to me, but someone else must approve Jenkins Builder: Verified laforge: Looks good to me, approved diff --git a/src/common/scheduler.c b/src/common/scheduler.c index 25aefe8..cacae6c 100644 --- a/src/common/scheduler.c +++ b/src/common/scheduler.c @@ -35,6 +35,7 @@ #include <osmocom/core/stats.h> #include <osmocom/gsm/protocol/gsm_08_58.h> +#include <osmocom/gsm/gsm0502.h> #include <osmocom/gsm/a5.h> #include <osmo-bts/gsm_data.h> @@ -542,9 +543,7 @@ .chan_nr = RSL_CHAN_OSMO_PDCH, /* Rx and Tx, multiple coding schemes: CS-2..4 and MCS-1..9 (3GPP TS - * 05.03, chapter 5), regular interleaving as specified for xCCH. - * NOTE: the burst buffer is three times bigger because the - * payload of EDGE bursts is three times longer. */ + * 05.03, chapter 5), regular interleaving as specified for xCCH. */ .rts_fn = rts_data_fn, .dl_fn = tx_pdtch_fn, .ul_fn = rx_pdtch_fn, @@ -1070,26 +1069,24 @@ (active) ? "Activating" : "Deactivating", trx_chan_desc[chan].name); - /* free burst memory, to cleanly start with burst 0 */ - if (chan_state->dl_bursts) { - talloc_free(chan_state->dl_bursts); - chan_state->dl_bursts = NULL; - } - if (chan_state->ul_bursts) { - talloc_free(chan_state->ul_bursts); - chan_state->ul_bursts = NULL; - } - if (chan_state->ul_bursts_prev) { - talloc_free(chan_state->ul_bursts_prev); - chan_state->ul_bursts_prev = NULL; - } - if (active) { /* Clean up everything */ memset(chan_state, 0, sizeof(*chan_state)); /* Bind to generic 'struct gsm_lchan' */ chan_state->lchan = lchan; + + /* Allocate memory for Rx/Tx burst buffers. Use the maximim size + * of 4 * (3 * 2 * 58) bytes, which is sufficient to store 4 8PSK + * modulated bursts. */ + const size_t buf_size = 4 * GSM_NBITS_NB_8PSK_PAYLOAD; + if (trx_chan_desc[chan].dl_fn != NULL) + chan_state->dl_bursts = talloc_zero_size(l1ts, buf_size); + if (trx_chan_desc[chan].ul_fn != NULL) { + chan_state->ul_bursts = talloc_zero_size(l1ts, buf_size); + if (L1SAP_IS_LINK_SACCH(trx_chan_desc[chan].link_id)) + chan_state->ul_bursts_prev = talloc_zero_size(l1ts, buf_size); + } } else { chan_state->ho_rach_detect = 0; @@ -1097,6 +1094,11 @@ trx_sched_queue_filter(&l1ts->dl_prims, trx_chan_desc[chan].chan_nr, trx_chan_desc[chan].link_id); + + /* Release memory used by Rx/Tx burst buffers */ + TALLOC_FREE(chan_state->dl_bursts); + TALLOC_FREE(chan_state->ul_bursts); + TALLOC_FREE(chan_state->ul_bursts_prev); } chan_state->active = active; diff --git a/src/osmo-bts-trx/sched_lchan_pdtch.c b/src/osmo-bts-trx/sched_lchan_pdtch.c index 8300bdb..ddb4478 100644 --- a/src/osmo-bts-trx/sched_lchan_pdtch.c +++ b/src/osmo-bts-trx/sched_lchan_pdtch.c @@ -60,13 +60,6 @@ if (bi->flags & TRX_BI_F_ACCESS_BURST) return rx_rach_fn(l1ts, bi); - /* allocate burst memory, if not already */ - if (!*bursts_p) { - *bursts_p = talloc_zero_size(l1ts, GSM0503_EGPRS_BURSTS_NBITS); - if (!*bursts_p) - return -ENOMEM; - } - /* clear burst */ if (bi->bid == 0) { memset(*bursts_p, 0, GSM0503_EGPRS_BURSTS_NBITS); @@ -156,11 +149,8 @@ int rc = 0; /* send burst, if we already got a frame */ - if (br->bid > 0) { - if (!*bursts_p) - return -ENODEV; + if (br->bid > 0) goto send_burst; - } /* get mac block from queue */ msg = _sched_dequeue_prim(l1ts, br); @@ -170,18 +160,11 @@ * take care of filling C0 with dummy bursts to keep expected * power transmit levels */ - goto no_msg; + return -ENODEV; } /* BURST BYPASS */ - /* allocate burst memory, if not already */ - if (!*bursts_p) { - *bursts_p = talloc_zero_size(l1ts, GSM0503_EGPRS_BURSTS_NBITS); - if (!*bursts_p) - return -ENOMEM; - } - /* encode bursts */ rc = gsm0503_pdtch_egprs_encode(*bursts_p, msg->l2h, msg->tail - msg->l2h); if (rc < 0) @@ -193,7 +176,7 @@ "(len=%ld)\n", (long)(msg->tail - msg->l2h)); /* free message */ msgb_free(msg); - goto no_msg; + return -EINVAL; } else if (rc == GSM0503_EGPRS_BURSTS_NBITS) { *mod = TRX_MOD_T_8PSK; } else { @@ -226,12 +209,4 @@ LOGL1SB(DL1P, LOGL_DEBUG, l1ts, br, "Transmitting burst=%u.\n", br->bid); return 0; - -no_msg: - /* free burst memory */ - if (*bursts_p) { - talloc_free(*bursts_p); - *bursts_p = NULL; - } - return -ENODEV; } diff --git a/src/osmo-bts-trx/sched_lchan_tchf.c b/src/osmo-bts-trx/sched_lchan_tchf.c index 0d0f52a..3e6bdc2 100644 --- a/src/osmo-bts-trx/sched_lchan_tchf.c +++ b/src/osmo-bts-trx/sched_lchan_tchf.c @@ -99,13 +99,6 @@ LOGL1SB(DL1P, LOGL_DEBUG, l1ts, bi, "Received TCH/F, bid=%u\n", bi->bid); - /* allocate burst memory, if not already */ - if (!*bursts_p) { - *bursts_p = talloc_zero_size(l1ts, 928); - if (!*bursts_p) - return -ENOMEM; - } - /* shift the buffer by 4 bursts leftwards */ if (bi->bid == 0) { memcpy(*bursts_p, *bursts_p + 464, 464); @@ -460,24 +453,14 @@ struct msgb *msg; /* send burst, if we already got a frame */ - if (br->bid > 0) { - if (!*bursts_p) - return -ENODEV; + if (br->bid > 0) goto send_burst; - } /* BURST BYPASS */ - /* allocate burst memory, if not already, - * otherwise shift buffer by 4 bursts for interleaving */ - if (!*bursts_p) { - *bursts_p = talloc_zero_size(l1ts, 928); - if (!*bursts_p) - return -ENOMEM; - } else { - memcpy(*bursts_p, *bursts_p + 464, 464); - memset(*bursts_p + 464, 0, 464); - } + /* shift buffer by 4 bursts for interleaving */ + memcpy(*bursts_p, *bursts_p + 464, 464); + memset(*bursts_p + 464, 0, 464); /* dequeue a message to be transmitted */ msg = tch_dl_dequeue(l1ts, br); diff --git a/src/osmo-bts-trx/sched_lchan_tchh.c b/src/osmo-bts-trx/sched_lchan_tchh.c index c6c2cd0..f464b56 100644 --- a/src/osmo-bts-trx/sched_lchan_tchh.c +++ b/src/osmo-bts-trx/sched_lchan_tchh.c @@ -122,13 +122,6 @@ LOGL1SB(DL1P, LOGL_DEBUG, l1ts, bi, "Received TCH/H, bid=%u\n", bi->bid); - /* allocate burst memory, if not already */ - if (!*bursts_p) { - *bursts_p = talloc_zero_size(l1ts, 696); - if (!*bursts_p) - return -ENOMEM; - } - /* shift the buffer by 2 bursts leftwards */ if (bi->bid == 0) { memcpy(*bursts_p, *bursts_p + 232, 232); @@ -361,28 +354,18 @@ struct msgb *msg; /* send burst, if we already got a frame */ - if (br->bid > 0) { - if (!*bursts_p) - return -ENODEV; + if (br->bid > 0) goto send_burst; - } /* BURST BYPASS */ - /* allocate burst memory, if not already, - * otherwise shift buffer by 2 bursts for interleaving */ - if (!*bursts_p) { - *bursts_p = talloc_zero_size(l1ts, 696); - if (!*bursts_p) - return -ENOMEM; + /* shift buffer by 2 bursts for interleaving */ + memcpy(*bursts_p, *bursts_p + 232, 232); + if (chan_state->dl_ongoing_facch) { + memcpy(*bursts_p + 232, *bursts_p + 464, 232); + memset(*bursts_p + 464, 0, 232); } else { - memcpy(*bursts_p, *bursts_p + 232, 232); - if (chan_state->dl_ongoing_facch) { - memcpy(*bursts_p + 232, *bursts_p + 464, 232); - memset(*bursts_p + 464, 0, 232); - } else { - memset(*bursts_p + 232, 0, 232); - } + memset(*bursts_p + 232, 0, 232); } /* dequeue a message to be transmitted */ diff --git a/src/osmo-bts-trx/sched_lchan_xcch.c b/src/osmo-bts-trx/sched_lchan_xcch.c index e936621..786b509 100644 --- a/src/osmo-bts-trx/sched_lchan_xcch.c +++ b/src/osmo-bts-trx/sched_lchan_xcch.c @@ -69,21 +69,6 @@ LOGL1SB(DL1P, LOGL_DEBUG, l1ts, bi, "Received Data, bid=%u\n", bi->bid); - /* allocate burst memory, if not already */ - if (!*bursts_p) { - *bursts_p = talloc_zero_size(l1ts, 464); - if (!*bursts_p) - return -ENOMEM; - } - - /* UL-SACCH requires additional memory to keep a copy of each previous - * burst set. */ - if (L1SAP_IS_LINK_SACCH(trx_chan_desc[bi->chan].link_id) && !chan_state->ul_bursts_prev) { - chan_state->ul_bursts_prev = talloc_zero_size(l1ts, 464); - if (!chan_state->ul_bursts_prev) - return -ENOMEM; - } - /* clear burst & store frame number of first burst */ if (bi->bid == 0) { memset(*bursts_p, 0, 464); @@ -174,46 +159,27 @@ ubit_t *burst, **bursts_p = &l1ts->chan_state[br->chan].dl_bursts; /* send burst, if we already got a frame */ - if (br->bid > 0) { - if (!*bursts_p) - return -ENODEV; + if (br->bid > 0) goto send_burst; - } /* get mac block from queue */ msg = _sched_dequeue_prim(l1ts, br); - if (msg) - goto got_msg; - - LOGL1SB(DL1P, LOGL_INFO, l1ts, br, "No prim for transmit.\n"); - -no_msg: - /* free burst memory */ - if (*bursts_p) { - talloc_free(*bursts_p); - *bursts_p = NULL; + if (msg == NULL) { + LOGL1SB(DL1P, LOGL_INFO, l1ts, br, "No prim for transmit.\n"); + return -ENODEV; } - return -ENODEV; -got_msg: /* check validity of message */ if (msgb_l2len(msg) != GSM_MACBLOCK_LEN) { LOGL1SB(DL1P, LOGL_FATAL, l1ts, br, "Prim has odd len=%u != %u\n", msgb_l2len(msg), GSM_MACBLOCK_LEN); /* free message */ msgb_free(msg); - goto no_msg; + return -EINVAL; } /* BURST BYPASS */ - /* allocate burst memory, if not already */ - if (!*bursts_p) { - *bursts_p = talloc_zero_size(l1ts, 464); - if (!*bursts_p) - return -ENOMEM; - } - /* encode bursts */ gsm0503_xcch_encode(*bursts_p, msg->l2h); -- To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/31672 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-Change-Id: I6a5f76023fc492786076a63016f81285b3576c33 Gerrit-Change-Number: 31672 Gerrit-PatchSet: 8 Gerrit-Owner: fixeria &lt;vyanitskiy(a)sysmocom.de&gt; Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: falconia &lt;falcon(a)freecalypso.org&gt; Gerrit-Reviewer: fixeria &lt;vyanitskiy(a)sysmocom.de&gt; Gerrit-Reviewer: laforge &lt;laforge(a)osmocom.org&gt; Gerrit-Reviewer: lynxis lazus &lt;lynxis(a)fe80.eu&gt; Gerrit-Reviewer: msuraev &lt;msuraev(a)sysmocom.de&gt; Gerrit-Reviewer: pespin &lt;pespin(a)sysmocom.de&gt; Gerrit-MessageType: merged