[PATCH] osmo-pcu[master]: Simplify TS alloc: improve readability

This is merely a historical archive of years 2008-2021, before the migration to mailman3.

A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.

Max gerrit-no-reply at lists.osmocom.org
Thu Sep 14 13:54:46 UTC 2017


Hello Neels Hofmeyr, Jenkins Builder,

I'd like you to reexamine a change.  Please visit

    https://gerrit.osmocom.org/3760

to look at the new patch set (#15).

Simplify TS alloc: improve readability

* consistently format log messages to make it possible to grep for test
  output in source code
* move TRX check inside local tfi_find_free() wrapper
* drop unused code
* assign reserved_*_slots only when multislot masks are found

Change-Id: I02da2b8ba8c9c8815dae0e39e1fed277ca0df171
Related: OS#2282
---
M src/gprs_rlcmac_ts_alloc.cpp
1 file changed, 25 insertions(+), 90 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/osmo-pcu refs/changes/60/3760/15

diff --git a/src/gprs_rlcmac_ts_alloc.cpp b/src/gprs_rlcmac_ts_alloc.cpp
index 76a84c7..a3a9670 100644
--- a/src/gprs_rlcmac_ts_alloc.cpp
+++ b/src/gprs_rlcmac_ts_alloc.cpp
@@ -248,9 +248,7 @@
 			if (free_tfi) {
 				tfi = find_free_tfi(pdch, dir);
 				if (tfi < 0) {
-					LOGP(DRLCMAC, LOGL_DEBUG,
-						"- Skipping TS %d, because "
-						"no TFI available\n", ts);
+					LOGP(DRLCMAC, LOGL_DEBUG, "- Skipping TS %d, because no TFI available\n", ts);
 					continue;
 				}
 			}
@@ -258,26 +256,20 @@
 			if (dir == GPRS_RLCMAC_UL_TBF) {
 				usf = find_free_usf(pdch);
 				if (usf < 0) {
-					LOGP(DRLCMAC, LOGL_DEBUG,
-						"- Skipping TS %d, because "
-						"no USF available\n", ts);
+					LOGP(DRLCMAC, LOGL_DEBUG, "- Skipping TS %d, because no USF available\n", ts);
 					continue;
 				}
 			}
 			if (min_ts >= 0)
-				LOGP(DRLCMAC, LOGL_DEBUG,
-					"- Skipping TS %d, because "
-					"num TBFs %d > %d\n",
-					min_ts, min_used, num_tbfs);
+				LOGP(DRLCMAC, LOGL_DEBUG, "- Skipping TS %d, because num TBFs %d > %d\n",
+				     min_ts, min_used, num_tbfs);
 			min_used = num_tbfs;
 			min_ts = ts;
 			min_tfi = tfi;
 			min_usf = usf;
 		} else {
-			LOGP(DRLCMAC, LOGL_DEBUG,
-				"- Skipping TS %d, because "
-				"num TBFs %d >= %d\n",
-				ts, num_tbfs, min_used);
+			LOGP(DRLCMAC, LOGL_DEBUG, "- Skipping TS %d, because num TBFs %d >= %d\n",
+			     ts, num_tbfs, min_used);
 		}
 	}
 
