pespin has uploaded this change for review.

View Change

ms: Get rid of ms->delay field

Simply apply the content of the configured timer when the MS goes idle.
Having that field is convenient to do tricky stuff in unit tests, but
makes the main osmo-pcu app more complex for no good enough reason.

Change-Id: I8d44318b37b6605afd84db8ccec0d75e6db293b9
---
M src/bts.cpp
M src/gprs_ms.c
M src/gprs_ms.h
M tests/alloc/AllocTest.cpp
M tests/ms/MsTest.cpp
M tests/tbf/TbfTest.cpp
M tests/ulc/PdchUlcTest.cpp
7 files changed, 30 insertions(+), 24 deletions(-)

git pull ssh://gerrit.osmocom.org:29418/osmo-pcu refs/changes/90/32390/1
diff --git a/src/bts.cpp b/src/bts.cpp
index 4cbe68e..d4bb35f 100644
--- a/src/bts.cpp
+++ b/src/bts.cpp
@@ -241,11 +241,8 @@
static int bts_talloc_destructor(struct gprs_rlcmac_bts* bts)
{
struct GprsMs *ms;
- while ((ms = llist_first_entry_or_null(&bts->ms_list, struct GprsMs, list))) {
- ms_set_timeout(ms, 0);
- bts_stat_item_dec(bts, STAT_MS_PRESENT);
+ while ((ms = llist_first_entry_or_null(&bts->ms_list, struct GprsMs, list)))
talloc_free(ms);
- }

gprs_bssgp_destroy(bts);

diff --git a/src/gprs_ms.c b/src/gprs_ms.c
index 877422f..1450d19 100644
--- a/src/gprs_ms.c
+++ b/src/gprs_ms.c
@@ -170,8 +170,6 @@
if (!ms->ctrs)
goto free_ret;

- ms_set_timeout(ms, osmo_tdef_get(bts->pcu->T_defs, -2030, OSMO_TDEF_S, -1));
-
if (use_ref)
ms_ref(ms, use_ref);
return ms;
@@ -219,6 +217,8 @@
/* MS has no attached TBFs anymore. */
static void ms_becomes_idle(struct GprsMs *ms)
{
+ unsigned long delay_rel_sec = osmo_tdef_get(ms->bts->pcu->T_defs, -2030, OSMO_TDEF_S, -1);
+
ms_set_reserved_slots(ms, NULL, 0, 0);
ms->first_common_ts = NULL;

@@ -226,7 +226,7 @@
* Skip delaying free() through release timer if delay is configured to be 0.
* This is useful for synced freed during unit tests.
*/
- if (ms->delay == 0) {
+ if (delay_rel_sec == 0) {
talloc_free(ms);
return;
}
@@ -240,8 +240,8 @@
return;
}

- LOGPMS(ms, DMS, LOGL_INFO, "Schedule MS release in %u secs\n", ms->delay);
- osmo_timer_schedule(&ms->timer, ms->delay, 0);
+ LOGPMS(ms, DMS, LOGL_INFO, "Schedule MS release in %lu secs\n", delay_rel_sec);
+ osmo_timer_schedule(&ms->timer, delay_rel_sec, 0);
}

static void ms_becomes_active(struct GprsMs *ms)
@@ -398,9 +398,6 @@
struct llist_item *pos;
struct gprs_rlcmac_tbf *tbf;

- /* free immediately when it becomes idle: */
- ms->delay = 0;
-
tbf = ul_tbf_as_tbf(ms_ul_tbf(ms));
if (tbf && !tbf_timers_pending(tbf, T_MAX))
tbf_free(tbf);
@@ -415,7 +412,8 @@
}

/* Flag it with invalid data so that it cannot be looked up anymore and
- * shows up specially if listed in VTY: */
+ * shows up specially if listed in VTY. Furthermore, it will also trigger
+ * immediate free() when it becomes idle: */
ms->tlli = GSM_RESERVED_TMSI;
ms->new_dl_tlli = ms->tlli;
ms->new_ul_tlli = ms->tlli;
diff --git a/src/gprs_ms.h b/src/gprs_ms.h
index 03508ff..4f6456e 100644
--- a/src/gprs_ms.h
+++ b/src/gprs_ms.h
@@ -80,7 +80,6 @@

struct osmo_use_count use_count;
struct osmo_timer_list timer;
- unsigned delay;

int64_t last_cs_not_low;

@@ -216,11 +215,6 @@
return ms->mode;
}

