pespin has submitted this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/27897 )
Change subject: paging: Prioritize requests for new subscribers over retransmitions ......................................................................
paging: Prioritize requests for new subscribers over retransmitions
Currently, the Tx paging_req queue at the BSC always has new paging requests adding at the end (as long as the subscriber is not already in the queue). That means, if the queue is full of retransmitions, it will take a long time until the first paging_req is sent towards that subscriber. The rationale here is that it makes sense to attempt the first paging ASAP, and give lower prio to paging_req retransmitions, since it may well be that those other subsribers are not available/reachable and won't answer.
Related: SYS#5922 Change-Id: I1ae6d97152c458247bc538233b97c2d245196359 --- M src/osmo-bsc/paging.c 1 file changed, 24 insertions(+), 20 deletions(-)
Approvals: laforge: Looks good to me, but someone else must approve fixeria: Looks good to me, but someone else must approve pespin: Looks good to me, approved Jenkins Builder: Verified
diff --git a/src/osmo-bsc/paging.c b/src/osmo-bsc/paging.c index f81713e..3275473 100644 --- a/src/osmo-bsc/paging.c +++ b/src/osmo-bsc/paging.c @@ -266,20 +266,6 @@ osmo_timer_setup(&bts->paging.credit_timer, paging_give_credit, &bts->paging); }
-/*! do we have any pending paging requests for given subscriber? */ -static int paging_pending_request(struct gsm_bts_paging_state *bts, - struct bsc_subscr *bsub) -{ - struct gsm_paging_request *req; - - llist_for_each_entry(req, &bts->pending_requests, entry) { - if (bsub == req->bsub) - return 1; - } - - return 0; -} - /*! Call-back once T3113 (paging timeout) expires for given paging_request */ static void paging_T3113_expired(void *data) { @@ -342,15 +328,27 @@ static int _paging_request(const struct bsc_paging_params *params, struct gsm_bts *bts) { struct gsm_bts_paging_state *bts_entry = &bts->paging; - struct gsm_paging_request *req; + struct gsm_paging_request *req, *last_initial_req = NULL; unsigned int t3113_timeout_s;
rate_ctr_inc(rate_ctr_group_get_ctr(bts->bts_ctrs, BTS_CTR_PAGING_ATTEMPTED));
- if (paging_pending_request(bts_entry, params->bsub)) { - LOG_PAGING_BTS(params, bts, DPAG, LOGL_INFO, "Paging request already pending for this subscriber\n"); - rate_ctr_inc(rate_ctr_group_get_ctr(bts->bts_ctrs, BTS_CTR_PAGING_ALREADY)); - return -EEXIST; + /* Iterate list of pending requests to find if we already have one for + * the given subscriber. While on it, find the last + * not-yet-ever-once-transmitted request; the new request will be added + * immediately after it, giving higher prio to initial transmissions + * (no retrans). This avoids new subscribers being paged to be delayed + * if the paging queue is full due to a lot of retranmissions. + * Retranmissions usually mean MS are not reachable/available, so the + * rationale here is to prioritize new subs which may be available. */ + llist_for_each_entry(req, &bts_entry->pending_requests, entry) { + if (params->bsub == req->bsub) { + LOG_PAGING_BTS(params, bts, DPAG, LOGL_INFO, "Paging request already pending for this subscriber\n"); + rate_ctr_inc(rate_ctr_group_get_ctr(bts->bts_ctrs, BTS_CTR_PAGING_ALREADY)); + return -EEXIST; + } + if (req->attempts == 0) + last_initial_req = req; }
LOG_PAGING_BTS(params, bts, DPAG, LOGL_DEBUG, "Start paging\n"); @@ -364,9 +362,15 @@ req->chan_type = params->chan_needed; req->msc = params->msc; osmo_timer_setup(&req->T3113, paging_T3113_expired, req); + + /* there's no initial req (attempts==0), add to the start of the list */ + if (last_initial_req == NULL) + llist_add(&req->entry, &bts_entry->pending_requests); + else/* Add in the middle of the list after last_initial_req */ + __llist_add(&req->entry, &last_initial_req->entry, last_initial_req->entry.next); + t3113_timeout_s = calculate_timer_3113(bts); osmo_timer_schedule(&req->T3113, t3113_timeout_s, 0); - llist_add_tail(&req->entry, &bts_entry->pending_requests); paging_schedule_if_needed(bts_entry);
return 0;