@@ -374,7 +366,7 @@
 /*! Return free TFI
  *
  *  \param[in] bts Pointer to BTS struct
- *  \param[in] trx Pointer to TRX struct
+ *  \param[in] trx Optional pointer to TRX struct
  *  \param[in] ms Pointer to MS object
  *  \param[in] dir DL or UL direction
  *  \param[in] use_trx which TRX to use or -1 if it should be selected based on what MS uses
@@ -386,6 +378,15 @@
 {
 	int tfi;
 	uint8_t trx_no;
+
+	if (trx) {
+		if (use_trx >= 0 && use_trx != trx->trx_no) {
+			LOGP(DRLCMAC, LOGL_ERROR, "- Requested incompatible TRX %d (current is %d)\n",
+			     use_trx, trx->trx_no);
+			return -EINVAL;
+		}
+		use_trx = trx->trx_no;
+	}
 
 	if (use_trx == -1 && ms->current_trx())
 		use_trx = ms->current_trx()->trx_no;
@@ -532,17 +533,14 @@
 
 	if (ms->ms_class()) {
 		ms_class = &gprs_ms_multislot_class[ms->ms_class()];
-		LOGP(DRLCMAC, LOGL_DEBUG, "Slot Allocation (Algorithm B) for "
-			"class %d\n", ms->ms_class());
+		LOGP(DRLCMAC, LOGL_DEBUG, "Slot Allocation (Algorithm B) for class %d\n", ms->ms_class());
 	} else {
 		ms_class = &gprs_ms_multislot_class[12];
-		LOGP(DRLCMAC, LOGL_DEBUG, "Slot Allocation (Algorithm B) for "
-			"unknown class (assuming 12)\n");
+		LOGP(DRLCMAC, LOGL_DEBUG, "Slot Allocation (Algorithm B) for unknown class (assuming 12)\n");
 	}
 
 	if (ms_class->tx == MS_NA) {
-		LOGP(DRLCMAC, LOGL_NOTICE, "Multislot class %d not "
-			"applicable.\n", ms->ms_class());
+		LOGP(DRLCMAC, LOGL_NOTICE, "Multislot class %d not applicable.\n", ms->ms_class());
 		return -EINVAL;
 	}
 
@@ -569,8 +567,7 @@
 	if (Trb == MS_C)
 		Trb = 1;
 
-	LOGP(DRLCMAC, LOGL_DEBUG, "- Rx=%d Tx=%d Sum Rx+Tx=%s  Tta=%s Ttb=%d "
-		" Tra=%d Trb=%d Type=%d\n", ms_class->rx, Tx,
+	LOGP(DRLCMAC, LOGL_DEBUG, "- Rx=%d Tx=%d Sum Rx+Tx=%s  Tta=%s Ttb=%d  Tra=%d Trb=%d Type=%d\n", ms_class->rx, Tx,
 		(Sum == MS_NA) ? "N/A" : digit[Sum],
 		(Tta == MS_NA) ? "N/A" : digit[Tta], Ttb, Tra, Trb, Type);
 
@@ -673,15 +670,6 @@
 		rx_window = rx_good & rx_valid_win;
 		rx_slot_count = pcu_bitcount(rx_window);
 
-#if 0
-		LOGP(DRLCMAC, LOGL_DEBUG, "n_tx=%d, n_rx=%d, mask_sel=%d, "
-			"tx=%02x, rx=%02x, mask=%02x, bad=%02x, good=%02x, "
-			"ul=%02x, dl=%02x\n",
-			tx_slot_count, rx_slot_count, mask_sel,
-			tx_window, rx_window, rx_mask[mask_sel], rx_bad, rx_good,
-			*ul_slots, *dl_slots);
-#endif
-
 		/* Check compliance with TS 45.002, table 6.4.2.2.1 */
 		/* Whether to skip this round doesn not only depend on the bit
 		 * sets but also on mask_sel. Therefore this check must be done
@@ -702,16 +690,6 @@
 					continue;
 			} else {
 				/* No supported row in table 6.4.2.2.1. */
-#ifdef ENABLE_TS_ALLOC_DEBUG
-				LOGP(DRLCMAC, LOGL_DEBUG,
-					"- Skipping DL/UL slots: (TS=0)\"%s\"(TS=7), "
-					"combination not supported\n",
-					set_flag_chars(set_flag_chars(set_flag_chars(
-								slot_info,
-								rx_bad, 'x', '.'),
-							rx_window, 'D'),
-						tx_window, 'U'));
-#endif
 				continue;
 			}
 		}
