Attention is currently required from: Hoernchen. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/26190 )

Change subject: rework the counters and stats so they work with the threaded mgw ......................................................................

Patch Set 13:

(9 comments)

File include/osmocom/mgcp/mgcp_trunk.h:

https://gerrit.osmocom.org/c/osmo-mgw/+/26190/comment/241577c6_fb79c067 PS13, Line 38: struct mgcp_ratectr_trunk ratectr; /* NULL for the threads! */ And this kind of stuff is what makes me speak again about having different structs for the global state and one for each worker thread.

File src/libosmo-mgcp/mgcp_endp.c:

https://gerrit.osmocom.org/c/osmo-mgw/+/26190/comment/3f1d0163_347537f3 PS13, Line 128: if (endp->callid) { So these stats are being dropped? why? I don't recall reading about it in the commit description.

File src/libosmo-mgcp/mgcp_stat.c:

https://gerrit.osmocom.org/c/osmo-mgw/+/26190/comment/e4e8553e_45ecaa82 PS13, Line 84: atomic_uint_least64_t packets_rx = conn->atomic_counters[RTP_PACKETS_RX_CTR]; AFAIU these don't need to be atomic, since they are basically copied locally and used here, no synchonization needs to happen back.

File src/libosmo-mgcp/mgcp_threads.c:

https://gerrit.osmocom.org/c/osmo-mgw/+/26190/comment/43193be7_c55f3b34 PS13, Line 428: for (int i = 0; i < _MGCP_GENERAL_NUM_ENUMS; i++) { This kind of loops make me thing whether we'd be better using a mutex rather than accessing crazy amounts of atomic variables (barriers, etc.)

https://gerrit.osmocom.org/c/osmo-mgw/+/26190/comment/2dbf0814_e82c8eff PS13, Line 433: atomic_uint_least64_t endpoints_used = 0; why does this need to be atomic? It's just a local variable not being accessed by other threads?

https://gerrit.osmocom.org/c/osmo-mgw/+/26190/comment/e891b57e_33e1722b PS13, Line 490: /* wait for the threads to be done with init, so main thread can safely read atomics*/ space: atomics */

File src/libosmo-mgcp/mgcp_threads_vty.c:

https://gerrit.osmocom.org/c/osmo-mgw/+/26190/comment/271344c9_1f30c61f PS13, Line 34: atomic_uint_least64_t tx_packets, tx_bytes; same ehre, why are these atomic?

File src/libosmo-mgcp/mgcp_trunk.c:

https://gerrit.osmocom.org/c/osmo-mgw/+/26190/comment/fa73d7f9_686e2888 PS13, Line 143: trunk->number_endpoints = number_endpoints; another stat being removed here?

File tests/mgcp/mgcp_test.c:

https://gerrit.osmocom.org/c/osmo-mgw/+/26190/comment/008d4859_bb485834 PS13, Line 1315: atomic_uint_least64_t test_ctr_in; no need to be atomic?