pespin has submitted this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/28090 ) Change subject: Revert "stats: new trackers for lchan life duration" ...................................................................... Revert "stats: new trackers for lchan life duration" This reverts commit 5e2ac29703b4b05f202734698fecdb04b4461bd8. This patch was found to be a troublemaker regarding osmo-bsc performance, since it's scheduling one timer every 100ms for each channel. On a BSC with dozens of BTS, each with several TRX, this ends up in a huge amount of timers scheduled in a tight timeframe, which ends up in osmo-bsc spending CPU time getting in and out of the poll() main loop. Related: SYS#5922 Change-Id: Ibd5123e7f04ae8f4eb8f08b63525527f526f0b2c --- M include/osmocom/bsc/bts.h M include/osmocom/bsc/gsm_data.h M src/osmo-bsc/bts.c M src/osmo-bsc/bts_trx_vty.c M src/osmo-bsc/bts_vty.c M src/osmo-bsc/gsm_data.c M src/osmo-bsc/lchan_fsm.c 7 files changed, 0 insertions(+), 91 deletions(-) Approvals: fixeria: Looks good to me, but someone else must approve osmith: Looks good to me, approved iedemam: Looks good to me, but someone else must approve Jenkins Builder: Verified diff --git a/include/osmocom/bsc/bts.h b/include/osmocom/bsc/bts.h index 0ea019d..58ea39c 100644 --- a/include/osmocom/bsc/bts.h +++ b/include/osmocom/bsc/bts.h @@ -60,8 +60,6 @@ BTS_CTR_CHAN_ACT_SDCCH, BTS_CTR_CHAN_ACT_TCH, BTS_CTR_CHAN_ACT_NACK, - BTS_CTR_CHAN_TCH_ACTIVE_DECISECONDS_TOTAL, - BTS_CTR_CHAN_SDCCH_ACTIVE_DECISECONDS_TOTAL, BTS_CTR_RSL_UNKNOWN, BTS_CTR_RSL_IPA_NACK, BTS_CTR_RSL_DELETE_IND, diff --git a/include/osmocom/bsc/gsm_data.h b/include/osmocom/bsc/gsm_data.h index 4babe5e..cb20b9e 100644 --- a/include/osmocom/bsc/gsm_data.h +++ b/include/osmocom/bsc/gsm_data.h @@ -888,8 +888,6 @@ uint8_t interf_band; /* MS power control state */ struct lchan_power_ctrl_state ms_power_ctrl; - /* Interval timing to capture duration per activation and cumulative active time */ - struct osmo_time_cc active_cc; }; /* One Timeslot in a TRX */ @@ -1148,7 +1146,6 @@ char *gsm_ts_name(const struct gsm_bts_trx_ts *ts); char *gsm_ts_and_pchan_name(const struct gsm_bts_trx_ts *ts); void lchan_update_name(struct gsm_lchan *lchan); -uint64_t gsm_lchan_active_duration_ms(const struct gsm_lchan *lchan); static inline char *gsm_lchan_name(const struct gsm_lchan *lchan) { diff --git a/src/osmo-bsc/bts.c b/src/osmo-bsc/bts.c index 45b4ebe..c04a59e 100644 --- a/src/osmo-bsc/bts.c +++ b/src/osmo-bsc/bts.c @@ -1099,12 +1099,6 @@ [BTS_CTR_CHAN_ACT_NACK] = \ { "chan_act:nack", "Number of Channel Activations that the BTS NACKed" }, - [BTS_CTR_CHAN_TCH_ACTIVE_DECISECONDS_TOTAL] = \ - { "chan_tch:active_deciseconds:total", - "Cumulative number of deciseconds of TCH channel activity" }, - [BTS_CTR_CHAN_SDCCH_ACTIVE_DECISECONDS_TOTAL] = \ - { "chan_sdcch:active_deciseconds:total", - "Cumulative number of deciseconds of SDCCH channel activity" }, [BTS_CTR_RSL_UNKNOWN] = \ { "rsl:unknown", "Number of unknown/unsupported RSL messages received from BTS" }, diff --git a/src/osmo-bsc/bts_trx_vty.c b/src/osmo-bsc/bts_trx_vty.c index d232d46..a3d6033 100644 --- a/src/osmo-bsc/bts_trx_vty.c +++ b/src/osmo-bsc/bts_trx_vty.c @@ -569,13 +569,6 @@ vty_out(vty, "BTS %u, TRX %u, Timeslot %u, Lchan %u: Type %s%s", lchan->ts->trx->bts->nr, lchan->ts->trx->nr, lchan->ts->nr, lchan->nr, gsm_lchant_name(lchan->type), VTY_NEWLINE); - - if (lchan->activate.concluded) { - vty_out(vty, " Activated %s seconds ago%s", - osmo_int_to_float_str_c(OTC_SELECT, gsm_lchan_active_duration_ms(lchan), 3), - VTY_NEWLINE); - } - vty_out_dyn_ts_details(vty, lchan->ts); vty_out(vty, " Connection: %u, State: %s%s%s%s", lchan->conn ? 1: 0, lchan_state_name(lchan), diff --git a/src/osmo-bsc/bts_vty.c b/src/osmo-bsc/bts_vty.c index 8af96ed..4e60cd8 100644 --- a/src/osmo-bsc/bts_vty.c +++ b/src/osmo-bsc/bts_vty.c @@ -3677,8 +3677,6 @@ struct gsm_bts_trx *trx; int ts_hopping_total; int ts_non_hopping_total; - const struct rate_ctr *activations_tch; - const struct rate_ctr *activations_sdcch; vty_out(vty, "BTS %u is of %s type in band %s, has CI %u LAC %u, " "BSIC %u (NCC=%u, BCC=%u) and %u TRX%s", @@ -3853,29 +3851,6 @@ rate_ctr_group_get_ctr(bts->bts_ctrs, BTS_CTR_CHREQ_TOTAL)->current, rate_ctr_group_get_ctr(bts->bts_ctrs, BTS_CTR_CHREQ_NO_CHANNEL)->current, VTY_NEWLINE); - - vty_out(vty, " Channel Activations :%s", VTY_NEWLINE); - activations_tch = rate_ctr_group_get_ctr(bts->bts_ctrs, BTS_CTR_CHAN_ACT_TCH); - vty_out(vty, " TCH %"PRIu64"", activations_tch->current); - if (activations_tch->intv[RATE_CTR_INTV_HOUR].rate > 0) { - const struct rate_ctr *active_time_tch_ds = rate_ctr_group_get_ctr(bts->bts_ctrs, BTS_CTR_CHAN_TCH_ACTIVE_DECISECONDS_TOTAL); - vty_out(vty, " (avg lifespan %s seconds in last hour)", - osmo_int_to_float_str_c(OTC_SELECT, - 100 * active_time_tch_ds->intv[RATE_CTR_INTV_HOUR].rate - / activations_tch->intv[RATE_CTR_INTV_HOUR].rate, 3)); - } - vty_out(vty, "%s", VTY_NEWLINE); - activations_sdcch = rate_ctr_group_get_ctr(bts->bts_ctrs, BTS_CTR_CHAN_ACT_SDCCH); - vty_out(vty, " SDCCH %"PRIu64"", activations_sdcch->current); - if (activations_sdcch->intv[RATE_CTR_INTV_HOUR].rate > 0) { - const struct rate_ctr *active_time_sdcch_ds = rate_ctr_group_get_ctr(bts->bts_ctrs, BTS_CTR_CHAN_SDCCH_ACTIVE_DECISECONDS_TOTAL); - vty_out(vty, " (avg lifespan %s seconds in last hour)", - osmo_int_to_float_str_c(OTC_SELECT, - 100 * active_time_sdcch_ds->intv[RATE_CTR_INTV_HOUR].rate - / activations_sdcch->intv[RATE_CTR_INTV_HOUR].rate, 3)); - } - vty_out(vty, "%s", VTY_NEWLINE); - vty_out(vty, " Channel Failures : %"PRIu64" rf_failures, %"PRIu64" rll failures%s", rate_ctr_group_get_ctr(bts->bts_ctrs, BTS_CTR_CHAN_RF_FAIL)->current, rate_ctr_group_get_ctr(bts->bts_ctrs, BTS_CTR_CHAN_RLL_ERR)->current, diff --git a/src/osmo-bsc/gsm_data.c b/src/osmo-bsc/gsm_data.c index 3438eaf..22be92d 100644 --- a/src/osmo-bsc/gsm_data.c +++ b/src/osmo-bsc/gsm_data.c @@ -342,15 +342,6 @@ lchan->nr - (lchan->vamos.is_secondary ? ts->max_primary_lchans : 0)); } -/* If the lchan is currently active, return the duration since activation in milliseconds. - * Otherwise return 0. */ -uint64_t gsm_lchan_active_duration_ms(const struct gsm_lchan *lchan) -{ - if (!lchan->activate.concluded) - return 0; - return lchan->active_cc.total_sum / 1000; -} - /* obtain the MO structure for a given object instance */ static inline struct gsm_abis_mo * gsm_objclass2mo(struct gsm_bts *bts, uint8_t obj_class, diff --git a/src/osmo-bsc/lchan_fsm.c b/src/osmo-bsc/lchan_fsm.c index d693189..5df6c57 100644 --- a/src/osmo-bsc/lchan_fsm.c +++ b/src/osmo-bsc/lchan_fsm.c @@ -200,33 +200,10 @@ static void lchan_on_fully_established(struct gsm_lchan *lchan) { - struct rate_ctr *rate_ctr; - if (lchan->activate.concluded) return; lchan->activate.concluded = true; - /* Assign timekeeper to appropriate rate counter */ - switch (lchan->type) { - case GSM_LCHAN_TCH_H: - case GSM_LCHAN_TCH_F: - rate_ctr = rate_ctr_group_get_ctr(lchan->ts->trx->bts->bts_ctrs, BTS_CTR_CHAN_TCH_ACTIVE_DECISECONDS_TOTAL); - break; - case GSM_LCHAN_SDCCH: - rate_ctr = rate_ctr_group_get_ctr(lchan->ts->trx->bts->bts_ctrs, BTS_CTR_CHAN_SDCCH_ACTIVE_DECISECONDS_TOTAL); - break; - default: - rate_ctr = NULL; - break; - } - lchan->active_cc.cfg.rate_ctr = rate_ctr; - - /* This reset allows showing on vty how long ago a single active lchan was activated. - * This reset does not affect rate_ctr statistics. */ - osmo_time_cc_cleanup(&lchan->active_cc); - /* Start the timekeeper */ - osmo_time_cc_set_flag(&lchan->active_cc, true); - switch (lchan->activate.info.activ_for) { case ACTIVATE_FOR_MS_CHANNEL_REQUEST: /* No signalling to do here, MS is free to use the channel, and should go on to connect @@ -486,16 +463,6 @@ OSMO_ASSERT(lchan->fi); lchan->fi->priv = lchan; lchan_fsm_update_id(lchan); - - /* Configure timekeeper to track this lchan's cumulative active milliseconds */ - lchan->active_cc = (struct osmo_time_cc){ - .cfg = { - .gran_usec = 1*100000, - .forget_sum_usec = 1, - .rate_ctr = NULL, - }, - }; - LOGPFSML(lchan->fi, LOGL_DEBUG, "new lchan\n"); lchan_reset(lchan); } @@ -544,8 +511,6 @@ .tsc_set = 1, .interf_dbm = INTERF_DBM_UNKNOWN, .interf_band = INTERF_BAND_UNKNOWN, - - .active_cc = lchan->active_cc, }; } @@ -553,10 +518,6 @@ { struct gsm_lchan *lchan = lchan_fi_lchan(fi); struct gsm_bts *bts = lchan->ts->trx->bts; - - /* Stop the timekeeper */ - osmo_time_cc_set_flag(&lchan->active_cc, false); - lchan_reset(lchan); osmo_fsm_inst_dispatch(lchan->ts->fi, TS_EV_LCHAN_UNUSED, lchan); -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/28090 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Change-Id: Ibd5123e7f04ae8f4eb8f08b63525527f526f0b2c Gerrit-Change-Number: 28090 Gerrit-PatchSet: 2 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: iedemam <michael(a)kapsulate.com> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: merged