@@ -721,14 +699,6 @@
 			continue;
 
 		if (!rx_good) {
-#ifdef ENABLE_TS_ALLOC_DEBUG
-			LOGP(DRLCMAC, LOGL_DEBUG,
-				"- Skipping DL/UL slots: (TS=0)\"%s\"(TS=7), "
-				"no DL slots available\n",
-				set_flag_chars(set_flag_chars(slot_info,
-						rx_bad, 'x', '.'),
-						tx_window, 'U'));
-#endif
 			continue;
 		}
 
@@ -742,18 +712,6 @@
 			req_common_slots = OSMO_MIN(req_common_slots, 2);
 
 		if (req_common_slots != common_slot_count) {
-#ifdef ENABLE_TS_ALLOC_DEBUG
-			LOGP(DRLCMAC, LOGL_DEBUG,
-				"- Skipping DL/UL slots: (TS=0)\"%s\"(TS=7), "
-				"invalid number of common TS: %d (expected %d)\n",
-				set_flag_chars(set_flag_chars(set_flag_chars(
-							slot_info,
-							rx_bad, 'x', '.'),
-						rx_window, 'D'),
-					tx_window, 'U'),
-				common_slot_count,
-				req_common_slots);
-#endif
 			continue;
 		}
 
@@ -777,19 +735,6 @@
 				}
 			}
 		}
-
-#ifdef ENABLE_TS_ALLOC_DEBUG
-		LOGP(DRLCMAC, LOGL_DEBUG,
-			"- Considering DL/UL slots: (TS=0)\"%s\"(TS=7), "
-			"capacity = %d\n",
-			set_flag_chars(set_flag_chars(set_flag_chars(set_flag_chars(
-					slot_info,
-					rx_bad, 'x', '.'),
-					rx_window, 'D'),
-					tx_window, 'U'),
-					rx_window & tx_window, 'C'),
-			capacity);
-#endif
 
 		if (capacity <= max_capacity)
 			continue;
@@ -850,20 +795,10 @@
 		return -EINVAL;
 	}
 
-	reserved_dl_slots = dl_slots = ms->reserved_dl_slots();
-	reserved_ul_slots = ul_slots = ms->reserved_ul_slots();
+	dl_slots = ms->reserved_dl_slots();
+	ul_slots = ms->reserved_ul_slots();
 	first_common_ts = ms->first_common_ts();
 	trx = ms->current_trx();
-
-	if (trx) {
-		if (use_trx >= 0 && use_trx != trx->trx_no) {
-			LOGP(DRLCMAC, LOGL_ERROR,
-				"- Requested incompatible TRX %d (current is %d)\n",
-				use_trx, trx->trx_no);
-			return -EINVAL;
-		}
-		use_trx = trx->trx_no;
-	}
 
 	/* Step 2a: Find usable TRX and TFI */
 	tfi = tfi_find_free(bts->bts, trx, ms, tbf->direction, use_trx, &trx_no);
@@ -880,11 +815,11 @@
 		rc = find_multi_slots(trx, ms, &ul_slots, &dl_slots);
 		if (rc < 0)
 			return rc;
-
-		reserved_dl_slots = dl_slots;
-		reserved_ul_slots = ul_slots;
 	}
 
+	reserved_dl_slots = dl_slots;
+	reserved_ul_slots = ul_slots;
+
 	/* Step 3: Derive the slot set for the current TBF */
 	if (single) {
 		/* Make sure to consider the first common slot only */

-- 
To view, visit https://gerrit.osmocom.org/3760
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I02da2b8ba8c9c8815dae0e39e1fed277ca0df171
Gerrit-PatchSet: 15
Gerrit-Project: osmo-pcu
Gerrit-Branch: master
Gerrit-Owner: Max <msuraev at sysmocom.de>
Gerrit-Reviewer: Holger Freyther <holger at freyther.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Neels Hofmeyr <nhofmeyr at sysmocom.de>



More information about the gerrit-log mailing list