[PATCH] osmo-pcu[master]: Simplify TS alloc: cosmetic, use proper formatting

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
Wed Sep 13 16:15:25 UTC 2017


Hello Jenkins Builder,

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

    https://gerrit.osmocom.org/3914

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

Simplify TS alloc: cosmetic, use proper formatting

The find_multi_slots() function has 4 nested for(;;) loops but it's
formatted as one. Before trying to split this into smth saner, let's
first fix code formatting to make nested loops obvious. Also, fix typos
in comments.

Change-Id: I50b59b12e8d938bd96f9b29d00a649cd3e77bc5e
Related: OS#2282
---
M src/gprs_rlcmac_ts_alloc.cpp
1 file changed, 142 insertions(+), 141 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/osmo-pcu refs/changes/14/3914/2

diff --git a/src/gprs_rlcmac_ts_alloc.cpp b/src/gprs_rlcmac_ts_alloc.cpp
index 301b2eb..cfef05a 100644
--- a/src/gprs_rlcmac_ts_alloc.cpp
+++ b/src/gprs_rlcmac_ts_alloc.cpp
@@ -604,8 +604,7 @@
 			rx_mask[MASK_TT] = (0x100 >> OSMO_MAX(Ttb, Tta)) - 1;
 			rx_mask[MASK_TT] &= ~((1 << (Trb + num_tx)) - 1);
 			rx_mask[MASK_TR] = (0x100 >> Ttb) - 1;
