Attention is currently required from: neels.

pespin has posted comments on this change. ( 
https://gerrit.osmocom.org/c/libosmocore/+/36538?usp=email )

Change subject: add osmo_stats_report_lock api
......................................................................


Patch Set 1:

(4 comments)

Patchset:

PS1:
I think with this you are only sorting out one of the concurrency problems when 
using rate_ctr/stats. What if for instance a rate_ctr_group is destroyed or 
added meanwhile? there's several linked lists in there.

Did you look at osmo_itq already to pass the updated counters instead?


File src/core/stats.c:

https://gerrit.osmocom.org/c/libosmocore/+/36538/comment/d347845f_486dbc1b
PS1, Line 808:          pthread_mutex_destroy(g_report_lock);
What happens if a pthread_mutex is destroyed while used?
Probably missing some API documentation about this needing to be called during 
program startup while only 1 thread exists to avoid problems.


https://gerrit.osmocom.org/c/libosmocore/+/36538/comment/040ad420_b7d69456 
PS1, Line 819: void osmo_stats_report_lock(bool lock)
I'd definetly have 2 APIs here, one for lock and one for unlock.
Easier to read and trace in lots of different ways.


https://gerrit.osmocom.org/c/libosmocore/+/36538/comment/3addf1b5_6f05fb3d
PS1, Line 831:  pthread_mutex_t *lock = g_report_lock;
what's the point of this local variable?



--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/36538?usp=email
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: Ib335bea7d2a440ca284e6c439066f96456bf2c2d
Gerrit-Change-Number: 36538
Gerrit-PatchSet: 1
Gerrit-Owner: neels <nhofm...@sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanits...@sysmocom.de>
Gerrit-CC: pespin <pes...@sysmocom.de>
Gerrit-Attention: neels <nhofm...@sysmocom.de>
Gerrit-Comment-Date: Mon, 08 Apr 2024 11:04:17 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment

Reply via email to