-static inline void ms_set_timeout(struct GprsMs *ms, unsigned secs)
-{
- ms->delay = secs;
-}
-
static inline unsigned ms_nack_rate_dl(const struct GprsMs *ms)
{
return ms->nack_rate_dl;
diff --git a/tests/alloc/AllocTest.cpp b/tests/alloc/AllocTest.cpp
index 8a67152..a84bd64 100644
--- a/tests/alloc/AllocTest.cpp
+++ b/tests/alloc/AllocTest.cpp
@@ -225,7 +225,7 @@
ms = ms_alloc(bts, __func__);
ms_set_ms_class(ms, ms_class);
/* Avoid delaying free to avoid tons of to-be-freed ms objects queuing */
- ms_set_timeout(ms, 0);
+ OSMO_ASSERT(osmo_tdef_set(the_pcu->T_defs, -2030, 0, OSMO_TDEF_S) == 0);
ul_tbf = ul_tbf_alloc(bts, ms, -1, true);
if (!ul_tbf) {
ms_unref(ms, __func__);
@@ -273,7 +273,7 @@
ms = ms_alloc(bts, __func__);
ms_set_ms_class(ms, ms_class);
/* Avoid delaying free to avoid tons of to-be-freed ms objects queuing */
- ms_set_timeout(ms, 0);
+ OSMO_ASSERT(osmo_tdef_set(the_pcu->T_defs, -2030, 0, OSMO_TDEF_S) == 0);
dl_tbf = dl_tbf_alloc(bts, ms, -1, true);
if (!dl_tbf) {
ms_unref(ms, __func__);
@@ -329,7 +329,7 @@
ms = ms_alloc(bts, __func__);
ms_set_ms_class(ms, ms_class);
/* Avoid delaying free to avoid tons of to-be-freed ms objects queuing */
- ms_set_timeout(ms, 0);
+ OSMO_ASSERT(osmo_tdef_set(the_pcu->T_defs, -2030, 0, OSMO_TDEF_S) == 0);
ul_tbf = ul_tbf_alloc(bts, ms, -1, false);
if (!ul_tbf) {
ms_unref(ms, __func__);
diff --git a/tests/ms/MsTest.cpp b/tests/ms/MsTest.cpp
index 2a15a7b..776f77c7 100644
--- a/tests/ms/MsTest.cpp
+++ b/tests/ms/MsTest.cpp
@@ -378,9 +378,10 @@

printf("=== start %s ===\n", __func__);

+ OSMO_ASSERT(osmo_tdef_set(the_pcu->T_defs, -2030, 1, OSMO_TDEF_S) == 0);
+
ms = ms_alloc(bts, __func__);
ms_set_tlli(ms, tlli);
- ms_set_timeout(ms, 1);

OSMO_ASSERT(ms_is_idle(ms));

@@ -415,6 +416,8 @@
talloc_free(ul_tbf);
talloc_free(bts);
printf("=== end %s ===\n", __func__);
+ /* Return the timer to the usually expected value 0 for other tests: */
+ OSMO_ASSERT(osmo_tdef_set(the_pcu->T_defs, -2030, 0, OSMO_TDEF_S) == 0);
}

static void test_ms_cs_selection()
diff --git a/tests/tbf/TbfTest.cpp b/tests/tbf/TbfTest.cpp
index ad08318..76394e2 100644
--- a/tests/tbf/TbfTest.cpp
+++ b/tests/tbf/TbfTest.cpp
@@ -627,7 +627,7 @@
fprintf(stderr, "=== end %s ===\n", __func__);

/* Restore MS release timeout to 0 to make sure it is freed immediately: */
- ms_set_timeout(ms, 0);
+ OSMO_ASSERT(osmo_tdef_set(the_pcu->T_defs, -2030, 0, OSMO_TDEF_S) == 0);
TALLOC_FREE(the_pcu);
}

diff --git a/tests/ulc/PdchUlcTest.cpp b/tests/ulc/PdchUlcTest.cpp
index be370bd..a13479f 100644
--- a/tests/ulc/PdchUlcTest.cpp
+++ b/tests/ulc/PdchUlcTest.cpp
@@ -320,6 +320,7 @@
log_parse_category_mask(osmo_stderr_target, "DPCU,1:DRLCMAC,1:DRLCMACUL,1:DMS,1");

the_pcu = gprs_pcu_alloc(tall_pcu_ctx);
+ OSMO_ASSERT(osmo_tdef_set(the_pcu->T_defs, -2030, 0, OSMO_TDEF_S) == 0);

test_reserve_multiple();
test_fn_wrap_around();

To view, visit change 32390. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: osmo-pcu
Gerrit-Branch: master
Gerrit-Change-Id: I8d44318b37b6605afd84db8ccec0d75e6db293b9
Gerrit-Change-Number: 32390
Gerrit-PatchSet: 1
Gerrit-Owner: pespin <pespin@sysmocom.de>
Gerrit-MessageType: newchange