-			rx_mask[MASK_TR] &=
-				~((1 << (OSMO_MAX(Trb, Tra) + num_tx)) - 1);
+			rx_mask[MASK_TR] &= ~((1 << (OSMO_MAX(Trb, Tra) + num_tx)) - 1);
 		} else {
 			/* Class type 2 MS have independant RX and TX */
 			rx_mask[MASK_TT] = 0xff;
@@ -615,177 +614,179 @@
 		rx_mask[MASK_TT] = (rx_mask[MASK_TT] << 3) | (rx_mask[MASK_TT] >> 5);
 		rx_mask[MASK_TR] = (rx_mask[MASK_TR] << 3) | (rx_mask[MASK_TR] >> 5);
 
-	/* Rotate group of TX slots: UUU-----, -UUU----, ..., UU-----U */
-	for (ul_ts = 0; ul_ts < 8; ul_ts += 1, tx_valid_win <<= 1) {
-		unsigned tx_slot_count;
-		int max_rx;
-		uint16_t rx_valid_win;
-		uint32_t checked_rx[256/32] = {0};
+		/* Rotate group of TX slots: UUU-----, -UUU----, ..., UU-----U */
+		for (ul_ts = 0; ul_ts < 8; ul_ts += 1, tx_valid_win <<= 1) {
+			unsigned tx_slot_count;
+			int max_rx;
+			uint16_t rx_valid_win;
+			uint32_t checked_rx[256/32] = {0};
 
-		/* Wrap valid window */
-		tx_valid_win = (tx_valid_win | tx_valid_win >> 8) & 0xff;
+			/* Wrap valid window */
+			tx_valid_win = (tx_valid_win | tx_valid_win >> 8) & 0xff;
 
-		tx_window = tx_valid_win;
+			tx_window = tx_valid_win;
 
-		/* Filter out unavailable slots */
-		tx_window &= *ul_slots;
+			/* Filter out unavailable slots */
+			tx_window &= *ul_slots;
 
-		/* Skip if the the first TS (ul_ts) is not in the set */
-		if ((tx_window & (1 << ul_ts)) == 0)
-			continue;
+			/* Skip if the the first TS (ul_ts) is not in the set */
+			if ((tx_window & (1 << ul_ts)) == 0)
+				continue;
 
-		/* Skip if the the last TS (ul_ts+num_tx-1) is not in the set */
-		if ((tx_window & (1 << ((ul_ts+num_tx-1) % 8))) == 0)
-			continue;
+			/* Skip if the the last TS (ul_ts+num_tx-1) is not in the set */
+			if ((tx_window & (1 << ((ul_ts+num_tx-1) % 8))) == 0)
+				continue;
 
-		tx_slot_count = pcu_bitcount(tx_window);
+			tx_slot_count = pcu_bitcount(tx_window);
 
-		max_rx = OSMO_MIN(ms_class->rx, ms_class->sum - num_tx);
-		rx_valid_win = (1 << max_rx) - 1;
+			max_rx = OSMO_MIN(ms_class->rx, ms_class->sum - num_tx);
+			rx_valid_win = (1 << max_rx) - 1;
 
-	/* Rotate group of RX slots: DDD-----, -DDD----, ..., DD-----D */
-	for (dl_ts = 0; dl_ts < 8; dl_ts += 1, rx_valid_win <<= 1) {
-		/* Wrap valid window */
-		rx_valid_win = (rx_valid_win | rx_valid_win >> 8) & 0xff;
+			/* Rotate group of RX slots: DDD-----, -DDD----, ..., DD-----D */
+			for (dl_ts = 0; dl_ts < 8; dl_ts += 1, rx_valid_win <<= 1) {
+				/* Wrap valid window */
+				rx_valid_win = (rx_valid_win | rx_valid_win >> 8) & 0xff;
 
-	/* Validate with both Tta/Ttb/Trb and Ttb/Tra/Trb */
-	for (mask_sel = MASK_TT; mask_sel <= MASK_TR; mask_sel += 1) {
-		unsigned common_slot_count;
-		unsigned req_common_slots;
-		unsigned rx_slot_count;
-		uint16_t rx_bad;
-		uint8_t rx_good;
-		unsigned ts;
-		int capacity;
+				/* Validate with both Tta/Ttb/Trb and Ttb/Tra/Trb */
+				for (mask_sel = MASK_TT; mask_sel <= MASK_TR; mask_sel += 1) {
+					unsigned common_slot_count;
+					unsigned req_common_slots;
+					unsigned rx_slot_count;
+					uint16_t rx_bad;
+					uint8_t rx_good;
+					unsigned ts;
+					int capacity;
 
-		/* Filter out bad slots */
-		rx_bad = (uint16_t)(0xff & ~rx_mask[mask_sel]) << ul_ts;
-		rx_bad = (rx_bad | (rx_bad >> 8)) & 0xff;
-		rx_good = *dl_slots & ~rx_bad;
+					/* Filter out bad slots */
+					rx_bad = (uint16_t)(0xff & ~rx_mask[mask_sel]) << ul_ts;
+					rx_bad = (rx_bad | (rx_bad >> 8)) & 0xff;
+					rx_good = *dl_slots & ~rx_bad;
 
-		/* TODO: CHECK this calculation -> separate function for unit
-		 * testing */
+					/* TODO: CHECK this calculation -> separate function for unit testing */
 
-		rx_window = rx_good & rx_valid_win;
-		rx_slot_count = pcu_bitcount(rx_window);
+					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);
+					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
-		 * before doing the test_and_set_bit shortcut. */
-		if (ms_class->type == 1) {
-			unsigned slot_sum = rx_slot_count + tx_slot_count;
-			/* Assume down+up/dynamic.
-			 * TODO: For ext-dynamic, down only, up only add more
-			 *       cases.
-			 */
-			if (slot_sum <= 6 && tx_slot_count < 3) {
-			       if (mask_sel != MASK_TR)
-				       /* Skip Tta */
-				       continue;
-			} else if (slot_sum > 6 && tx_slot_count < 3) {
-				if (mask_sel != MASK_TT)
-					/* Skip Tra */
-					continue;
-			} else {
-				/* No supported row in table 6.4.2.2.1. */
+					/* 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
+					 * before doing the test_and_set_bit shortcut. */
+					if (ms_class->type == 1) {
+						unsigned slot_sum = rx_slot_count + tx_slot_count;
+						/* Assume down+up/dynamic.
+						 * TODO: For ext-dynamic, down only, up only add more cases.
+						 */
+						if (slot_sum <= 6 && tx_slot_count < 3) {
+							if (mask_sel != MASK_TR)
+								/* Skip Tta */
+								continue;
+						} else if (slot_sum > 6 && tx_slot_count < 3) {
+							if (mask_sel != MASK_TT)
+								/* Skip Tra */
+								continue;
+						} else {
+							/* No supported row in TS 45.002, table 6.4.2.2.1. */
 #ifdef ENABLE_TS_ALLOC_DEBUG
-				snprintf(slot_info, 9, OSMO_BIT_SPEC, OSMO_BIT_PRINT_EX(rx_bad, 'x'));
-				masked_override_with(slot_info, rx_window, 'D');
-				masked_override_with(slot_info, tx_window, 'U');
-				LOGP(DRLCMAC, LOGL_DEBUG,
-				     "- Skipping DL/UL slots: (TS=0)\"%s\"(TS=7), "
-				     "combination not supported\n", slot_info);
+							snprintf(slot_info, 9, OSMO_BIT_SPEC,
+								 OSMO_BIT_PRINT_EX(rx_bad, 'x'));
+							masked_override_with(slot_info, rx_window, 'D');
+							masked_override_with(slot_info, tx_window, 'U');
+							LOGP(DRLCMAC, LOGL_DEBUG,
+							     "- Skipping DL/UL slots: (TS=0)\"%s\"(TS=7), "
+							     "combination not supported\n", slot_info);
 #endif
-				continue;
-			}
-		}
+							continue;
+						}
+					}
 
-		/* Avoid repeated RX combination check */
-		if (test_and_set_bit(checked_rx, rx_window))
-			continue;
+					/* Avoid repeated RX combination check */
+					if (test_and_set_bit(checked_rx, rx_window))
+						continue;
 
-		if (!rx_good) {
+					if (!rx_good) {
 #ifdef ENABLE_TS_ALLOC_DEBUG
-			snprintf(slot_info, 9, OSMO_BIT_SPEC, OSMO_BIT_PRINT_EX(rx_bad, 'x'));
-			masked_override_with(slot_info, tx_window, 'U');
-			LOGP(DRLCMAC, LOGL_DEBUG,
-			     "- Skipping DL/UL slots: (TS=0)\"%s\"(TS=7), "
-			     "no DL slots available\n", slot_info);
+						snprintf(slot_info, 9, OSMO_BIT_SPEC, OSMO_BIT_PRINT_EX(rx_bad, 'x'));
+						masked_override_with(slot_info, tx_window, 'U');
+						LOGP(DRLCMAC, LOGL_DEBUG,
+						     "- Skipping DL/UL slots: (TS=0)\"%s\"(TS=7), "
+						     "no DL slots available\n", slot_info);
 #endif
-			continue;
-		}
+						continue;
+					}
 
-		if (!rx_window)
-			continue;
+					if (!rx_window)
+						continue;
 
-		/* Check number of common slots according to TS 54.002, 6.4.2.2 */
-		common_slot_count = pcu_bitcount(tx_window & rx_window);
-		req_common_slots = OSMO_MIN(tx_slot_count, rx_slot_count);
-		if (ms_class->type == 1)
-			req_common_slots = OSMO_MIN(req_common_slots, 2);
+					/* Check number of common slots according to TS 45.002, §6.4.2.2 */
+					common_slot_count = pcu_bitcount(tx_window & rx_window);
+					req_common_slots = OSMO_MIN(tx_slot_count, rx_slot_count);
+					if (ms_class->type == 1)
+						req_common_slots = OSMO_MIN(req_common_slots, 2);
 
-		if (req_common_slots != common_slot_count) {
+					if (req_common_slots != common_slot_count) {
 #ifdef ENABLE_TS_ALLOC_DEBUG
-			snprintf(slot_info, 9, OSMO_BIT_SPEC, OSMO_BIT_PRINT_EX(rx_bad, 'x'));
-			masked_override_with(slot_info, rx_window, 'D');
-			masked_override_with(slot_info, tx_window, 'U');
-			LOGP(DRLCMAC, LOGL_DEBUG,
-			     "- Skipping DL/UL slots: (TS=0)\"%s\"(TS=7), "
-			     "invalid number of common TS: %d (expected %d)\n",
-			     slot_info, common_slot_count, req_common_slots);
+						snprintf(slot_info, 9, OSMO_BIT_SPEC, OSMO_BIT_PRINT_EX(rx_bad, 'x'));
+						masked_override_with(slot_info, rx_window, 'D');
+						masked_override_with(slot_info, tx_window, 'U');
+						LOGP(DRLCMAC, LOGL_DEBUG,
+						     "- Skipping DL/UL slots: (TS=0)\"%s\"(TS=7), "
+						     "invalid number of common TS: %d (expected %d)\n",
+						     slot_info, common_slot_count, req_common_slots);
 #endif
-			continue;
-		}
+						continue;
+					}
 
-		/* Compute capacity */
-		capacity = 0;
+					/* Compute capacity */
+					capacity = 0;
 
-		for (ts = 0; ts < ARRAY_SIZE(trx->pdch); ts++) {
-			int c;
-			const struct gprs_rlcmac_pdch *pdch = &trx->pdch[ts];
-			if (rx_window & (1 << ts)) {
-				c = 32 - pdch->num_reserved(GPRS_RLCMAC_DL_TBF);
-				c = OSMO_MAX(c, 1);
-				capacity += c;
-			}
-			/* Only consider common slots for UL */
-			if (tx_window & rx_window & (1 << ts)) {
-				if (find_free_usf(pdch) >= 0) {
-					c = 32 - pdch->num_reserved(GPRS_RLCMAC_UL_TBF);
-					c = OSMO_MAX(c, 1);
-					capacity += c;
+					for (ts = 0; ts < ARRAY_SIZE(trx->pdch); ts++) {
+						int c;
+						const struct gprs_rlcmac_pdch *pdch = &trx->pdch[ts];
+						if (rx_window & (1 << ts)) {
+							c = 32 - pdch->num_reserved(GPRS_RLCMAC_DL_TBF);
+							c = OSMO_MAX(c, 1);
+							capacity += c;
+						}
+						/* Only consider common slots for UL */
+						if (tx_window & rx_window & (1 << ts)) {
+							if (find_free_usf(pdch) >= 0) {
+								c = 32 - pdch->num_reserved(GPRS_RLCMAC_UL_TBF);
+								c = OSMO_MAX(c, 1);
+								capacity += c;
+							}
+						}
+					}
+
+#ifdef ENABLE_TS_ALLOC_DEBUG
+					snprintf(slot_info, 9, OSMO_BIT_SPEC, OSMO_BIT_PRINT_EX(rx_bad, 'x'));
+					masked_override_with(slot_info, rx_window, 'D');
+					masked_override_with(slot_info, tx_window, 'U');
+					masked_override_with(slot_info, rx_window& tx_window, 'C');
+					LOGP(DRLCMAC, LOGL_DEBUG,
+					     "- Considering DL/UL slots: (TS=0)\"%s\"(TS=7), "
+					     "capacity = %d\n",
+					     slot_info, capacity);
+#endif
+
+					if (capacity <= max_capacity)
+						continue;
+
+					max_capacity = capacity;
+					max_ul_slots = tx_window;
+					max_dl_slots = rx_window;
 				}
 			}
 		}
-
-#ifdef ENABLE_TS_ALLOC_DEBUG
-		snprintf(slot_info, 9, OSMO_BIT_SPEC, OSMO_BIT_PRINT_EX(rx_bad, 'x'));
-		masked_override_with(slot_info, rx_window, 'D');
-		masked_override_with(slot_info, tx_window, 'U');
-		masked_override_with(slot_info, rx_window& tx_window, 'C');
-		LOGP(DRLCMAC, LOGL_DEBUG,
-		     "- Considering DL/UL slots: (TS=0)\"%s\"(TS=7), "
-		     "capacity = %d\n",
-		     slot_info, capacity);
-#endif
-
-		if (capacity <= max_capacity)
-			continue;
-
-		max_capacity = capacity;
-		max_ul_slots = tx_window;
-		max_dl_slots = rx_window;
-	}}}}
+	}
 
 	if (!max_ul_slots || !max_dl_slots) {
 		LOGP(DRLCMAC, LOGL_NOTICE,

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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I50b59b12e8d938bd96f9b29d00a649cd3e77bc5e
Gerrit-PatchSet: 2
Gerrit-Project: osmo-pcu
Gerrit-Branch: master
Gerrit-Owner: Max <msuraev at sysmocom.de>
Gerrit-Reviewer: Jenkins Builder



More information about the gerrit-log mailing list